codenarc-user — CodeNarc Users

Re: [Codenarc-user] implementing Cloneable

[Roshan D. <ros...@gm...>]

No, it doesn't fix the error.

In fact adds one more error to the mix "UnnecessaryPublicModifier The public
keyword is unnecessary for methods source: public Object clone() {"


On Mon, Jul 4, 2011 at 6:30 PM, Hamlet DArcy <ham...@ca...> wrote:

> Interesting... if you add the public modifier, does it fix the error? All
> of our test cases have the public modifier in it, so this is a case we
> didn't test.
>
> We check for a clone method like this:
>
> ClassNode classNode = ...
> boolean hasMethod = classNode.getDeclaredMethod('clone', [] as Parameter[])
>
>
> ----- Original Message -----
> >
> >
> > Hi,
> >
> >
> > Can someone help me see what is wrong in the following code:
> >
> -------------------------------------------------------------------------------
> > class Dummy implements Cloneable {
> > Object clone ( ) {
> > super . clone ( )
> > }
> > }
> >
> -------------------------------------------------------------------------------
> >
> > CodeNarc output shows and I can't figure out how to make it happy:
> >
> >
> -------------------------------------------------------------------------------
> > line: 1 CloneableWithoutClone The class Dummy implements Cloneable
> > but does not define a proper clone() method source: class Dummy
> > implements Cloneable {
> >
> >
> -------------------------------------------------------------------------------
> >
> > --
> > Roshan
> > Blog: http://roshandawrani.wordpress.com/
> > Twitter: @roshandawrani
> > Skype: roshandawrani
> >
> >
> >
> ------------------------------------------------------------------------------
> > All of the data generated in your IT infrastructure is seriously
> > valuable.
> > Why? It contains a definitive record of application performance,
> > security
> > threats, fraudulent activity, and more. Splunk takes this data and
> > makes
> > sense of it. IT sense. And common sense.
> > http://p.sf.net/sfu/splunk-d2d-c2
> > _______________________________________________
> > Codenarc-user mailing list
> > Cod...@li...
> > https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >
>



-- 
Roshan
Blog: http://roshandawrani.wordpress.com/
Twitter: @roshandawrani <http://twitter.com/roshandawrani>
Skype: roshandawrani

Re: [Codenarc-user] implementing Cloneable

[Hamlet D. <ham...@ca...>]

Interesting... if you add the public modifier, does it fix the error? All of our test cases have the public modifier in it, so this is a case we didn't test. 

We check for a clone method like this: 

ClassNode classNode = ... 
boolean hasMethod = classNode.getDeclaredMethod('clone', [] as Parameter[])


----- Original Message -----
> 
> 
> Hi,
> 
> 
> Can someone help me see what is wrong in the following code:
> -------------------------------------------------------------------------------
> class Dummy implements Cloneable {
> Object clone ( ) {
> super . clone ( )
> }
> }
> -------------------------------------------------------------------------------
> ​
> CodeNarc output shows and I can't figure out how to make it happy:
> 
> -------------------------------------------------------------------------------
> line: 1 CloneableWithoutClone The class Dummy implements Cloneable
> but does not define a proper clone() method source: class Dummy
> implements Cloneable {
> 
> -------------------------------------------------------------------------------
> 
> --
> Roshan
> Blog: http://roshandawrani.wordpress.com/
> Twitter: @roshandawrani
> Skype: roshandawrani
> 
> 
> ------------------------------------------------------------------------------
> All of the data generated in your IT infrastructure is seriously
> valuable.
> Why? It contains a definitive record of application performance,
> security
> threats, fraudulent activity, and more. Splunk takes this data and
> makes
> sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-d2d-c2
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 

Re: [Codenarc-user] Unable to use CodeNarc Web Console

[Hamlet D. <ham...@ca...>]

OK, I will try and fix this. 

It looks like reflection is being used and GAE doesn't like that. 



----- Original Message -----
> 
> Hi,
> 
> 
> I am trying to use CodeNarc Web Console first time today, and am
> wondering if I am using it right.
> 
> 
> I am trying to run the following piece of code:
> ---------------------------------------------------------------------
> class Dummy { }
> 
> def d = new Dummy ( ) ​
> 
> ---------------------------------------------------------------------
> and it seems to be resulting in:
> 
> ---------------------------------------------------------------------
> java.lang.IllegalAccessException: Reflection is not allowed on
> java.lang.String java.lang.ThreadGroup.name at
> org.codenarc.rule.concurrency.ThreadGroupAstVisitor.isConstructorNamed(ThreadGroupRule.groovy:57)
> 	at
> 	org.codenarc.rule.concurrency.ThreadGroupAstVisitor.visitConstructorCallExpression(ThreadGroupRule.groovy:40)
> 	at
> 	org.codenarc.rule.AbstractAstVisitor.super$3$visitConstructorOrMethod(AbstractAstVisitor.groovy)
> 	at
> 	org.codenarc.rule.AbstractAstVisitor$_visitConstructorOrMethod_closure4.doCall(AbstractAstVisitor.groovy:186)
> 	at
> 	org.codenarc.rule.AbstractAstVisitor$_visitConstructorOrMethod_closure4.doCall(AbstractAstVisitor.groovy)
> 
> ---------------------------------------------------------------------
> 
> --
> Roshan
> Blog: http://roshandawrani.wordpress.com/
> Twitter: @roshandawrani
> Skype: roshandawrani
> 
> 
> ------------------------------------------------------------------------------
> All of the data generated in your IT infrastructure is seriously
> valuable.
> Why? It contains a definitive record of application performance,
> security
> threats, fraudulent activity, and more. Splunk takes this data and
> makes
> sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-d2d-c2
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 

[Codenarc-user] [ANN] Announcing Grails CodeNarc Plugin 0.14

[Chris M. <chr...@ea...>]

Announcing the release of the Grails CodeNarc Plugin
<http://grails.org/plugin/codenarc>  version 0.14.

CodeNarc <http://codenarc.sourceforge.net/>  is a static analysis tool for
Groovy source code. The Grails CodeNarc Plugin
<http://grails.org/plugin/codenarc>  provides CodeNarc integration for your
Grails projects.

New Plugin Features and Fixes

·         Upgrade to CodeNarc 0.14.
http://jira.grails.org/browse/GPCODENARC-19

·         Move CodeNarc configuration to “BuildConfig.groovy”. Write out
warning message if CodeNarc config still exists in “Config.groovy”.
http://jira.grails.org/browse/GPCODENARC-18

·         Fix: Exclude CodeNarc Plugin from war builds by default.
http://jira.grails.org/browse/GPCODENARC-14

·         Fix: Including AbcComplexity and CyclomaticComplexity rules in
ruleset makes no rules work. Include GMetrics jar as a dependency.
http://jira.grails.org/browse/GPCODENARC-13

 

CodeNarc: New and Updated Rules 

·         ExplicitLinkedHashMapInstantiation rule (basic) - This rule checks
for the explicit instantiation of a LinkedHashMap using the no-arg
constructor. In Groovy, it is best to write "new LinkedHashMap()" as "[:]",
which creates the same object.

·         DuplicateMapKey rule (basic) - A map literal is created with
duplicated key. The map entry will be overwritten.

·         DuplicateSetValue rule (basic) - A Set literal is created with
duplicate constant value. A set cannot contain two elements with the same
value.

·         EqualsOverloaded rule (basic) - The class has an equals method,
but the parameter of the method is not of type Object. It is not overriding
equals but instead overloading it.

·         ForLoopShouldBeWhileLoop (basic) - A for-loop without an init and
an update statement can be simplified to a while loop. 

·         NonFinalSubclassOfSensitiveInterface rule (security) - The
permissions classes such as java.security.Permission and
java.security.BasicPermission are designed to be extended. Classes that
derive from these permissions classes, however, must prohibit extension.
This prohibition ensures that malicious subclasses cannot change the
properties of the derived class. Classes that implement sensitive interfaces
such as java.security.PrivilegedAction and
java.security.PrivilegedActionException must also be declared final for
analogous reasons.

·         ImportFromSunPackages rule (imports) - Avoid importing anything
from the 'sun.*' packages. These packages are not portable and are likely to
change.

·         UnnecessaryFinalOnPrivateMethod rule (unnecessary) - A private
method is marked final. Private methods cannot be overridden, so marking it
final is unnecessary.

·         InsecureRandom rule (security) - Reports usages of
java.util.Random, which can produce very predictable results. If two
instances of Random are created with the same seed and sequence of method
calls, they will generate the exact same results. Use
java.security.SecureRandom instead, which provides a cryptographically
strong random number generator. SecureRandom uses PRNG, which means they are
using a deterministic algorithm to produce a pseudo-random number from a
true random seed. SecureRandom produces non-deterministic output.

·         DirectConnectionManagement rule (jdbc) - The J2EE standard
requires that applications use the container's resource management
facilities to obtain connections to resources. Every major web application
container provides pooled database connection management as part of its
resource management framework. Duplicating this functionality in an
application is difficult and error prone, which is part of the reason it is
forbidden under the J2EE standard.

·         ComparisonWithSelf (basic) - Checks for using a comparison
operator or equals() or compareTo() to compare a variable to itself, e.g.: x
== x, x != x, x <=> x, x < x, x > x, x <= x, x >= x, x.equals(x), or
x.compareTo(x), where x is a variable.

·         ComparisonOfTwoConstants (basic) - Checks for using a comparison
operator or equals() or compareTo() to compare two constants to each other
or two literals that contain only constant values.

·         FileCreateTempFile rule (security) - The File.createTempFile()
method is insecure, and has been deprecated by the ESAPI secure coding
library. It has been replaced by the ESAPI
Randomizer.getRandomFilename(String) method.

·         SystemExit rule (security) - Web applications should never call
System.exit(). A call to System.exit() is probably part of leftover debug
code or code imported from a non-J2EE application.

·         ObjectFinalize rule (security) - The finalize() method should only
be called by the JVM after the object has been garbage collected.

·         JavaIoPackageAccess rule (security) - This rule reports violations
of the Enterprise JavaBeans specification by using the java.io package to
access files or the file system.

·         UnsafeArrayDeclaration rule (security) - Triggers a violation when
an array is declared public, final, and static. Secure coding principles
state that, in most cases, an array declared public, final and static is a
bug because arrays are mutable objects.

·         PublicFinalizeMethod rule (security) - Creates a violation when
the program violates secure coding principles by declaring a finalize()
method public.

·         NonFinalPublicField rule (security) - Finds code that violates
secure coding principles for mobile code by declaring a member variable
public but not final.

·         UnnecessaryElseStatement rule (unnecessary) - When an if statement
block ends with a return statement the else is unnecessary. The logic in the
else branch can be run without being in a new scope.

·         StaticConnection rule (concurrency) - Creates violations when a
java.sql.Connection object is used as a static field. Database connections
stored in static fields will be shared between threads, which is unsafe and
can lead to race conditions.

·         UnnecessaryPackageReference (unnecessary) - Checks for explicit
package reference for classes that Groovy imports by default, such as
java.lang.String, java.util.Map and groovy.lang.Closure.

·         AbstractClassWithPublicConstructor (design) - Checks for abstract
classes that define a public constructor, which is useless and confusing.

·         StaticSimpleDateFormatField (concurrency) - Checks for static
SimpleDateFormat fields. SimpleDateFormat objects are not threadsafe, and
should not be shared across threads.

·         IllegalPackageReference (generic) - Checks for reference to any of
the packages configured in packageNames.

·         SpockIgnoreRestUsed rule (junit) - If Spock's @IgnoreRest appears
on any method, then all non-annotated test methods are not executed. This
behaviour is almost always unintended. It's fine to use @IgnoreRest locally
during development, but when committing code, it should be removed.

·         SwallowThreadDeath rule (exceptions) - Detects code that catches
java.lang.ThreadDeath without rethrowing it

·         MisorderedStaticImports rule (imports) - Static imports should
never be declared after nonstatic imports.

·         ConfusingMethodName (naming) - Existing rule updated to analyze
fields and field names as well as just methods.

·         PublicInstanceField rule (design) - Using public fields is
considered to be a bad design. Use properties instead.

·         UnnecessaryNullCheck (unnecessary) - Updated rule to flag null and
not-null checks against the this reference. The this reference can never be
null. 

·         ClassForName rule (basic) - Using Class.forName(...) is a common
way to add dynamic behavior to a system. However, using this method can
cause resource leaks because the classes can be pinned in memory for long
periods of time.

·         StatelessSingleton rule (design) - Rule finds occurrences of the
Singleton pattern where the object has no state (mutable or otherwise).
There is no point in creating a stateless Singleton, just make a new
instance with the new keyword instead.

·         InconsistentPropertySynchronization rule (concurrency) - The rule
is a little smarter now and flags code that defines a synchronized getter
without any setter and vice versa. Those methods will be generated in
unsynchronized from by the Groovy compiler later.

·         ClosureAsLastMethodParameter rule (basic) - If a method is called
and the last parameter is an inline closure then it can be declared outside
of the method call brackets.

·         SerialPersistentFields rule (serialization) - To use a
Serializable object's serialPersistentFields correctly, it must be declared
private, static, and final.

·         UnnecessaryParenthesesForMethodCallWithClosure rule (unnecessary)
- If a method is called and the only parameter to that method is an inline
closure then the brackets of the method call can be omitted.

 

CodeNarc: Bug Fixes 

·         #3290486 - AbstractClassWithoutAbstractMethod no longer flags
marker interfaces as abstract classes that do not define a method.

·         #3202691 - ClassNameRule rule is changed to handle $ in the class
name, which is in Inner and Nested classes by default.

·         #3206167 - VariableNameRule now has a violation message that
states the name of the variable and the regex it did not match.

·         #3206667 - FieldName, VariableName, MethodName, ParameterName,
UnusedVariable, PropertyName, and UnusedPrivateField violations message now
contains the class name of the enclosing class. This helps you configure an
exclude.

·         #3206258 - LoggerForDifferentClass rule now accepts MyClass.name
as a valid name for the logger.

·         #3206238 - The DuplicateNumberLiteral rule now allows you to
ignore literals in the 1.2d, 1.2f, and 1.2G format.

·         #3207628 - The UnusedPrivateMethodParameter rule now allows you to
ignore parameters by name. By default, parameters named 'ignore' and
'ignored' do not trigger a violation. You can configure this with the
'ignoreRegex' property on the rule.

·         #3205696 - The ConsecutiveStringConcatenation rule no longer
suggesting that you join numbers together, it only suggests joining strings
together.

·         #3206150 - Fixed UnusedGroovyImport rule so that imports with
aliases are ignored and do not cause violations.

·         #3207605 - Fixed UnusedPrivateMethod rule to recognize static
method references.

·         #3207607 - Fixed UnusedPrivateMethod rule to recognize access of
privately defined getters and setters as properties.

·         #3205697 - Fixed bug where the source line displayed for annotated
nodes was sometimes showing just the annotation and not the node.

·         #3288895 - Expand default test file regex pattern to include
*TestCase

·         #3291474 - Enhance StaticDateFormatFieldRule to also check for
static fields initialized to a DateFormat, e.g. static final DATE_FORMAT =
DateFormat.getDateInstance(DateFormat.LONG)

·         #3291559 - Enhance StaticCalendarFieldRule to also check for
untyped static fields that are initialized to a Calendar, e.g. static final
CALENDAR = Calendar.getInstance()

·         #3293429 - Fix UnnecessaryNullCheck duplicate violations.

·         #3295887 - Fix AddEmptyString duplicate violations.

·         #3299713 - Fix ImportFromSamePackage, UnnecessaryGroovyImport:
Star import.

·         #3300225 - Fix UnnecessaryGroovyImport: Check static imports.

·         #3290324 - Fix UnnecessarySemicolon: the contents of multiline
strings and GStrings are no longer checked, just the strings themselves. For
example, embedding JavaScript or Java within a Multi-line String would
previously trigger the violation.  

·         #3305896 - Fix LoggerForDifferentClass: The rule now correctly
handles inner classes (that are not static)

·         #3305019 - Updated the EmptyCatchBlockRule so that exceptions
named ignore or ignored will not trigger errors. The name is configurable. 

·         #3309062 - UnnecessaryGroovyImportRule handles static imports
incorrectly

-          Fixed the Explicit[Collection]Instantiation rules so that the
error messages are more descriptive.  

-          Fixed the InconsistentPropertySynchronization rule so that it
recognizes the new @Synchronized annotation.

·         #3308930 - LoggerWithWrongModifiersRule now contains a parameter
'allowProtectedLogger' so that loggers can also be instantiated as
'protected final LOG = Logger.getLogger(this.class)'. Also, it has a
'allowNonStatic' logger property, that allows you to create a non static
logger.

·         #3308930 - LoggerForDifferentClassRule now contains a parameter
'allowDerivedClasses'. When set, a logger may be created about
this.getClass(). 

·         #3309748 - FieldName:Do not treat non-static final fields as
constants

·         #3310413 - Fix UnnecessaryPublicModifier does not catch public on
constructors.

·         #3310521 - For all ExplicitCallToXxxMethod rules: Ignore calls to
super.Xxx(). Patch from René Scheibe

·         #3313550 - Some HTML violation descriptions in the properties
files were not well-formed HTML. This has been fixed.

·         #3205688 - Fixed false positives in
UseAssertEqualsInsteadOfAssertTrue when using the JUnit assertFalse method.

CodeNarc: Breaking Changes **** 

·         Moved the following rules out of the "basic" ruleset into the new
"serialization" ruleset:

o   SerialVersionUID

o   SerializableClassMustDefineSerialVersionUID

·         [Developer] Moved import-specific helper methods out of
AbstractRule into ImportUtil.

·         [Developer] The ExplicitTypeInstantiationAstVisitor is now an
abstract class that requires you to specify a custom violation message. This
should affect no one, but it is a backwards breaking change.

Thanks 

*	Thank you to Victor Savkin for sending in a patch with the
ForLoopShouldBeWhileLoop, UnnecessaryElse, StatelessSingleton,
PublicInstanceField, and EmptyCatchBlock rules.
*	Thank you to Jan Ahrens and Stefan Armbruster for the
SpockIgnoreRestUsed rule.
*	Thank you to Rob Fletcher and Klaus Baumecker for the
SwallowThreadDeath rule.
*	Thank you to Erik Pragt for the MisorderedStaticImports rule.
*	Thank you to Marcin Erdmann for the MisorderedStaticImports,
ClosureAsLastMethodParameterInBracketsRule, and
UnnecessaryBracketsForMethodWithClosureCall rules.
*	Thank you to Hubert 'Mr. Haki' Klein Ikkink for updating the
ConfusingMethodName rule.
*	Thank you to René Scheibe for the ExplicitLinkedHashMapInstantiation
rule and UnnecessaryGroovyImportRule and ExplicitCallToXxxMethod patches.

Visit the CodeNarc Home Page <http://codenarc.sourceforge.net/> 

 

[Codenarc-user] implementing Cloneable

[Roshan D. <ros...@gm...>]

Hi,

Can someone help me see what is wrong in the following code:
-------------------------------------------------------------------------------
class Dummy implements Cloneable {
    Object clone() {
        super.clone()
    }
}
-------------------------------------------------------------------------------

CodeNarc output shows and I can't figure out how to make it happy:
-------------------------------------------------------------------------------
line: 1 CloneableWithoutClone The class Dummy implements Cloneable but does
not define a proper clone() method source: class Dummy implements Cloneable
{
-------------------------------------------------------------------------------

-- 
Roshan
Blog: http://roshandawrani.wordpress.com/
Twitter: @roshandawrani <http://twitter.com/roshandawrani>
Skype: roshandawrani

[Codenarc-user] Unable to use CodeNarc Web Console

[Roshan D. <ros...@gm...>]

Hi,

I am trying to use CodeNarc Web Console first time today, and am wondering
if I am using it right.

I am trying to run the following piece of code:
---------------------------------------------------------------------
class Dummy {}

def d = new Dummy()
---------------------------------------------------------------------
and it seems to be resulting in:
---------------------------------------------------------------------

java.lang.IllegalAccessException: Reflection is not allowed on
java.lang.String java.lang.ThreadGroup.name
	at org.codenarc.rule.concurrency.ThreadGroupAstVisitor.isConstructorNamed(ThreadGroupRule.groovy:57)
	at org.codenarc.rule.concurrency.ThreadGroupAstVisitor.visitConstructorCallExpression(ThreadGroupRule.groovy:40)
	at org.codenarc.rule.AbstractAstVisitor.super$3$visitConstructorOrMethod(AbstractAstVisitor.groovy)
	at org.codenarc.rule.AbstractAstVisitor$_visitConstructorOrMethod_closure4.doCall(AbstractAstVisitor.groovy:186)
	at org.codenarc.rule.AbstractAstVisitor$_visitConstructorOrMethod_closure4.doCall(AbstractAstVisitor.groovy)

---------------------------------------------------------------------

-- 
Roshan
Blog: http://roshandawrani.wordpress.com/
Twitter: @roshandawrani <http://twitter.com/roshandawrani>
Skype: roshandawrani

[Codenarc-user] Griffon CodeNarc Plugin updated to 0.14

[Hamlet D'A. <ham...@gm...>]

Hurray, the Griffon CodeNarc Plugin has been updated to use CodeNarc
0.14. There are now almost 250 static analysis rules to run against
your Griffon project.

If you're not using CodeNarc already... well there is no better time
that now to give it a try.

The install directions are at the project webpage:
http://griffon.codehaus.org/Codenarc+Plugin

Here are the change notes:

Changes in version 0.14 (June 2011)
-------------------------------------------
NEW AND UPDATED RULES
- ExplicitLinkedHashMapInstantiation rule (basic) - This rule checks
for the explicit instantiation of a LinkedHashMap using the no-arg
constructor. In Groovy, it is best to write "new LinkedHashMap()" as
"[:]", which creates the same object.
- DuplicateMapKey rule (basic) - A map literal is created with
duplicated key. The map entry will be overwritten.
- DuplicateSetValue rule (basic) - A Set literal is created with
duplicate constant value. A set cannot contain two elements with the
same value.
- EqualsOverloaded rule (basic) - The class has an equals method, but
the parameter of the method is not of type Object. It is not
overriding equals but instead overloading it.
- ForLoopShouldBeWhileLoop (basic) - A for-loop without an init and an
update statement can be simplified to a while loop.
- NonFinalSubclassOfSensitiveInterface rule (security) - The
permissions classes such as java.security.Permission and
java.security.BasicPermission are designed to be extended. Classes
that derive from these permissions classes, however, must prohibit
extension. This prohibition ensures that malicious subclasses cannot
change the properties of the derived class. Classes that implement
sensitive interfaces such as java.security.PrivilegedAction and
java.security.PrivilegedActionException must also be declared final
for analogous reasons.
- ImportFromSunPackages rule (imports) - Avoid importing anything from
the 'sun.*' packages. These packages are not portable and are likely
to change.
- UnnecessaryFinalOnPrivateMethod rule (unnecessary) - A private
method is marked final. Private methods cannot be overridden, so
marking it final is unnecessary.
- InsecureRandom rule (security) - Reports usages of java.util.Random,
which can produce very predictable results. If two instances of Random
are created with the same seed and sequence of method calls, they will
generate the exact same results. Use java.security.SecureRandom
instead, which provides a cryptographically strong random number
generator. SecureRandom uses PRNG, which means they are using a
deterministic algorithm to produce a pseudo-random number from a true
random seed. SecureRandom produces non-deterministic output.
- DirectConnectionManagement rule (jdbc) - The J2EE standard requires
that applications use the container's resource management facilities
to obtain connections to resources. Every major web application
container provides pooled database connection management as part of
its resource management framework. Duplicating this functionality in
an application is difficult and error prone, which is part of the
reason it is forbidden under the J2EE standard.
- ComparisonWithSelf (basic) - Checks for using a comparison operator
or equals() or compareTo() to compare a variable to itself, e.g.: x ==
x, x != x, x <=> x, x < x, x > x, x <= x, x >= x, x.equals(x), or
x.compareTo(x), where x is a variable.
- ComparisonOfTwoConstants (basic) - Checks for using a comparison
operator or equals() or compareTo() to compare two constants to each
other or two literals that contain only constant values.
- FileCreateTempFile rule (security) - The File.createTempFile()
method is insecure, and has been deprecated by the ESAPI secure coding
library. It has been replaced by the ESAPI
Randomizer.getRandomFilename(String) method.
- SystemExit rule (security) - Web applications should never call
System.exit(). A call to System.exit() is probably part of leftover
debug code or code imported from a non-J2EE application.
- ObjectFinalize rule (security) - The finalize() method should only
be called by the JVM after the object has been garbage collected.
- JavaIoPackageAccess rule (security) - This rule reports violations
of the Enterprise JavaBeans specification by using the java.io package
to access files or the file system.
- UnsafeArrayDeclaration rule (security) - Triggers a violation when
an array is declared public, final, and static. Secure coding
principles state that, in most cases, an array declared public, final
and static is a bug because arrays are mutable objects.
- PublicFinalizeMethod rule (security) - Creates a violation when the
program violates secure coding principles by declaring a finalize()
method public.
- NonFinalPublicField rule (security) - Finds code that violates
secure coding principles for mobile code by declaring a member
variable public but not final.
- UnnecessaryElseStatement rule (unnecessary) - When an if statement
block ends with a return statement the else is unnecessary. The logic
in the else branch can be run without being in a new scope.
- StaticConnection rule (concurrency) - Creates violations when a
java.sql.Connection object is used as a static field. Database
connections stored in static fields will be shared between threads,
which is unsafe and can lead to race conditions.
- UnnecessaryPackageReference (unnecessary) - Checks for explicit
package reference for classes that Groovy imports by default, such as
java.lang.String, java.util.Map and groovy.lang.Closure.
- AbstractClassWithPublicConstructor (design) - Checks for abstract
classes that define a public constructor, which is useless and
confusing.
- StaticSimpleDateFormatField (concurrency) - Checks for static
SimpleDateFormat fields. SimpleDateFormat objects are not threadsafe,
and should not be shared across threads.
- IllegalPackageReference (generic) - Checks for reference to any of
the packages configured in packageNames.
- SpockIgnoreRestUsed rule (junit) - If Spock's @IgnoreRest appears on
any method, then all non-annotated test methods are not executed. This
behaviour is almost always unintended. It's fine to use @IgnoreRest
locally during development, but when committing code, it should be
removed.
- SwallowThreadDeath rule (exceptions) - Detects code that catches
java.lang.ThreadDeath without rethrowing it
- MisorderedStaticImports rule (imports) - Static imports should never
be declared after nonstatic imports.
- ConfusingMethodName (naming) - Existing rule updated to analyze
fields and field names as well as just methods.
- PublicInstanceField rule (design) - Using public fields is
considered to be a bad design. Use properties instead.
- UnnecessaryNullCheck (unnecessary) - Updated rule to flag null and
not-null checks against the this reference. The this reference can
never be null.
- ClassForName rule (basic) - Using Class.forName(...) is a common way
to add dynamic behavior to a system. However, using this method can
cause resource leaks because the classes can be pinned in memory for
long periods of time.
- StatelessSingleton rule (design) - Rule finds occurrences of the
Singleton pattern where the object has no state (mutable or
otherwise). There is no point in creating a stateless Singleton, just
make a new instance with the new keyword instead.
- InconsistentPropertySynchronization rule (concurrency) - The rule is
a little smarter now and flags code that defines a synchronized getter
without any setter and vice versa. Those methods will be generated in
unsynchronized from by the Groovy compiler later.
- ClosureAsLastMethodParameter rule (basic) - If a method is called
and the last parameter is an inline closure then it can be declared
outside of the method call brackets.
- SerialPersistentFields rule (serialization) - To use a Serializable
object's serialPersistentFields correctly, it must be declared
private, static, and final.
- UnnecessaryParenthesesForMethodCallWithClosure rule (unnecessary) -
If a method is called and the only parameter to that method is an
inline closure then the brackets of the method call can be omitted.

BUG FIXES and NEW FEATURES
- Groovy 1.8 Support - CodeNarc continues to be built with Groovy 1.7,
but should be able to be run with a Groovy 1.8 Runtime.
- #3290486 - AbstractClassWithoutAbstractMethod no longer flags marker
interfaces as abstract classes that do not define a method.
- #3202691 - ClassNameRule rule is changed to handle $ in the class
name, which is in Inner and Nested classes by default.
- #3206167 - VariableNameRule now has a violation message that states
the name of the variable and the regex it did not match.
- #3206667 - FieldName, VariableName, MethodName, ParameterName,
UnusedVariable, PropertyName, and UnusedPrivateField violations
message now contains the class name of the enclosing class. This helps
you configure an exclude.
- #3206258 - LoggerForDifferentClass rule now accepts MyClass.name as
a valid name for the logger.
- #3206238 - The DuplicateNumberLiteral rule now allows you to ignore
literals in the 1.2d, 1.2f, and 1.2G format.
- #3207628 - The UnusedPrivateMethodParameter rule now allows you to
ignore parameters by name. By default, parameters named 'ignore' and
'ignored' do not trigger a violation. You can configure this with the
'ignoreRegex' property on the rule.
- #3205696 - The ConsecutiveStringConcatenation rule no longer
suggesting that you join numbers together, it only suggests joining
strings together.
- #3206150 - Fixed UnusedGroovyImport rule so that imports with
aliases are ignored and do not cause violations.
- #3207605 - Fixed UnusedPrivateMethod rule to recognize static method
references.
- #3207607 - Fixed UnusedPrivateMethod rule to recognize access of
privately defined getters and setters as properties.
- #3205697 - Fixed bug where the source line displayed for annotated
nodes was sometimes showing just the annotation and not the node.
- #3288895 - Expand default test file regex pattern to include *TestCase
- #3291474 - Enhance StaticDateFormatFieldRule to also check for
static fields initialized to a DateFormat, e.g. static final
DATE_FORMAT = DateFormat.getDateInstance(DateFormat.LONG)
- #3291559 - Enhance StaticCalendarFieldRule to also check for untyped
static fields that are initialized to a Calendar, e.g. static final
CALENDAR = Calendar.getInstance()
- #3293429 - Fix UnnecessaryNullCheck duplicate violations.
- #3295887 - Fix AddEmptyString duplicate violations.
- #3299713 - Fix ImportFromSamePackage, UnnecessaryGroovyImport: Star import.
- #3300225 - Fix UnnecessaryGroovyImport: Check static imports.
- #3290324 - Fix UnnecessarySemicolon: the contents of multiline
strings and GStrings are no longer checked, just the strings
themselves. For example, embedding JavaScript or Java within a
Multi-line String would previously trigger the violation.
- #3305896 - Fix LoggerForDifferentClass: The rule now correctly
handles inner classes (that are not static)
- #3305019 - Updated the EmptyCatchBlockRule so that exceptions named
ignore or ignored will not trigger errors. The name is configurable.
- #3309062 - UnnecessaryGroovyImportRule handles static imports incorrectly
-          - Fixed the Explicit[Collection]Instantiation rules so that
the error messages are more descriptive.
-          - Fixed the InconsistentPropertySynchronization rule so
that it recognizes the new @Synchronized annotation.
- #3308930 - LoggerWithWrongModifiersRule now contains a parameter
'allowProtectedLogger' so that loggers can also be instantiated as
'protected final LOG = Logger.getLogger(this.class)'. Also, it has a
'allowNonStatic' logger property, that allows you to create a non
static logger.
- #3308930 - LoggerForDifferentClassRule now contains a parameter
'allowDerivedClasses'. When set, a logger may be created about
this.getClass().
- #3309748 - FieldName:Do not treat non-static final fields as constants
- #3310413 - Fix UnnecessaryPublicModifier does not catch public on
constructors.
- #3310521 - For all ExplicitCallToXxxMethod rules: Ignore calls to
super.Xxx(). Patch from René Scheibe
- #3313550 - Some HTML violation descriptions in the properties files
were not well-formed HTML. This has been fixed.
- #3205688 - Fixed false positives in
UseAssertEqualsInsteadOfAssertTrue when using the JUnit assertFalse
method.

BREAKING CHANGES
*** Moved the following rules out of the "basic" ruleset into the new
"serialization" ruleset:
   - SerialVersionUID
   - SerializableClassMustDefineSerialVersionUID
*** Moved import-specific helper methods out of AbstractRule into ImportUtil.
*** The ExplicitTypeInstantiationAstVisitor is now an abstract class
that requires you to specify a custom violation message. This should
affect no one, but it is a backwards breaking change. ppp

THANKS
  Thank you to Victor Savkin for sending in a patch with the
ForLoopShouldBeWhileLoop, UnnecessaryElse, StatelessSingleton,
PublicInstanceField, and EmptyCatchBlock rules.
  Thank you to Jan Ahrens and Stefan Armbruster for the
SpockIgnoreRestUsed rule.
  Thank you to Rob Fletcher and Klaus Baumecker for the SwallowThreadDeath rule.
  Thank you to Erik Pragt for the MisorderedStaticImports rule.
  Thank you to Marcin Erdmann for the MisorderedStaticImports,
ClosureAsLastMethodParameterInBracketsRule, and
UnnecessaryBracketsForMethodWithClosureCall rules.
  Thank you to Hubert 'Mr. Haki' Klein Ikkink for updating the
ConfusingMethodName rule.
  Thank you to René Scheibe for the ExplicitLinkedHashMapInstantiation
rule and UnnecessaryGroovyImportRule and ExplicitCallToXxxMethod
patches.


-- 
Hamlet D'Arcy
ham...@gm...

[Codenarc-user] [ANN] Announcing CodeNarc 0.14

[Chris M. <chr...@ea...>]

The CodeNarc Team is proud to announce the release of version 0.14. 

CodeNarc <http://codenarc.sourceforge.net/>  is a static analysis tool for
Groovy source code. Version 0.14 adds 35 new rules (bringing the total to
241 rules). Try it out on the CodeNarc web console
<http://meetcodenarc.appspot.com/edit/14001> , running on Google App Engine.

New Features 

*	Groovy 1.8 Support - CodeNarc continues to be built with Groovy 1.7,
but should be able to be run with a Groovy 1.8 Runtime.

New and Updated Rules 

·         ExplicitLinkedHashMapInstantiation rule (basic) - This rule checks
for the explicit instantiation of a LinkedHashMap using the no-arg
constructor. In Groovy, it is best to write "new LinkedHashMap()" as "[:]",
which creates the same object.

·         DuplicateMapKey rule (basic) - A map literal is created with
duplicated key. The map entry will be overwritten.

·         DuplicateSetValue rule (basic) - A Set literal is created with
duplicate constant value. A set cannot contain two elements with the same
value.

·         EqualsOverloaded rule (basic) - The class has an equals method,
but the parameter of the method is not of type Object. It is not overriding
equals but instead overloading it.

·         ForLoopShouldBeWhileLoop (basic) - A for-loop without an init and
an update statement can be simplified to a while loop. 

·         NonFinalSubclassOfSensitiveInterface rule (security) - The
permissions classes such as java.security.Permission and
java.security.BasicPermission are designed to be extended. Classes that
derive from these permissions classes, however, must prohibit extension.
This prohibition ensures that malicious subclasses cannot change the
properties of the derived class. Classes that implement sensitive interfaces
such as java.security.PrivilegedAction and
java.security.PrivilegedActionException must also be declared final for
analogous reasons.

·         ImportFromSunPackages rule (imports) - Avoid importing anything
from the 'sun.*' packages. These packages are not portable and are likely to
change.

·         UnnecessaryFinalOnPrivateMethod rule (unnecessary) - A private
method is marked final. Private methods cannot be overridden, so marking it
final is unnecessary.

·         InsecureRandom rule (security) - Reports usages of
java.util.Random, which can produce very predictable results. If two
instances of Random are created with the same seed and sequence of method
calls, they will generate the exact same results. Use
java.security.SecureRandom instead, which provides a cryptographically
strong random number generator. SecureRandom uses PRNG, which means they are
using a deterministic algorithm to produce a pseudo-random number from a
true random seed. SecureRandom produces non-deterministic output.

·         DirectConnectionManagement rule (jdbc) - The J2EE standard
requires that applications use the container's resource management
facilities to obtain connections to resources. Every major web application
container provides pooled database connection management as part of its
resource management framework. Duplicating this functionality in an
application is difficult and error prone, which is part of the reason it is
forbidden under the J2EE standard.

·         ComparisonWithSelf (basic) - Checks for using a comparison
operator or equals() or compareTo() to compare a variable to itself, e.g.: x
== x, x != x, x <=> x, x < x, x > x, x <= x, x >= x, x.equals(x), or
x.compareTo(x), where x is a variable.

·         ComparisonOfTwoConstants (basic) - Checks for using a comparison
operator or equals() or compareTo() to compare two constants to each other
or two literals that contain only constant values.

·         FileCreateTempFile rule (security) - The File.createTempFile()
method is insecure, and has been deprecated by the ESAPI secure coding
library. It has been replaced by the ESAPI
Randomizer.getRandomFilename(String) method.

·         SystemExit rule (security) - Web applications should never call
System.exit(). A call to System.exit() is probably part of leftover debug
code or code imported from a non-J2EE application.

·         ObjectFinalize rule (security) - The finalize() method should only
be called by the JVM after the object has been garbage collected.

·         JavaIoPackageAccess rule (security) - This rule reports violations
of the Enterprise JavaBeans specification by using the java.io package to
access files or the file system.

·         UnsafeArrayDeclaration rule (security) - Triggers a violation when
an array is declared public, final, and static. Secure coding principles
state that, in most cases, an array declared public, final and static is a
bug because arrays are mutable objects.

·         PublicFinalizeMethod rule (security) - Creates a violation when
the program violates secure coding principles by declaring a finalize()
method public.

·         NonFinalPublicField rule (security) - Finds code that violates
secure coding principles for mobile code by declaring a member variable
public but not final.

·         UnnecessaryElseStatement rule (unnecessary) - When an if statement
block ends with a return statement the else is unnecessary. The logic in the
else branch can be run without being in a new scope.

·         StaticConnection rule (concurrency) - Creates violations when a
java.sql.Connection object is used as a static field. Database connections
stored in static fields will be shared between threads, which is unsafe and
can lead to race conditions.

·         UnnecessaryPackageReference (unnecessary) - Checks for explicit
package reference for classes that Groovy imports by default, such as
java.lang.String, java.util.Map and groovy.lang.Closure.

·         AbstractClassWithPublicConstructor (design) - Checks for abstract
classes that define a public constructor, which is useless and confusing.

·         StaticSimpleDateFormatField (concurrency) - Checks for static
SimpleDateFormat fields. SimpleDateFormat objects are not threadsafe, and
should not be shared across threads.

·         IllegalPackageReference (generic) - Checks for reference to any of
the packages configured in packageNames.

·         SpockIgnoreRestUsed rule (junit) - If Spock's @IgnoreRest appears
on any method, then all non-annotated test methods are not executed. This
behaviour is almost always unintended. It's fine to use @IgnoreRest locally
during development, but when committing code, it should be removed.

·         SwallowThreadDeath rule (exceptions) - Detects code that catches
java.lang.ThreadDeath without rethrowing it

·         MisorderedStaticImports rule (imports) - Static imports should
never be declared after nonstatic imports.

·         ConfusingMethodName (naming) - Existing rule updated to analyze
fields and field names as well as just methods.

·         PublicInstanceField rule (design) - Using public fields is
considered to be a bad design. Use properties instead.

·         UnnecessaryNullCheck (unnecessary) - Updated rule to flag null and
not-null checks against the this reference. The this reference can never be
null. 

·         ClassForName rule (basic) - Using Class.forName(...) is a common
way to add dynamic behavior to a system. However, using this method can
cause resource leaks because the classes can be pinned in memory for long
periods of time.

·         StatelessSingleton rule (design) - Rule finds occurrences of the
Singleton pattern where the object has no state (mutable or otherwise).
There is no point in creating a stateless Singleton, just make a new
instance with the new keyword instead.

·         InconsistentPropertySynchronization rule (concurrency) - The rule
is a little smarter now and flags code that defines a synchronized getter
without any setter and vice versa. Those methods will be generated in
unsynchronized from by the Groovy compiler later.

·         ClosureAsLastMethodParameter rule (basic) - If a method is called
and the last parameter is an inline closure then it can be declared outside
of the method call brackets.

·         SerialPersistentFields rule (serialization) - To use a
Serializable object's serialPersistentFields correctly, it must be declared
private, static, and final.

·         UnnecessaryParenthesesForMethodCallWithClosure rule (unnecessary)
- If a method is called and the only parameter to that method is an inline
closure then the brackets of the method call can be omitted.

 

Bug Fixes 

·         #3290486 - AbstractClassWithoutAbstractMethod no longer flags
marker interfaces as abstract classes that do not define a method.

·         #3202691 - ClassNameRule rule is changed to handle $ in the class
name, which is in Inner and Nested classes by default.

·         #3206167 - VariableNameRule now has a violation message that
states the name of the variable and the regex it did not match.

·         #3206667 - FieldName, VariableName, MethodName, ParameterName,
UnusedVariable, PropertyName, and UnusedPrivateField violations message now
contains the class name of the enclosing class. This helps you configure an
exclude.

·         #3206258 - LoggerForDifferentClass rule now accepts MyClass.name
as a valid name for the logger.

·         #3206238 - The DuplicateNumberLiteral rule now allows you to
ignore literals in the 1.2d, 1.2f, and 1.2G format.

·         #3207628 - The UnusedPrivateMethodParameter rule now allows you to
ignore parameters by name. By default, parameters named 'ignore' and
'ignored' do not trigger a violation. You can configure this with the
'ignoreRegex' property on the rule.

·         #3205696 - The ConsecutiveStringConcatenation rule no longer
suggesting that you join numbers together, it only suggests joining strings
together.

·         #3206150 - Fixed UnusedGroovyImport rule so that imports with
aliases are ignored and do not cause violations.

·         #3207605 - Fixed UnusedPrivateMethod rule to recognize static
method references.

·         #3207607 - Fixed UnusedPrivateMethod rule to recognize access of
privately defined getters and setters as properties.

·         #3205697 - Fixed bug where the source line displayed for annotated
nodes was sometimes showing just the annotation and not the node.

·         #3288895 - Expand default test file regex pattern to include
*TestCase

·         #3291474 - Enhance StaticDateFormatFieldRule to also check for
static fields initialized to a DateFormat, e.g. static final DATE_FORMAT =
DateFormat.getDateInstance(DateFormat.LONG)

·         #3291559 - Enhance StaticCalendarFieldRule to also check for
untyped static fields that are initialized to a Calendar, e.g. static final
CALENDAR = Calendar.getInstance()

·         #3293429 - Fix UnnecessaryNullCheck duplicate violations.

·         #3295887 - Fix AddEmptyString duplicate violations.

·         #3299713 - Fix ImportFromSamePackage, UnnecessaryGroovyImport:
Star import.

·         #3300225 - Fix UnnecessaryGroovyImport: Check static imports.

·         #3290324 - Fix UnnecessarySemicolon: the contents of multiline
strings and GStrings are no longer checked, just the strings themselves. For
example, embedding JavaScript or Java within a Multi-line String would
previously trigger the violation.  

·         #3305896 - Fix LoggerForDifferentClass: The rule now correctly
handles inner classes (that are not static)

·         #3305019 - Updated the EmptyCatchBlockRule so that exceptions
named ignore or ignored will not trigger errors. The name is configurable. 

·         #3309062 - UnnecessaryGroovyImportRule handles static imports
incorrectly

-          Fixed the Explicit[Collection]Instantiation rules so that the
error messages are more descriptive.  

-          Fixed the InconsistentPropertySynchronization rule so that it
recognizes the new @Synchronized annotation.

·         #3308930 - LoggerWithWrongModifiersRule now contains a parameter
'allowProtectedLogger' so that loggers can also be instantiated as
'protected final LOG = Logger.getLogger(this.class)'. Also, it has a
'allowNonStatic' logger property, that allows you to create a non static
logger.

·         #3308930 - LoggerForDifferentClassRule now contains a parameter
'allowDerivedClasses'. When set, a logger may be created about
this.getClass(). 

·         #3309748 - FieldName:Do not treat non-static final fields as
constants

·         #3310413 - Fix UnnecessaryPublicModifier does not catch public on
constructors.

·         #3310521 - For all ExplicitCallToXxxMethod rules: Ignore calls to
super.Xxx(). Patch from René Scheibe

·         #3313550 - Some HTML violation descriptions in the properties
files were not well-formed HTML. This has been fixed.

·         #3205688 - Fixed false positives in
UseAssertEqualsInsteadOfAssertTrue when using the JUnit assertFalse method.

Breaking Changes **** 

·         Moved the following rules out of the "basic" ruleset into the new
"serialization" ruleset:

o   SerialVersionUID

o   SerializableClassMustDefineSerialVersionUID

·         [Developer] Moved import-specific helper methods out of
AbstractRule into ImportUtil.

·         [Developer] The ExplicitTypeInstantiationAstVisitor is now an
abstract class that requires you to specify a custom violation message. This
should affect no one, but it is a backwards breaking change.

Thanks 

*	Thank you to Victor Savkin for sending in a patch with the
ForLoopShouldBeWhileLoop, UnnecessaryElse, StatelessSingleton,
PublicInstanceField, and EmptyCatchBlock rules.
*	Thank you to Jan Ahrens and Stefan Armbruster for the
SpockIgnoreRestUsed rule.
*	Thank you to Rob Fletcher and Klaus Baumecker for the
SwallowThreadDeath rule.
*	Thank you to Erik Pragt for the MisorderedStaticImports rule.
*	Thank you to Marcin Erdmann for the MisorderedStaticImports,
ClosureAsLastMethodParameterInBracketsRule, and
UnnecessaryBracketsForMethodWithClosureCall rules.
*	Thank you to Hubert 'Mr. Haki' Klein Ikkink for updating the
ConfusingMethodName rule.
*	Thank you to René Scheibe for the ExplicitLinkedHashMapInstantiation
rule and UnnecessaryGroovyImportRule and ExplicitCallToXxxMethod patches.

Visit the CodeNarc Home Page <http://codenarc.sourceforge.net/> 

 

Re: [Codenarc-user] [Codenarc-developer] Considering new "serializable" ruleset

[Hamlet D. <ham...@ca...>]

It is fine as long as we're explicit in the release notes about it. 

----- Original Message -----
> 
> 
> 
> 
> I am considering creating a new “serialization” ruleset, and
> including these three rules:
> 
> 
> 
> · SerialPersistentFields (0.14)
> 
> · SerialVersionUID (0.11)
> 
> · SerializableClassMustDefineSerialVersionUID (0.13)
> 
> 
> 
> That implies moving the last two rules, which were released in
> previous versions of CodeNarc. The SerialPersistentFields rule is
> included in the upcoming 0.14 release.
> 
> 
> 
> If I do that, I realize that might cause some (hopefully minor)
> breakage for users that already configured the two existing rules
> within the context of an included rulesets, but not if they are only
> configured within “codenarc.properties” or as individual rules.
> 
> 
> 
> Anyone have opinions on that, one way or another?
> 
> 
> 
> Chris
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with
> vRanger.
> Installation's a snap, and flexible recovery options mean your data
> is safe,
> secure and there when you need it. Discover what all the cheering's
> about.
> Get your free trial download today.
> http://p.sf.net/sfu/quest-dev2dev2
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
> 

[Codenarc-user] Considering new "serializable" ruleset

[Chris M. <chr...@ea...>]

I am considering creating a new "serialization" ruleset, and including these
three rules:

 

.         SerialPersistentFields (0.14)

.         SerialVersionUID (0.11)

.         SerializableClassMustDefineSerialVersionUID (0.13)

 

That implies moving the last two rules, which were released in previous
versions of CodeNarc. The SerialPersistentFields rule is included in the
upcoming 0.14 release.

 

If I do that, I realize that might cause some (hopefully minor) breakage for
users that already configured the two existing rules within the context of
an included rulesets, but not if they are only configured within
"codenarc.properties" or as individual rules.

 

Anyone have opinions on that, one way or another?

 

Chris

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[R. S. <ren...@go...>]

On 06/01/2011 11:36 AM, Hamlet DArcy wrote:
>> I think two rules for the logger is adding too much complexity. I
>> still
>> favor my original patch. This supports the two logger instantiation
> 
> 
> There are already two different rules since version 0.12: one for logger modifiers(LoggerWithWrongModifiersRule) and one for the parameter to the logger factory (LoggerForDifferentClassRule). The problem with the patch was that it added logic about the parameter into the rule about the visibility. So there were no longer two distinct rules, but two rules that overlapped with each other. If you want logic about accepting "this.getClass()" as a valid parameter, then that logic belongs in LoggerForDifferentClassRule. If you want logic about accepting a non-static logger, then that logic belongs in LoggerWithWrongModifiersRule. If you want to allow a non-static logger that accepts "this.getClass()" as a parameter then you need to configure two rules. 
> 
> Considering these properties: 
> 
>>> LoggerWithWrongModifiersRule has 2 properties:
>>>     boolean allowProtectedLogger = false
>>>     boolean allowNonStaticLogger = false
>>>
>>> LoggerForDifferentClassRule has 1 property:
>>>     boolean allowDerivedLogger = false
> 
> By default, the rules do what is right and most people will never know these rules have properties. There is no complexity for most of users, yet for those that want finer control then every use case is covered. 
> 
> I would leave the extra properties in. The default configuration does the right thing, and every use case is covered for those that want something different... 
> 
> What I would not do is accept the original patch because it mixes the logic about parameter checking between two different rules. 
Right. I missed the existance of the LoggerForDifferentClassRule until I
saw it in your changes.

Regards,
René Scheibe

> ----- Original Message -----
>> My thoughts match Chris'.
>>
>> I think two rules for the logger is adding too much complexity. I
>> still
>> favor my original patch. This supports the two logger instantiation
>> cases I have seen and classified as correct in recent years.
>>
>> Sure there are more cases out there (injecting via Spring, per
>> thread,
>> new Logger, etc.) but this rule should stick to the common ones. If
>> there are too many posibilities it will just create confusion. In
>> case
>> we missed a relevant case people will raise their hands.
>>
>> Regards,
>> René Scheibe
>>
>> On 06/01/2011 02:13 AM, Chris Mair wrote:
>>> Hamlet / René,
>>>
>>> Thanks for working on this Hamlet. I'm sorry I did not respond
>>> sooner. I see no value, for me anyway, in the
>>> LoggerWithWrongModifiersRule supporting allowProtectedLogger or
>>> allowNonStaticLogger properties. I will not enable those because I
>>> do want to find the genuine bugs where a logger is incorrectly
>>> defined as protected or as non-static. I guess I will just
>>> configure exceptions for those (relatively rare) framework
>>> superclasses where I define the LOG in the superclass for all
>>> child classes.
>>>
>>> Will you use those properties René? If they provide value, then
>>> fine. But if we are adding complexity that no one will (or should)
>>> use, then I don't want to do that.
>>>
>>> Chris
>>> -----Original Message-----
>>> From: Hamlet DArcy [mailto:ham...@ca...]
>>> Sent: Tuesday, May 31, 2011 4:55 AM
>>> To: René Scheibe
>>> Cc: Cod...@li...
>>> Subject: Re: [Codenarc-user] LoggerWithWrongModifiersRule
>>> supporting subclasses
>>>
>>> I think the best solution is this because it covers every use case:
>>>
>>> LoggerWithWrongModifiersRule has 2 properties:
>>>     boolean allowProtectedLogger = false
>>>     boolean allowNonStaticLogger = false
>>>
>>> LoggerForDifferentClassRule has 1 property:
>>>     boolean allowDerivedLogger = false
>>>
>>>
>>> Then everything is covered for everyone with no overlap.
>>>
>>>
>>> ----- Original Message -----
>>>> It's not about the "protected" (this is just required).
>>>>
>>>> The interesting part is the non-static one. Because of it, a
>>>> superclass uses the logger of the derived class based on
>>>> "getLogger(this.class)".
>>>>
>>>> With such a logger it's for example possible to write every log
>>>> message of a class (including its superclass) to a separate file
>>>> because they are using the same logger.
>>>>
>>>> Regards,
>>>> René Scheibe
>>>>
>>>> On 05/31/2011 09:55 AM, Hamlet DArcy wrote:
>>>>> Personally, I would require loggers to be static, private, and
>>>>> final.
>>>>> If someone really needed a protected logger then i would still
>>>>> make
>>>>> it static and final, just protected.
>>>>>
>>>>> So I think we need both these settings.
>>>>>
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>>> That's a great question.
>>>>>>
>>>>>> I do use that pattern in framework superclasses:
>>>>>> 	protected final LOG = Logger.getLogger(this.class)
>>>>>>
>>>>>> to provide a built-in logger for all subclasses. And so I have
>>>>>> to
>>>>>> configure exceptions for this rule for those superclasses.
>>>>>>
>>>>>> Is there any value in splitting up the "exception" configuration
>>>>>> into two properties, if we do allow it?
>>>>>>   * allowNonStaticLogger
>>>>>>   * allowProtectedLogger
>>>>>>
>>>>>> Are there use cases we can envision where we (or anyone) would
>>>>>> choose to allow non-static but not protected, or vice versa?
>>>>>>
>>>>>> Chris
>>>>>> -----Original Message-----
>>>>>> From: Hamlet D'Arcy [mailto:ham...@gm...]
>>>>>> Sent: Monday, May 30, 2011 2:50 AM
>>>>>> To: René Scheibe; Cod...@li...
>>>>>> Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting
>>>>>> subclasses
>>>>>>
>>>>>> Hi everyone,
>>>>>>
>>>>>> This email is about the idea about the
>>>>>> LoggerWithWrongModifiersRule
>>>>>> supporting subclasses.
>>>>>>
>>>>>> René filed an issue (and supplied a patch) describing the issue:
>>>>>> https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&g
>>>>>> roup_id=2
>>>>>> 50145&atid=1126575#
>>>>>>
>>>>>> Basically, the question is how subclasses should be allowed to
>>>>>> access a Logger in a parent class.
>>>>>>
>>>>>> Normally Loggers are private, static, and final. This patch
>>>>>> allows
>>>>>> you to accept this as valid code when you turn a certain
>>>>>> property
>>>>>> on:
>>>>>>
>>>>>>     protected final LOG = Logger.getLogger(this.class)
>>>>>>
>>>>>> I'd like to discuss this just a little more before committing
>>>>>> the
>>>>>> patch.
>>>>>>
>>>>>> With this change, it is acceptable to have a non-static logger.
>>>>>> So
>>>>>> each instance of a subclass makes and creates a logger. This
>>>>>> does
>>>>>> not seem right, and I would reject it in a code review as
>>>>>> improper
>>>>>> logging. I would accept this form though:
>>>>>>
>>>>>>     protected static final LOG = Logger.getLogger(MyClass.class)
>>>>>>
>>>>>> Do we really want to allow non-static Loggers?
>>>>>>
>>>>>> if so, I recommend two new properties instead of just one:
>>>>>>   * allowNonStaticLogger
>>>>>>   * allowProtectedLogger
>>>>>>
>>>>>> Then the user can control the behavior a little better.
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Hamlet D'Arcy
>>>>>> ham...@gm...
>>>>>>
>>>>>> -------------------------------------------------------------------
>>>>>> ---------
>>>>>> --
>>>>>> vRanger cuts backup time in half-while increasing security.
>>>>>> With the market-leading solution for virtual backup and
>>>>>> recovery,
>>>>>> you get blazing-fast, flexible, and affordable data protection.
>>>>>> Download your free trial now.
>>>>>> http://p.sf.net/sfu/quest-d2dcopy1
>>>>>> _______________________________________________
>>>>>> Codenarc-user mailing list
>>>>>> Cod...@li...
>>>>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>>>>
>>>>>>
>>>>>> -------------------------------------------------------------------
>>>>>> ----------- Simplify data backup and recovery for your virtual
>>>>>> environment with vRanger.
>>>>>> Installation's a snap, and flexible recovery options mean your
>>>>>> data
>>>>>> is safe, secure and there when you need it. Data protection
>>>>>> magic?
>>>>>> Nope - It's vRanger. Get your free trial download today.
>>>>>> http://p.sf.net/sfu/quest-sfdev2dev
>>>>>> _______________________________________________
>>>>>> Codenarc-user mailing list
>>>>>> Cod...@li...
>>>>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>>>>
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> -------- Simplify data backup and recovery for your virtual
>>>> environment with vRanger.
>>>> Installation's a snap, and flexible recovery options mean your
>>>> data is
>>>> safe, secure and there when you need it. Data protection magic?
>>>> Nope - It's vRanger. Get your free trial download today.
>>>> http://p.sf.net/sfu/quest-sfdev2dev
>>>> _______________________________________________
>>>> Codenarc-user mailing list
>>>> Cod...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Simplify data backup and recovery for your virtual environment with
>>> vRanger.
>>> Installation's a snap, and flexible recovery options mean your data
>>> is safe, secure and there when you need it. Data protection magic?
>>> Nope - It's vRanger. Get your free trial download today.
>>> http://p.sf.net/sfu/quest-sfdev2dev
>>> _______________________________________________
>>> Codenarc-user mailing list
>>> Cod...@li...
>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>
>>
>>

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[Hamlet D. <ham...@ca...>]

> I think two rules for the logger is adding too much complexity. I
> still
> favor my original patch. This supports the two logger instantiation


There are already two different rules since version 0.12: one for logger modifiers(LoggerWithWrongModifiersRule) and one for the parameter to the logger factory (LoggerForDifferentClassRule). The problem with the patch was that it added logic about the parameter into the rule about the visibility. So there were no longer two distinct rules, but two rules that overlapped with each other. If you want logic about accepting "this.getClass()" as a valid parameter, then that logic belongs in LoggerForDifferentClassRule. If you want logic about accepting a non-static logger, then that logic belongs in LoggerWithWrongModifiersRule. If you want to allow a non-static logger that accepts "this.getClass()" as a parameter then you need to configure two rules. 

Considering these properties: 

> > LoggerWithWrongModifiersRule has 2 properties:
> >     boolean allowProtectedLogger = false
> >     boolean allowNonStaticLogger = false
> > 
> > LoggerForDifferentClassRule has 1 property:
> >     boolean allowDerivedLogger = false

By default, the rules do what is right and most people will never know these rules have properties. There is no complexity for most of users, yet for those that want finer control then every use case is covered. 

I would leave the extra properties in. The default configuration does the right thing, and every use case is covered for those that want something different... 

What I would not do is accept the original patch because it mixes the logic about parameter checking between two different rules. 

--
Hamlet D'Arcy


----- Original Message -----
> My thoughts match Chris'.
> 
> I think two rules for the logger is adding too much complexity. I
> still
> favor my original patch. This supports the two logger instantiation
> cases I have seen and classified as correct in recent years.
> 
> Sure there are more cases out there (injecting via Spring, per
> thread,
> new Logger, etc.) but this rule should stick to the common ones. If
> there are too many posibilities it will just create confusion. In
> case
> we missed a relevant case people will raise their hands.
> 
> Regards,
> René Scheibe
> 
> On 06/01/2011 02:13 AM, Chris Mair wrote:
> > Hamlet / René,
> > 
> > Thanks for working on this Hamlet. I'm sorry I did not respond
> > sooner. I see no value, for me anyway, in the
> > LoggerWithWrongModifiersRule supporting allowProtectedLogger or
> > allowNonStaticLogger properties. I will not enable those because I
> > do want to find the genuine bugs where a logger is incorrectly
> > defined as protected or as non-static. I guess I will just
> > configure exceptions for those (relatively rare) framework
> > superclasses where I define the LOG in the superclass for all
> > child classes.
> > 
> > Will you use those properties René? If they provide value, then
> > fine. But if we are adding complexity that no one will (or should)
> > use, then I don't want to do that.
> > 
> > Chris
> > -----Original Message-----
> > From: Hamlet DArcy [mailto:ham...@ca...]
> > Sent: Tuesday, May 31, 2011 4:55 AM
> > To: René Scheibe
> > Cc: Cod...@li...
> > Subject: Re: [Codenarc-user] LoggerWithWrongModifiersRule
> > supporting subclasses
> > 
> > I think the best solution is this because it covers every use case:
> > 
> > LoggerWithWrongModifiersRule has 2 properties:
> >     boolean allowProtectedLogger = false
> >     boolean allowNonStaticLogger = false
> > 
> > LoggerForDifferentClassRule has 1 property:
> >     boolean allowDerivedLogger = false
> > 
> > 
> > Then everything is covered for everyone with no overlap.
> > 
> > 
> > ----- Original Message -----
> >> It's not about the "protected" (this is just required).
> >>
> >> The interesting part is the non-static one. Because of it, a
> >> superclass uses the logger of the derived class based on
> >> "getLogger(this.class)".
> >>
> >> With such a logger it's for example possible to write every log
> >> message of a class (including its superclass) to a separate file
> >> because they are using the same logger.
> >>
> >> Regards,
> >> René Scheibe
> >>
> >> On 05/31/2011 09:55 AM, Hamlet DArcy wrote:
> >>> Personally, I would require loggers to be static, private, and
> >>> final.
> >>> If someone really needed a protected logger then i would still
> >>> make
> >>> it static and final, just protected.
> >>>
> >>> So I think we need both these settings.
> >>>
> >>>
> >>>
> >>> ----- Original Message -----
> >>>> That's a great question.
> >>>>
> >>>> I do use that pattern in framework superclasses:
> >>>> 	protected final LOG = Logger.getLogger(this.class)
> >>>>
> >>>> to provide a built-in logger for all subclasses. And so I have
> >>>> to
> >>>> configure exceptions for this rule for those superclasses.
> >>>>
> >>>> Is there any value in splitting up the "exception" configuration
> >>>> into two properties, if we do allow it?
> >>>>   * allowNonStaticLogger
> >>>>   * allowProtectedLogger
> >>>>
> >>>> Are there use cases we can envision where we (or anyone) would
> >>>> choose to allow non-static but not protected, or vice versa?
> >>>>
> >>>> Chris
> >>>> -----Original Message-----
> >>>> From: Hamlet D'Arcy [mailto:ham...@gm...]
> >>>> Sent: Monday, May 30, 2011 2:50 AM
> >>>> To: René Scheibe; Cod...@li...
> >>>> Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting
> >>>> subclasses
> >>>>
> >>>> Hi everyone,
> >>>>
> >>>> This email is about the idea about the
> >>>> LoggerWithWrongModifiersRule
> >>>> supporting subclasses.
> >>>>
> >>>> René filed an issue (and supplied a patch) describing the issue:
> >>>> https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&g
> >>>> roup_id=2
> >>>> 50145&atid=1126575#
> >>>>
> >>>> Basically, the question is how subclasses should be allowed to
> >>>> access a Logger in a parent class.
> >>>>
> >>>> Normally Loggers are private, static, and final. This patch
> >>>> allows
> >>>> you to accept this as valid code when you turn a certain
> >>>> property
> >>>> on:
> >>>>
> >>>>     protected final LOG = Logger.getLogger(this.class)
> >>>>
> >>>> I'd like to discuss this just a little more before committing
> >>>> the
> >>>> patch.
> >>>>
> >>>> With this change, it is acceptable to have a non-static logger.
> >>>> So
> >>>> each instance of a subclass makes and creates a logger. This
> >>>> does
> >>>> not seem right, and I would reject it in a code review as
> >>>> improper
> >>>> logging. I would accept this form though:
> >>>>
> >>>>     protected static final LOG = Logger.getLogger(MyClass.class)
> >>>>
> >>>> Do we really want to allow non-static Loggers?
> >>>>
> >>>> if so, I recommend two new properties instead of just one:
> >>>>   * allowNonStaticLogger
> >>>>   * allowProtectedLogger
> >>>>
> >>>> Then the user can control the behavior a little better.
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> Hamlet D'Arcy
> >>>> ham...@gm...
> >>>>
> >>>> -------------------------------------------------------------------
> >>>> ---------
> >>>> --
> >>>> vRanger cuts backup time in half-while increasing security.
> >>>> With the market-leading solution for virtual backup and
> >>>> recovery,
> >>>> you get blazing-fast, flexible, and affordable data protection.
> >>>> Download your free trial now.
> >>>> http://p.sf.net/sfu/quest-d2dcopy1
> >>>> _______________________________________________
> >>>> Codenarc-user mailing list
> >>>> Cod...@li...
> >>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>>>
> >>>>
> >>>> -------------------------------------------------------------------
> >>>> ----------- Simplify data backup and recovery for your virtual
> >>>> environment with vRanger.
> >>>> Installation's a snap, and flexible recovery options mean your
> >>>> data
> >>>> is safe, secure and there when you need it. Data protection
> >>>> magic?
> >>>> Nope - It's vRanger. Get your free trial download today.
> >>>> http://p.sf.net/sfu/quest-sfdev2dev
> >>>> _______________________________________________
> >>>> Codenarc-user mailing list
> >>>> Cod...@li...
> >>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>>>
> >>
> >>
> >> ----------------------------------------------------------------------
> >> -------- Simplify data backup and recovery for your virtual
> >> environment with vRanger.
> >> Installation's a snap, and flexible recovery options mean your
> >> data is
> >> safe, secure and there when you need it. Data protection magic?
> >> Nope - It's vRanger. Get your free trial download today.
> >> http://p.sf.net/sfu/quest-sfdev2dev
> >> _______________________________________________
> >> Codenarc-user mailing list
> >> Cod...@li...
> >> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>
> > 
> > ------------------------------------------------------------------------------
> > Simplify data backup and recovery for your virtual environment with
> > vRanger.
> > Installation's a snap, and flexible recovery options mean your data
> > is safe, secure and there when you need it. Data protection magic?
> > Nope - It's vRanger. Get your free trial download today.
> > http://p.sf.net/sfu/quest-sfdev2dev
> > _______________________________________________
> > Codenarc-user mailing list
> > Cod...@li...
> > https://lists.sourceforge.net/lists/listinfo/codenarc-user
> > 
> 
> 

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[R. S. <ren...@go...>]

My thoughts match Chris'.

I think two rules for the logger is adding too much complexity. I still
favor my original patch. This supports the two logger instantiation
cases I have seen and classified as correct in recent years.

Sure there are more cases out there (injecting via Spring, per thread,
new Logger, etc.) but this rule should stick to the common ones. If
there are too many posibilities it will just create confusion. In case
we missed a relevant case people will raise their hands.

Regards,
René Scheibe

On 06/01/2011 02:13 AM, Chris Mair wrote:
> Hamlet / René,
> 
> Thanks for working on this Hamlet. I'm sorry I did not respond sooner. I see no value, for me anyway, in the LoggerWithWrongModifiersRule supporting allowProtectedLogger or allowNonStaticLogger properties. I will not enable those because I do want to find the genuine bugs where a logger is incorrectly defined as protected or as non-static. I guess I will just configure exceptions for those (relatively rare) framework superclasses where I define the LOG in the superclass for all child classes.
> 
> Will you use those properties René? If they provide value, then fine. But if we are adding complexity that no one will (or should) use, then I don't want to do that.
> 
> Chris
> -----Original Message-----
> From: Hamlet DArcy [mailto:ham...@ca...] 
> Sent: Tuesday, May 31, 2011 4:55 AM
> To: René Scheibe
> Cc: Cod...@li...
> Subject: Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses
> 
> I think the best solution is this because it covers every use case: 
> 
> LoggerWithWrongModifiersRule has 2 properties: 
>     boolean allowProtectedLogger = false
>     boolean allowNonStaticLogger = false
> 
> LoggerForDifferentClassRule has 1 property: 
>     boolean allowDerivedLogger = false
> 
> 
> Then everything is covered for everyone with no overlap. 
> 
> 
> ----- Original Message -----
>> It's not about the "protected" (this is just required).
>>
>> The interesting part is the non-static one. Because of it, a 
>> superclass uses the logger of the derived class based on 
>> "getLogger(this.class)".
>>
>> With such a logger it's for example possible to write every log 
>> message of a class (including its superclass) to a separate file 
>> because they are using the same logger.
>>
>> Regards,
>> René Scheibe
>>
>> On 05/31/2011 09:55 AM, Hamlet DArcy wrote:
>>> Personally, I would require loggers to be static, private, and 
>>> final.
>>> If someone really needed a protected logger then i would still make 
>>> it static and final, just protected.
>>>
>>> So I think we need both these settings.
>>>
>>>
>>>
>>> ----- Original Message -----
>>>> That's a great question.
>>>>
>>>> I do use that pattern in framework superclasses:
>>>> 	protected final LOG = Logger.getLogger(this.class)
>>>>
>>>> to provide a built-in logger for all subclasses. And so I have to 
>>>> configure exceptions for this rule for those superclasses.
>>>>
>>>> Is there any value in splitting up the "exception" configuration 
>>>> into two properties, if we do allow it?
>>>>   * allowNonStaticLogger
>>>>   * allowProtectedLogger
>>>>
>>>> Are there use cases we can envision where we (or anyone) would 
>>>> choose to allow non-static but not protected, or vice versa?
>>>>
>>>> Chris
>>>> -----Original Message-----
>>>> From: Hamlet D'Arcy [mailto:ham...@gm...]
>>>> Sent: Monday, May 30, 2011 2:50 AM
>>>> To: René Scheibe; Cod...@li...
>>>> Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting 
>>>> subclasses
>>>>
>>>> Hi everyone,
>>>>
>>>> This email is about the idea about the LoggerWithWrongModifiersRule 
>>>> supporting subclasses.
>>>>
>>>> René filed an issue (and supplied a patch) describing the issue:
>>>> https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&g
>>>> roup_id=2
>>>> 50145&atid=1126575#
>>>>
>>>> Basically, the question is how subclasses should be allowed to 
>>>> access a Logger in a parent class.
>>>>
>>>> Normally Loggers are private, static, and final. This patch allows 
>>>> you to accept this as valid code when you turn a certain property 
>>>> on:
>>>>
>>>>     protected final LOG = Logger.getLogger(this.class)
>>>>
>>>> I'd like to discuss this just a little more before committing the 
>>>> patch.
>>>>
>>>> With this change, it is acceptable to have a non-static logger. So 
>>>> each instance of a subclass makes and creates a logger. This does 
>>>> not seem right, and I would reject it in a code review as improper 
>>>> logging. I would accept this form though:
>>>>
>>>>     protected static final LOG = Logger.getLogger(MyClass.class)
>>>>
>>>> Do we really want to allow non-static Loggers?
>>>>
>>>> if so, I recommend two new properties instead of just one:
>>>>   * allowNonStaticLogger
>>>>   * allowProtectedLogger
>>>>
>>>> Then the user can control the behavior a little better.
>>>>
>>>>
>>>>
>>>> --
>>>> Hamlet D'Arcy
>>>> ham...@gm...
>>>>
>>>> -------------------------------------------------------------------
>>>> ---------
>>>> --
>>>> vRanger cuts backup time in half-while increasing security.
>>>> With the market-leading solution for virtual backup and recovery, 
>>>> you get blazing-fast, flexible, and affordable data protection.
>>>> Download your free trial now.
>>>> http://p.sf.net/sfu/quest-d2dcopy1
>>>> _______________________________________________
>>>> Codenarc-user mailing list
>>>> Cod...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>>
>>>>
>>>> -------------------------------------------------------------------
>>>> ----------- Simplify data backup and recovery for your virtual 
>>>> environment with vRanger.
>>>> Installation's a snap, and flexible recovery options mean your data 
>>>> is safe, secure and there when you need it. Data protection magic?
>>>> Nope - It's vRanger. Get your free trial download today.
>>>> http://p.sf.net/sfu/quest-sfdev2dev
>>>> _______________________________________________
>>>> Codenarc-user mailing list
>>>> Cod...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>>
>>
>>
>> ----------------------------------------------------------------------
>> -------- Simplify data backup and recovery for your virtual 
>> environment with vRanger.
>> Installation's a snap, and flexible recovery options mean your data is 
>> safe, secure and there when you need it. Data protection magic?
>> Nope - It's vRanger. Get your free trial download today.
>> http://p.sf.net/sfu/quest-sfdev2dev
>> _______________________________________________
>> Codenarc-user mailing list
>> Cod...@li...
>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>
> 
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with vRanger. 
> Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Data protection magic?
> Nope - It's vRanger. Get your free trial download today. 
> http://p.sf.net/sfu/quest-sfdev2dev
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[Chris M. <chr...@ea...>]

Hamlet / René,

Thanks for working on this Hamlet. I'm sorry I did not respond sooner. I see no value, for me anyway, in the LoggerWithWrongModifiersRule supporting allowProtectedLogger or allowNonStaticLogger properties. I will not enable those because I do want to find the genuine bugs where a logger is incorrectly defined as protected or as non-static. I guess I will just configure exceptions for those (relatively rare) framework superclasses where I define the LOG in the superclass for all child classes.

Will you use those properties René? If they provide value, then fine. But if we are adding complexity that no one will (or should) use, then I don't want to do that.

Chris
-----Original Message-----
From: Hamlet DArcy [mailto:ham...@ca...] 
Sent: Tuesday, May 31, 2011 4:55 AM
To: René Scheibe
Cc: Cod...@li...
Subject: Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

I think the best solution is this because it covers every use case: 

LoggerWithWrongModifiersRule has 2 properties: 
    boolean allowProtectedLogger = false
    boolean allowNonStaticLogger = false

LoggerForDifferentClassRule has 1 property: 
    boolean allowDerivedLogger = false


Then everything is covered for everyone with no overlap. 


----- Original Message -----
> It's not about the "protected" (this is just required).
> 
> The interesting part is the non-static one. Because of it, a 
> superclass uses the logger of the derived class based on 
> "getLogger(this.class)".
> 
> With such a logger it's for example possible to write every log 
> message of a class (including its superclass) to a separate file 
> because they are using the same logger.
> 
> Regards,
> René Scheibe
> 
> On 05/31/2011 09:55 AM, Hamlet DArcy wrote:
> > Personally, I would require loggers to be static, private, and 
> > final.
> > If someone really needed a protected logger then i would still make 
> > it static and final, just protected.
> > 
> > So I think we need both these settings.
> > 
> > 
> > 
> > ----- Original Message -----
> >> That's a great question.
> >>
> >> I do use that pattern in framework superclasses:
> >> 	protected final LOG = Logger.getLogger(this.class)
> >>
> >> to provide a built-in logger for all subclasses. And so I have to 
> >> configure exceptions for this rule for those superclasses.
> >>
> >> Is there any value in splitting up the "exception" configuration 
> >> into two properties, if we do allow it?
> >>   * allowNonStaticLogger
> >>   * allowProtectedLogger
> >>
> >> Are there use cases we can envision where we (or anyone) would 
> >> choose to allow non-static but not protected, or vice versa?
> >>
> >> Chris
> >> -----Original Message-----
> >> From: Hamlet D'Arcy [mailto:ham...@gm...]
> >> Sent: Monday, May 30, 2011 2:50 AM
> >> To: René Scheibe; Cod...@li...
> >> Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting 
> >> subclasses
> >>
> >> Hi everyone,
> >>
> >> This email is about the idea about the LoggerWithWrongModifiersRule 
> >> supporting subclasses.
> >>
> >> René filed an issue (and supplied a patch) describing the issue:
> >> https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&g
> >> roup_id=2
> >> 50145&atid=1126575#
> >>
> >> Basically, the question is how subclasses should be allowed to 
> >> access a Logger in a parent class.
> >>
> >> Normally Loggers are private, static, and final. This patch allows 
> >> you to accept this as valid code when you turn a certain property 
> >> on:
> >>
> >>     protected final LOG = Logger.getLogger(this.class)
> >>
> >> I'd like to discuss this just a little more before committing the 
> >> patch.
> >>
> >> With this change, it is acceptable to have a non-static logger. So 
> >> each instance of a subclass makes and creates a logger. This does 
> >> not seem right, and I would reject it in a code review as improper 
> >> logging. I would accept this form though:
> >>
> >>     protected static final LOG = Logger.getLogger(MyClass.class)
> >>
> >> Do we really want to allow non-static Loggers?
> >>
> >> if so, I recommend two new properties instead of just one:
> >>   * allowNonStaticLogger
> >>   * allowProtectedLogger
> >>
> >> Then the user can control the behavior a little better.
> >>
> >>
> >>
> >> --
> >> Hamlet D'Arcy
> >> ham...@gm...
> >>
> >> -------------------------------------------------------------------
> >> ---------
> >> --
> >> vRanger cuts backup time in half-while increasing security.
> >> With the market-leading solution for virtual backup and recovery, 
> >> you get blazing-fast, flexible, and affordable data protection.
> >> Download your free trial now.
> >> http://p.sf.net/sfu/quest-d2dcopy1
> >> _______________________________________________
> >> Codenarc-user mailing list
> >> Cod...@li...
> >> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>
> >>
> >> -------------------------------------------------------------------
> >> ----------- Simplify data backup and recovery for your virtual 
> >> environment with vRanger.
> >> Installation's a snap, and flexible recovery options mean your data 
> >> is safe, secure and there when you need it. Data protection magic?
> >> Nope - It's vRanger. Get your free trial download today.
> >> http://p.sf.net/sfu/quest-sfdev2dev
> >> _______________________________________________
> >> Codenarc-user mailing list
> >> Cod...@li...
> >> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>
> 
> 
> ----------------------------------------------------------------------
> -------- Simplify data backup and recovery for your virtual 
> environment with vRanger.
> Installation's a snap, and flexible recovery options mean your data is 
> safe, secure and there when you need it. Data protection magic?
> Nope - It's vRanger. Get your free trial download today.
> http://p.sf.net/sfu/quest-sfdev2dev
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 

------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger. 
Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today. 
http://p.sf.net/sfu/quest-sfdev2dev
_______________________________________________
Codenarc-user mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-user

Re: [Codenarc-user] FieldNameRule: definition of what's a constant

[<chr...@we...>]

I agree. And though this has the potential to "break" some existing users (including me, by the way), I agree it is a "bug" and should be fixed.

I have opened an issue for that:
	https://sourceforge.net/tracker/?func=detail&aid=3309748&group_id=250145&atid=1126573

Chris
-----Original Message-----
From: René Scheibe [mailto:ren...@go...] 
Sent: Tuesday, May 31, 2011 3:30 AM
To: Hamlet DArcy
Cc: codenarc-user
Subject: Re: [Codenarc-user] FieldNameRule: definition of what's a constant

On 05/31/2011 09:23 AM, Hamlet DArcy wrote:
>> So I vote to change the finalRegex to the staticFinalRegex.
> 
> You want it like this, right? 
> 
>     String regex = DEFAULT_FIELD_NAME
>     String staticRegex
>     String finalRegex
>     String staticFinalRegex = DEFAULT_CONST_NAME

Yes. Exactly. :-)

> I agree. It's up to Chris if he thinks it is a backwards incompatible change or a bug to be fixed. 
> 
> 
> 
> 
> ----- Original Message -----
>> For me it's a bug, not only changed expectations. This results in
>> people
>> having incorrectly named variables.
>>
>> So I vote to change the finalRegex to the staticFinalRegex.
>>
>> Regards,
>> René Scheibe
>>
>> On 05/31/2011 07:25 AM, Hamlet DArcy wrote:
>>> IMO, I would also have a different "finalRegex" default property. I
>>> think we won't change the default because of backwards
>>> compatibility.
>>>  
>>>
>>> ----- Original Message -----
>>>> I was wondering why the "finalRegex" contains the
>>>> DEFAULT_CONST_NAME
>>>> pattern. I think only the "staticFinalRegex" should contain the
>>>> DEFAULT_CONST_NAME pattern, as only "static final" declares class
>>>> constants.
>>>>
>>>> Have a look at Sun's code conventions:
>>>> http://www.oracle.com/technetwork/java/codeconventions-135099.html#367
>>>> http://download.oracle.com/javase/tutorial/java/nutsandbolts/variables.html
>>>>
>>>> I was confused when violations popped up for things I am doing
>>>> since
>>>> years. :-)
>>>>
>>>> Regards,
>>>> René Scheibe
>>>>
>>>> ------------------------------------------------------------------------------
>>>> vRanger cuts backup time in half-while increasing security.
>>>> With the market-leading solution for virtual backup and recovery,
>>>> you get blazing-fast, flexible, and affordable data protection.
>>>> Download your free trial now.
>>>> http://p.sf.net/sfu/quest-d2dcopy1
>>>> _______________________________________________
>>>> Codenarc-user mailing list
>>>> Cod...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>>
>>
>>


------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger. 
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today. 
http://p.sf.net/sfu/quest-sfdev2dev
_______________________________________________
Codenarc-user mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-user

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[Hamlet D. <ham...@ca...>]

I think the best solution is this because it covers every use case: 

LoggerWithWrongModifiersRule has 2 properties: 
    boolean allowProtectedLogger = false
    boolean allowNonStaticLogger = false

LoggerForDifferentClassRule has 1 property: 
    boolean allowDerivedLogger = false


Then everything is covered for everyone with no overlap. 


----- Original Message -----
> It's not about the "protected" (this is just required).
> 
> The interesting part is the non-static one. Because of it, a
> superclass
> uses the logger of the derived class based on
> "getLogger(this.class)".
> 
> With such a logger it's for example possible to write every log
> message
> of a class (including its superclass) to a separate file because they
> are using the same logger.
> 
> Regards,
> René Scheibe
> 
> On 05/31/2011 09:55 AM, Hamlet DArcy wrote:
> > Personally, I would require loggers to be static, private, and
> > final.
> > If someone really needed a protected logger then i would still make
> > it static and final, just protected.
> > 
> > So I think we need both these settings.
> > 
> > 
> > 
> > ----- Original Message -----
> >> That's a great question.
> >>
> >> I do use that pattern in framework superclasses:
> >> 	protected final LOG = Logger.getLogger(this.class)
> >>
> >> to provide a built-in logger for all subclasses. And so I have to
> >> configure
> >> exceptions for this rule for those superclasses.
> >>
> >> Is there any value in splitting up the "exception" configuration
> >> into
> >> two
> >> properties, if we do allow it?
> >>   * allowNonStaticLogger
> >>   * allowProtectedLogger
> >>
> >> Are there use cases we can envision where we (or anyone) would
> >> choose
> >> to
> >> allow non-static but not protected, or vice versa?
> >>
> >> Chris
> >> -----Original Message-----
> >> From: Hamlet D'Arcy [mailto:ham...@gm...]
> >> Sent: Monday, May 30, 2011 2:50 AM
> >> To: René Scheibe; Cod...@li...
> >> Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting
> >> subclasses
> >>
> >> Hi everyone,
> >>
> >> This email is about the idea about the
> >> LoggerWithWrongModifiersRule
> >> supporting subclasses.
> >>
> >> René filed an issue (and supplied a patch) describing the issue:
> >> https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&group_id=2
> >> 50145&atid=1126575#
> >>
> >> Basically, the question is how subclasses should be allowed to
> >> access
> >> a
> >> Logger in a parent class.
> >>
> >> Normally Loggers are private, static, and final. This patch allows
> >> you to
> >> accept this as valid code when you turn a certain property on:
> >>
> >>     protected final LOG = Logger.getLogger(this.class)
> >>
> >> I'd like to discuss this just a little more before committing the
> >> patch.
> >>
> >> With this change, it is acceptable to have a non-static logger. So
> >> each
> >> instance of a subclass makes and creates a logger. This does not
> >> seem
> >> right,
> >> and I would reject it in a code review as improper logging. I
> >> would
> >> accept
> >> this form though:
> >>
> >>     protected static final LOG = Logger.getLogger(MyClass.class)
> >>
> >> Do we really want to allow non-static Loggers?
> >>
> >> if so, I recommend two new properties instead of just one:
> >>   * allowNonStaticLogger
> >>   * allowProtectedLogger
> >>
> >> Then the user can control the behavior a little better.
> >>
> >>
> >>
> >> --
> >> Hamlet D'Arcy
> >> ham...@gm...
> >>
> >> ----------------------------------------------------------------------------
> >> --
> >> vRanger cuts backup time in half-while increasing security.
> >> With the market-leading solution for virtual backup and recovery,
> >> you
> >> get
> >> blazing-fast, flexible, and affordable data protection.
> >> Download your free trial now.
> >> http://p.sf.net/sfu/quest-d2dcopy1
> >> _______________________________________________
> >> Codenarc-user mailing list
> >> Cod...@li...
> >> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>
> >>
> >> ------------------------------------------------------------------------------
> >> Simplify data backup and recovery for your virtual environment
> >> with
> >> vRanger.
> >> Installation's a snap, and flexible recovery options mean your
> >> data
> >> is safe,
> >> secure and there when you need it. Data protection magic?
> >> Nope - It's vRanger. Get your free trial download today.
> >> http://p.sf.net/sfu/quest-sfdev2dev
> >> _______________________________________________
> >> Codenarc-user mailing list
> >> Cod...@li...
> >> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>
> 
> 
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with
> vRanger.
> Installation's a snap, and flexible recovery options mean your data
> is safe,
> secure and there when you need it. Data protection magic?
> Nope - It's vRanger. Get your free trial download today.
> http://p.sf.net/sfu/quest-sfdev2dev
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[R. S. <ren...@go...>]

It's not about the "protected" (this is just required).

The interesting part is the non-static one. Because of it, a superclass
uses the logger of the derived class based on "getLogger(this.class)".

With such a logger it's for example possible to write every log message
of a class (including its superclass) to a separate file because they
are using the same logger.

Regards,
René Scheibe

On 05/31/2011 09:55 AM, Hamlet DArcy wrote:
> Personally, I would require loggers to be static, private, and final. 
> If someone really needed a protected logger then i would still make it static and final, just protected. 
> 
> So I think we need both these settings. 
> 
> 
> 
> ----- Original Message -----
>> That's a great question.
>>
>> I do use that pattern in framework superclasses:
>> 	protected final LOG = Logger.getLogger(this.class)
>>
>> to provide a built-in logger for all subclasses. And so I have to
>> configure
>> exceptions for this rule for those superclasses.
>>
>> Is there any value in splitting up the "exception" configuration into
>> two
>> properties, if we do allow it?
>>   * allowNonStaticLogger
>>   * allowProtectedLogger
>>
>> Are there use cases we can envision where we (or anyone) would choose
>> to
>> allow non-static but not protected, or vice versa?
>>
>> Chris
>> -----Original Message-----
>> From: Hamlet D'Arcy [mailto:ham...@gm...]
>> Sent: Monday, May 30, 2011 2:50 AM
>> To: René Scheibe; Cod...@li...
>> Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting
>> subclasses
>>
>> Hi everyone,
>>
>> This email is about the idea about the LoggerWithWrongModifiersRule
>> supporting subclasses.
>>
>> René filed an issue (and supplied a patch) describing the issue:
>> https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&group_id=2
>> 50145&atid=1126575#
>>
>> Basically, the question is how subclasses should be allowed to access
>> a
>> Logger in a parent class.
>>
>> Normally Loggers are private, static, and final. This patch allows
>> you to
>> accept this as valid code when you turn a certain property on:
>>
>>     protected final LOG = Logger.getLogger(this.class)
>>
>> I'd like to discuss this just a little more before committing the
>> patch.
>>
>> With this change, it is acceptable to have a non-static logger. So
>> each
>> instance of a subclass makes and creates a logger. This does not seem
>> right,
>> and I would reject it in a code review as improper logging. I would
>> accept
>> this form though:
>>
>>     protected static final LOG = Logger.getLogger(MyClass.class)
>>
>> Do we really want to allow non-static Loggers?
>>
>> if so, I recommend two new properties instead of just one:
>>   * allowNonStaticLogger
>>   * allowProtectedLogger
>>
>> Then the user can control the behavior a little better.
>>
>>
>>
>> --
>> Hamlet D'Arcy
>> ham...@gm...
>>
>> ----------------------------------------------------------------------------
>> --
>> vRanger cuts backup time in half-while increasing security.
>> With the market-leading solution for virtual backup and recovery, you
>> get
>> blazing-fast, flexible, and affordable data protection.
>> Download your free trial now.
>> http://p.sf.net/sfu/quest-d2dcopy1
>> _______________________________________________
>> Codenarc-user mailing list
>> Cod...@li...
>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>
>>
>> ------------------------------------------------------------------------------
>> Simplify data backup and recovery for your virtual environment with
>> vRanger.
>> Installation's a snap, and flexible recovery options mean your data
>> is safe,
>> secure and there when you need it. Data protection magic?
>> Nope - It's vRanger. Get your free trial download today.
>> http://p.sf.net/sfu/quest-sfdev2dev
>> _______________________________________________
>> Codenarc-user mailing list
>> Cod...@li...
>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[Hamlet D. <ham...@ca...>]

Personally, I would require loggers to be static, private, and final. 
If someone really needed a protected logger then i would still make it static and final, just protected. 

So I think we need both these settings. 



----- Original Message -----
> That's a great question.
> 
> I do use that pattern in framework superclasses:
> 	protected final LOG = Logger.getLogger(this.class)
> 
> to provide a built-in logger for all subclasses. And so I have to
> configure
> exceptions for this rule for those superclasses.
> 
> Is there any value in splitting up the "exception" configuration into
> two
> properties, if we do allow it?
>   * allowNonStaticLogger
>   * allowProtectedLogger
> 
> Are there use cases we can envision where we (or anyone) would choose
> to
> allow non-static but not protected, or vice versa?
> 
> Chris
> -----Original Message-----
> From: Hamlet D'Arcy [mailto:ham...@gm...]
> Sent: Monday, May 30, 2011 2:50 AM
> To: René Scheibe; Cod...@li...
> Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting
> subclasses
> 
> Hi everyone,
> 
> This email is about the idea about the LoggerWithWrongModifiersRule
> supporting subclasses.
> 
> René filed an issue (and supplied a patch) describing the issue:
> https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&group_id=2
> 50145&atid=1126575#
> 
> Basically, the question is how subclasses should be allowed to access
> a
> Logger in a parent class.
> 
> Normally Loggers are private, static, and final. This patch allows
> you to
> accept this as valid code when you turn a certain property on:
> 
>     protected final LOG = Logger.getLogger(this.class)
> 
> I'd like to discuss this just a little more before committing the
> patch.
> 
> With this change, it is acceptable to have a non-static logger. So
> each
> instance of a subclass makes and creates a logger. This does not seem
> right,
> and I would reject it in a code review as improper logging. I would
> accept
> this form though:
> 
>     protected static final LOG = Logger.getLogger(MyClass.class)
> 
> Do we really want to allow non-static Loggers?
> 
> if so, I recommend two new properties instead of just one:
>   * allowNonStaticLogger
>   * allowProtectedLogger
> 
> Then the user can control the behavior a little better.
> 
> 
> 
> --
> Hamlet D'Arcy
> ham...@gm...
> 
> ----------------------------------------------------------------------------
> --
> vRanger cuts backup time in half-while increasing security.
> With the market-leading solution for virtual backup and recovery, you
> get
> blazing-fast, flexible, and affordable data protection.
> Download your free trial now.
> http://p.sf.net/sfu/quest-d2dcopy1
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 
> 
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with
> vRanger.
> Installation's a snap, and flexible recovery options mean your data
> is safe,
> secure and there when you need it. Data protection magic?
> Nope - It's vRanger. Get your free trial download today.
> http://p.sf.net/sfu/quest-sfdev2dev
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 

Re: [Codenarc-user] FieldNameRule: definition of what's a constant

[R. S. <ren...@go...>]

On 05/31/2011 09:23 AM, Hamlet DArcy wrote:
>> So I vote to change the finalRegex to the staticFinalRegex.
> 
> You want it like this, right? 
> 
>     String regex = DEFAULT_FIELD_NAME
>     String staticRegex
>     String finalRegex
>     String staticFinalRegex = DEFAULT_CONST_NAME

Yes. Exactly. :-)

> I agree. It's up to Chris if he thinks it is a backwards incompatible change or a bug to be fixed. 
> 
> 
> 
> 
> ----- Original Message -----
>> For me it's a bug, not only changed expectations. This results in
>> people
>> having incorrectly named variables.
>>
>> So I vote to change the finalRegex to the staticFinalRegex.
>>
>> Regards,
>> René Scheibe
>>
>> On 05/31/2011 07:25 AM, Hamlet DArcy wrote:
>>> IMO, I would also have a different "finalRegex" default property. I
>>> think we won't change the default because of backwards
>>> compatibility.
>>>  
>>>
>>> ----- Original Message -----
>>>> I was wondering why the "finalRegex" contains the
>>>> DEFAULT_CONST_NAME
>>>> pattern. I think only the "staticFinalRegex" should contain the
>>>> DEFAULT_CONST_NAME pattern, as only "static final" declares class
>>>> constants.
>>>>
>>>> Have a look at Sun's code conventions:
>>>> http://www.oracle.com/technetwork/java/codeconventions-135099.html#367
>>>> http://download.oracle.com/javase/tutorial/java/nutsandbolts/variables.html
>>>>
>>>> I was confused when violations popped up for things I am doing
>>>> since
>>>> years. :-)
>>>>
>>>> Regards,
>>>> René Scheibe
>>>>
>>>> ------------------------------------------------------------------------------
>>>> vRanger cuts backup time in half-while increasing security.
>>>> With the market-leading solution for virtual backup and recovery,
>>>> you get blazing-fast, flexible, and affordable data protection.
>>>> Download your free trial now.
>>>> http://p.sf.net/sfu/quest-d2dcopy1
>>>> _______________________________________________
>>>> Codenarc-user mailing list
>>>> Cod...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>>>
>>
>>

Re: [Codenarc-user] FieldNameRule: definition of what's a constant

[Hamlet D. <ham...@ca...>]

> So I vote to change the finalRegex to the staticFinalRegex.

You want it like this, right? 

    String regex = DEFAULT_FIELD_NAME
    String staticRegex
    String finalRegex
    String staticFinalRegex = DEFAULT_CONST_NAME

I agree. It's up to Chris if he thinks it is a backwards incompatible change or a bug to be fixed. 




----- Original Message -----
> For me it's a bug, not only changed expectations. This results in
> people
> having incorrectly named variables.
> 
> So I vote to change the finalRegex to the staticFinalRegex.
> 
> Regards,
> René Scheibe
> 
> On 05/31/2011 07:25 AM, Hamlet DArcy wrote:
> > IMO, I would also have a different "finalRegex" default property. I
> > think we won't change the default because of backwards
> > compatibility.
> >  
> > 
> > ----- Original Message -----
> >> I was wondering why the "finalRegex" contains the
> >> DEFAULT_CONST_NAME
> >> pattern. I think only the "staticFinalRegex" should contain the
> >> DEFAULT_CONST_NAME pattern, as only "static final" declares class
> >> constants.
> >>
> >> Have a look at Sun's code conventions:
> >> http://www.oracle.com/technetwork/java/codeconventions-135099.html#367
> >> http://download.oracle.com/javase/tutorial/java/nutsandbolts/variables.html
> >>
> >> I was confused when violations popped up for things I am doing
> >> since
> >> years. :-)
> >>
> >> Regards,
> >> René Scheibe
> >>
> >> ------------------------------------------------------------------------------
> >> vRanger cuts backup time in half-while increasing security.
> >> With the market-leading solution for virtual backup and recovery,
> >> you get blazing-fast, flexible, and affordable data protection.
> >> Download your free trial now.
> >> http://p.sf.net/sfu/quest-d2dcopy1
> >> _______________________________________________
> >> Codenarc-user mailing list
> >> Cod...@li...
> >> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> >>
> 
> 

Re: [Codenarc-user] FieldNameRule: definition of what's a constant

[R. S. <ren...@go...>]

For me it's a bug, not only changed expectations. This results in people
having incorrectly named variables.

So I vote to change the finalRegex to the staticFinalRegex.

Regards,
René Scheibe

On 05/31/2011 07:25 AM, Hamlet DArcy wrote:
> IMO, I would also have a different "finalRegex" default property. I think we won't change the default because of backwards compatibility.
>  
> 
> ----- Original Message -----
>> I was wondering why the "finalRegex" contains the DEFAULT_CONST_NAME
>> pattern. I think only the "staticFinalRegex" should contain the
>> DEFAULT_CONST_NAME pattern, as only "static final" declares class
>> constants.
>>
>> Have a look at Sun's code conventions:
>> http://www.oracle.com/technetwork/java/codeconventions-135099.html#367
>> http://download.oracle.com/javase/tutorial/java/nutsandbolts/variables.html
>>
>> I was confused when violations popped up for things I am doing since
>> years. :-)
>>
>> Regards,
>> René Scheibe
>>
>> ------------------------------------------------------------------------------
>> vRanger cuts backup time in half-while increasing security.
>> With the market-leading solution for virtual backup and recovery,
>> you get blazing-fast, flexible, and affordable data protection.
>> Download your free trial now.
>> http://p.sf.net/sfu/quest-d2dcopy1
>> _______________________________________________
>> Codenarc-user mailing list
>> Cod...@li...
>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>

Re: [Codenarc-user] FieldNameRule: definition of what's a constant

[Hamlet D. <ham...@ca...>]

IMO, I would also have a different "finalRegex" default property. I think we won't change the default because of backwards compatibility.
 

----- Original Message -----
> I was wondering why the "finalRegex" contains the DEFAULT_CONST_NAME
> pattern. I think only the "staticFinalRegex" should contain the
> DEFAULT_CONST_NAME pattern, as only "static final" declares class
> constants.
> 
> Have a look at Sun's code conventions:
> http://www.oracle.com/technetwork/java/codeconventions-135099.html#367
> http://download.oracle.com/javase/tutorial/java/nutsandbolts/variables.html
> 
> I was confused when violations popped up for things I am doing since
> years. :-)
> 
> Regards,
> René Scheibe
> 
> ------------------------------------------------------------------------------
> vRanger cuts backup time in half-while increasing security.
> With the market-leading solution for virtual backup and recovery,
> you get blazing-fast, flexible, and affordable data protection.
> Download your free trial now.
> http://p.sf.net/sfu/quest-d2dcopy1
> _______________________________________________
> Codenarc-user mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-user
> 

Re: [Codenarc-user] ExplicitCallToEqualsMethod and implementing equals method

[Chris M. <chr...@ea...>]

I agree. Can you please open a bug tracker issue for that? I think we can
and should ignore calls within an equals() method definition.

Chris
-----Original Message-----
From: René Scheibe [mailto:ren...@go...] 
Sent: Monday, May 30, 2011 3:33 PM
To: codenarc-user
Subject: [Codenarc-user] ExplicitCallToEqualsMethod and implementing equals
method

How is it possible to implement an equals method with the help of the
superclass' equals method?

@Override
boolean equals(Object that) {
    ...
    super.equals(that)
}

Shouldn't the ExplicitCallToEqualsMethod rule allow this?

Regards,
René Scheibe

----------------------------------------------------------------------------
--
vRanger cuts backup time in half-while increasing security.
With the market-leading solution for virtual backup and recovery, you get
blazing-fast, flexible, and affordable data protection.
Download your free trial now. 
http://p.sf.net/sfu/quest-d2dcopy1
_______________________________________________
Codenarc-user mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-user

Re: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

[Chris M. <chr...@ea...>]

That's a great question.

I do use that pattern in framework superclasses:
	protected final LOG = Logger.getLogger(this.class)

to provide a built-in logger for all subclasses. And so I have to configure
exceptions for this rule for those superclasses.

Is there any value in splitting up the "exception" configuration into two
properties, if we do allow it?
  * allowNonStaticLogger
  * allowProtectedLogger

Are there use cases we can envision where we (or anyone) would choose to
allow non-static but not protected, or vice versa?

Chris
-----Original Message-----
From: Hamlet D'Arcy [mailto:ham...@gm...] 
Sent: Monday, May 30, 2011 2:50 AM
To: René Scheibe; Cod...@li...
Subject: [Codenarc-user] LoggerWithWrongModifiersRule supporting subclasses

Hi everyone,

This email is about the idea about the LoggerWithWrongModifiersRule
supporting subclasses.

René filed an issue (and supplied a patch) describing the issue:
https://sourceforge.net/tracker/index.php?func=detail&aid=3308930&group_id=2
50145&atid=1126575#

Basically, the question is how subclasses should be allowed to access a
Logger in a parent class.

Normally Loggers are private, static, and final. This patch allows you to
accept this as valid code when you turn a certain property on:

    protected final LOG = Logger.getLogger(this.class)

I'd like to discuss this just a little more before committing the patch.

With this change, it is acceptable to have a non-static logger. So each
instance of a subclass makes and creates a logger. This does not seem right,
and I would reject it in a code review as improper logging. I would accept
this form though:

    protected static final LOG = Logger.getLogger(MyClass.class)

Do we really want to allow non-static Loggers?

if so, I recommend two new properties instead of just one:
  * allowNonStaticLogger
  * allowProtectedLogger

Then the user can control the behavior a little better.



--
Hamlet D'Arcy
ham...@gm...

----------------------------------------------------------------------------
--
vRanger cuts backup time in half-while increasing security.
With the market-leading solution for virtual backup and recovery, you get
blazing-fast, flexible, and affordable data protection.
Download your free trial now. 
http://p.sf.net/sfu/quest-d2dcopy1
_______________________________________________
Codenarc-user mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-user

[Codenarc-user] FieldNameRule: definition of what's a constant

[R. S. <ren...@go...>]

I was wondering why the "finalRegex" contains the DEFAULT_CONST_NAME
pattern. I think only the "staticFinalRegex" should contain the
DEFAULT_CONST_NAME pattern, as only "static final" declares class constants.

Have a look at Sun's code conventions:
http://www.oracle.com/technetwork/java/codeconventions-135099.html#367
http://download.oracle.com/javase/tutorial/java/nutsandbolts/variables.html

I was confused when violations popped up for things I am doing since
years. :-)

Regards,
René Scheibe