CodeNarc Files

CodeNarc
-------------------------------------------------------------------------------
http://www.codenarc.org

Changes in version 0.13 (March 2011)
-------------------------------------------
NEW FEATURES
- New and improved syntax for defining custom rulesets. Just list the rule names you want. No need to specify
  ruleset filename or rule class. No more tweaking your custom rulesets with every CodeNarc release when we
  add new rules to the "basic" ruleset or one of the others. Feature #3193684.
- Better support for command line runners. You can now specify the report type "console" as a command line parameter,
  and the CodeNarc text report is output to the console instead of a file. Feature #3162487
- All rules now provide a nicely formatted message. This makes console tools and the CodeNarc Web Console much more
  user friendly. Feature #3162847
- Greatly improved styling of the HTML report. Feature #3192593
- Improved error messages when CodeNarc is mis-configured and configuration file does not compile. Feature #3193468
- Improved println rule. If a class defines a println method, then it is OK to call this method because it won't dispatch to System.out.println. Feature #3194121

NEW RULES
  Dead/Unnecessary Code
- EmptyInstanceInitializer rule (basic) - Feature #3163464 - An empty instance initializer. It is safe to remove it.
- EmptyStaticInitializer rule (basic) - Feature #3161685 - An empty static initializer was found. It is safe to remove it.
- EmptyMethod rule (basic) - Feature #3163806 - A method was found without an implementation. If the method is overriding or implementing a parent method, then mark it with the @Override annotation.
- UnnecessaryCallToSubstring rule (unnecessary) - Feature #3164688 - Calling String.substring(0) always returns the original string. This code is meaningless.
- UnnecessaryModOne rule (unnecessary) - Feature #3164684 - Any expression mod 1 (exp % 1) is guaranteed to always return zero. This code is probably an error, and should be either (exp & 1) or (exp % 2).
- UnnecessarySelfAssignment rule (unnecessary) - Feature #3164674 - Method contains a pointless self-assignment to a variable or property.
- UnnecessaryTransientModifier rule (unnecessary) - Feature #3164672 - The field is marked as transient, but the class isn't Serializable, so marking it as transient has no effect.
- UnnecessaryFail rule (junit) - Feature #3105925 - In a unit test, catching an exception and immediately calling Assert.fail() is pointless and hides the stack trace. It is better to rethrow the exception or not catch the exception at all.
- UnnecessaryPublicModifier rule (unnecessary) - Feature #3166320 - The 'public' modifier is not required on methods or classes.
- UnnecessaryDefInMethodDeclaration rule (unnecessary) - Feature #3165469 - If a method has a visibility modifier, then the def keyword is unneeded. For instance 'def private method() {}' is redundant and can be simplified to 'private method() {}'.
- UnnecessarySemicolon rule (unnecessary) - Feature #3176207 - Semicolons as line terminators are not required in Groovy: remove them. Do not use a semicolon as a replacement for empty braces on for and while loops; this is a confusing practice.
- UnnecessaryGString rule (unnecessary) - Feature #3183521 - String objects should be created with single quotes, and GString objects created with double quotes. Creating normal String objects with double quotes is confusing to readers.

  Bugs/Bad Practices
- AssignmentInConditional rule (basic) - An assignment operator (=) was used in a conditional test. This is usually a typo, and the comparison operator (==) was intended.
- SerializableClassMustDefineSerialVersionUID rule (basic) - Feature #3161076 - Classes that implement Serializable should define a serialVersionUID.
- ConsecutiveStringConcatenation rule (basic) - Feature #3163826 - Catches concatenation of two string literals on the same line. These can safely by joined.
- ConsecutiveLiteralAppends rule (basic) - Feature #3161779 - Violations occur when method calls to append(Object) are chained together with literals as parameters. The chained calls can be joined into one invocation.
- AddEmptyString rule (basic) - Feature #3161763 - Finds empty string literals which are being added. This is an inefficient way to convert any type to a String.
- BrokenOddnessCheck rule (basic) - Feature #3164687 - The code uses x % 2 == 1 to check to see if a value is odd, but this won't work for negative numbers (e.g., (-5) % 2 == -1). If this code is intending to check for oddness, consider using x & 1 == 1, or x % 2 != 0.
- ExceptionExtendsError rule (exceptions) - Feature #3161749 - Errors are system exceptions. Do not extend them.
- MissingNewInThrowStatementRule (exceptions) - Feature #3166115 - Improved existing rule to catch throwing a class literal, which always causes a RuntimeException.
- UseAssertTrueInsteadOfAssertEquals (junit) - Feature #3172959 - This rule was expanded to handle assert statements as well as JUnit style assertions.
- GroovyLangImmutable rule (basic) - Feature #3175158 - The groovy.lang.Immutable annotation has been deprecated and replaced by groovy.transform.Immutable. Do not use the Immutable in groovy.lang.
- IntegerGetInteger rule (basic) - Feature #3174771 - This rule catches usages of java.lang.Integer.getInteger(String, ...) which reads an Integer from the System properties. It is often mistakenly used to attempt to read user input or parse a String into an Integer. It is a poor piece of API to use; replace it with System.properties['prop'].
- BooleanGetBoolean rule (basic) - Feature #3174770 - This rule catches usages of java.lang.Boolean.getBoolean(String) which reads a boolean from the System properties. It is often mistakenly used to attempt to read user input or parse a String into a boolean. It is a poor piece of API to use; replace it with System.properties['prop̈́'].
- ChainedTest rule (junit) - Feature #3175647 - A test methodency that invokes another test method is a chained test; the methods are dependent on one another. Tests should be isolated, and not be dependent on one another.
- CoupledTestCase rule (junit) - Feature #3175645 - This rule finds test cases that are coupled to other test cases, either by invoking static methods on another test case or by creating instances of another test case. If you require shared logic in test cases then extract that logic to a new class where it can properly be reused.

  Concurrency Problems
- BusyWait rule (concurrency) - Feature #3170271 - Busy waiting (forcing a Thread.sleep() while waiting on a condition) should be avoided. Prefer using the gate and barrier objects in the java.util.concurrent package.
- DoubleCheckedLocking rule (concurrency) - Feature #3170263 - This rule detects double checked locking, where a 'lock hint' is tested for null before initializing an object within a synchronized block. Double checked locking does not guarantee correctness and is an anti-pattern.
- InconsistentPropertyLocking rule (concurrency) - Feature #3164700 - Class contains similarly-named get and set methods where one method of the pair is marked either @WithReadLock or @WithWriteLock and the other is not locked at all.
- InconsistentPropertySynchronization rule (concurrency) - Feature #3164699 - Class contains similarly-named get and set methods where the set method is synchronized and the get method is not, or the get method is synchronized and the set method is not.
- StaticCalendarField rule (concurrency) - Feature #3164702 - Calendar objects should not be used as static fields. Calendars are inherently unsafe for multithreaded use. Sharing a single instance across thread boundaries without proper synchronization will result in erratic behavior of the application.
- StaticDateFormatField rule (concurrency) - DateFormat objects should not be used as static fields. DateFormat are inherently unsafe for multithreaded use. Sharing a single instance across thread boundaries without proper synchronization will result in erratic behavior of the application.
- StaticMatcherField rule (concurrency) - Feature #3170276 - Matcher objects should not be used as static fields. Calendars are inherently unsafe for multithreaded use. Sharing a single instance across thread boundaries without proper synchronization will result in erratic behavior of the application.
- SynchronizedOnBoxedPrimitive rule (concurrency) - Feature #3164708 - The code synchronizes on a boxed primitive constant, such as an Integer. Since Integer objects can be cached and shared, this code could be synchronizing on the same object as other, unrelated code, leading to unresponsiveness and possible deadlock
- SynchronizedOnReentrantLock rule (concurrency) - Feature #3170262 - Synchronizing on a ReentrantLock field is almost never the intended usage. A ReentrantLock should be obtained using the lock() method and released in a finally block using the unlock() method.
- SynchronizedOnString rule (concurrency) - Feature #3164707 - Synchronization on a String field can lead to deadlock because Strings are interned by the JVM and can be shared.
- SynchronizedReadObjectMethod rule (concurrency) - Feature #3164704 - Catches Serializable classes that define a synchronized readObject method. By definition, an object created by deserialization is only reachable by one thread, and thus there is no need for readObject() to be synchronized. If the readObject() method itself is causing the object to become visible to another thread, that is an example of very dubious coding style.
- ThreadGroup rule (concurrency) - Feature #3161692 - Avoid using ThreadGroup; although it is intended to be used in a threaded environment it contains methods that are not thread safe.
- VolatileArrayField rule (concurrency) - Feature #3170265 - Volatile array fields are unsafe because the contents of the array are not treated as volatile. Changing the entire array reference is visible to other threads, but changing an array element is not.
- WaitOutsideOfWhileLoop rule (concurrency) - Feature #3127703 - Calls to Object.wait() must be within a while loop. This ensures that the awaited condition has not already been satisfied by another thread before the wait() is invoked. It also ensures that the proper thread was resumed and guards against incorrect notification.

BUG FIXES
- Fix Bug #3162499 UnnecessaryObjectReferences: This rule was not ignoring 'this' references, leading to false positives. 
- Fix Bug #3164150 UnnecessaryReturnKeyword: Rule is no longer triggered if the return is for a closure.
- Fix Bug #3165139 LoggingSwallowsStacktrace: No longer reports multiple logger errors.
- Fix bug #3166311: Error on numeric/boolean property values if they contain leading or trailing whitespace in "codenarc.properties" or in XML ruleset file.
- Fix bug #3170295: @SuppressWarnings does not work with GMetrics-based rules (CC and ABC).
- Fix bug #3190483: The UnusedPrivateField rule now ignores fields named serialVersionUID. The rule has a property called ignoreFieldNames that can be used to add other ignores as well.
- Fix bug #3162975: The UnusedPrivate* rules now correctly read anonymous classes.
- Fix bug #3155974: The UnusedPrivate* rules now correctly read inner classes.
- Fix bug #3183443: Restricted the ConfusingTernary rule to reduce false positives. The rule looks for inverted conditionals in ternary statements. It did not account for Groovy Truth, and suggested 'x != null' be inverted to 'x', but that is not the inverse. So the rule was relaxed. 
- Fix bug #3195027: The violation message for "Replace Getter with Property Access" suggests converting ".getURLs()" to ".uRLs". This has been corrected to ".URLs". 
- Fix bug #3196019: Compiling files with unresolved Grapes dependencies no longer causes an error.

POTENTIAL BREAKING CHANGES
- addViolation(ASTNode) is deprecated. Instead use addViolation(ASTNode, String) when adding violations to the output.

THANKS
 - Many thanks to Mathilde Lemée, Guillaume Laforge, and Tim Yates for helping us redesign our HTML report.
 - Big thanks to Evgeny Goldin and Cédric Champeau for reporting bugs which helps us improve the product so much faster.