PMD Coding practices - Advance rules

Advance rules - provided by PMD

Controversial Java rules

UnnecessaryConstructor

This rule detects when a constructor is not necessary; i.e., when there is only one constructor,its public, has an empty body, and takes no arguments.

NullAssignment

Assigning a "null" to a variable (outside of its declaration) is usually bad form.

Sometimes, this typeof assignment is an indication that the programmer doesn't completely understand what is going on in the code.

Note : This sort of assignment may used in some cases to dereference objects and encourage garbage collection.

OnlyOneReturn

A method should have only one exit point, and that should be the last statement in the method.

AssignmentInOperand

Avoid assignments in operands; this can make code more complicated and harder to read.

AtLeastOneConstructor

Each class should declare at least one constructor.

DontImportSun

Avoid importing anything from the 'sun.*' packages. These packages are not portable and are likely to change.

SuspiciousOctalEscape

A suspicious octal escape sequence was found inside a String literal.

The Java language specification says an octal escape sequence inside a literal String shall consist of a backslash followed by :

OctalDigit | OctalDigit OctalDigit | ZeroToThree OctalDigit OctalDigit

Any octal escape sequence followed by non-octal digits can be confusing,e.g. "\038" is interpreted as the octal escape sequence "\03" followed bythe literal character "8".

CallSuperInConstructor

It is a good practice to call super() in a constructor.

If super() is not called but another constructor (such as an overloaded constructor) is called, this rule will not report it.

UnnecessaryParentheses

Sometimes expressions are wrapped in unnecessary parentheses, making them look like function calls.

DefaultPackage

Use explicit scoping instead of the default package private level.

BooleanInversion

Use bitwise inversion to invert boolean values - it's the fastest way to do this.

DataflowAnomalyAnalysis

The dataflow analysis tracks local definitions, undefinitions and references to variables on different paths on the data flow.

From those informations there can be found various problems.

 1. UR - Anomaly : There is a reference to a variable that was not defined before. This is a bug and leads to an error.

 2. DU - Anomaly : A recently defined variable is undefined. These anomalies may appear in normal source text.

 3. DD - Anomaly : A recently defined variable is redefined. This is ominous but don't have to be a bug.

AvoidFinalLocalVariable

Avoid using final local variables, turn them into fields.

AvoidUsingShortType

Java uses the 'short' type to reduce memory usage, not to optimize calculation.

In fact, the JVM does not have any arithmetic capabilities for the short type : The JVM must convert the short into an int, do the proper calculation and convert the int back to a short.

Thus any storage gains found through use of the 'short' type may be offset by adverse impacts on performance.

AvoidUsingVolatile

Use of the keyword 'volatile' is generally used to fine tune a Java application, and therefore, requires a good expertise of the Java Memory Model. Moreover, its range of action is somewhat misknown.

Therefore, the volatile keyword should not be used for maintenance purpose and portability.

AvoidUsingNativeCode

Unnecessary reliance on Java Native Interface (JNI) calls directly reduces application portability and increases the maintenance burden.

AvoidAccessibilityAlteration

Methods such as getDeclaredConstructors(), getDeclaredConstructor(Class[]) and setAccessible(), as the interface Privileged Action, allows for the runtime alteration of variable, class, or method visibility, even if they are private.

This violates the principle of encapsulation.

DoNotCallGarbageCollectionExplicitly

Calls to System.gc(), Runtime.getRuntime().gc(), and System.runFinalization() are not advised.

Code should have the same behavior whether the garbage collection is disabled using the option -Xdisableexplicitgc or not.

Moreover, "modern" JVMs do a very good job handling garbage collections.

If memory usage issues unrelated to memory leaks develop within an application, it should be dealt with JVM options rather than within the code itself.

OneDeclarationPerLine

Java allows the use of several variables declaration of the same type on one line. However, it can lead to quite messy code.

This rule looks for several declarations on the same line.

AvoidPrefixingMethodParameters

Prefixing parameters by 'in' or 'out' pollutes the name of the parameters and reduces code readability.

To indicate whether or not a parameter will be modify in a method, its better to document method behavior with Javadoc.

AvoidLiteralsInIfCondition

Avoid using hard-coded literals in conditional statements.

By declaring them as static variables or private members with descriptive names maintainability is enhanced.

UseObjectForClearerAPI

When you write a public method, you should be thinking in terms of an API.

If your method is public, it means other class will use it, therefore, you want (or need) to offer a comprehensive and evolutive API.

If you pass a lot of information as a simple series of Strings, you may think of using an Object to represent all those information.

You'll get a simplier API [ such as doWork (Workload workload), rather than a tedious series of Strings ] and more importantly, if you need at some point to pass extra data, you'll be able to do so by simply modifying or extending Workload without any modification to your API.

UseConcurrentHashMap

Since Java5 brought a new implementation of the Map designed for multi-threaded access, you can perform efficient map reads without blocking other threads.

Clone Implementation

ProperCloneImplementation

Object clone() should be implemented with super.clone().

CloneThrowsCloneNotSupportedException

The method clone() should throw a CloneNotSupportedException.

CloneMethodMustImplementCloneable

The method clone() should only be implemented if the class implements the Cloneable interface with the exception of a final method that only throws CloneNotSupportedException.

Finalizer

EmptyFinalizer

Empty finalize methods serve no purpose and should be removed.

FinalizeOnlyCallsSuperFinalize

If the finalize() is implemented, it should do something besides just calling super.finalize().

FinalizeOverloaded

Methods named finalize() should not have parameters.

It is confusing and most likely an attempt to overload Object.finalize(). It will not be called by the VM.

FinalizeDoesNotCallSuperFinalize

If the finalize() is implemented, its last action should be to call super.finalize.

FinalizeShouldBeProtected

When overriding the finalize(), the new method should be set as protected.

If made public, other classes may invoke it at inappropriate times.

AvoidCallingFinalize

The method Object.finalize() is called by the garbage collector on an object when garbage collection determines that there are no more references to the object.

It should not be invoked by application logic.

JavaBeans

BeanMembersShouldSerialize

If a class is a bean, or is referenced by a bean directly or indirectly it needs to be serializable.

Member variables need to be marked as transient, static, or have accessor methods in the class.

Marking variables as transient is the safest and easiest modification.

Accessor methods should follow the Java naming conventions, i.e. for a variable named foo, getFoo() and setFoo() accessor methods should be provided.

MissingSerialVersionUID

Serializable classes should provide a serialVersionUID field.

Type Resolution

LooseCoupling

Avoid using implementation types (i.e., HashSet); use the interface (i.e, Set) instead

CloneMethodMustImplementCloneable

The method clone() should only be implemented if the class implements the Cloneable interface with the exception of a final method that only throws CloneNotSupportedException.

This version uses PMD's type resolution facilities, and can detect if the class implements or extends a Cloneable class.

UnusedImports

Avoid unused import statements. This rule will find unused on demand imports, i.e. import com.foo.*.

SignatureDeclareThrowsException

It is unclear which exceptions that can be thrown from the methods.

It might be difficult to document and understand the vague interfaces.

Use either a class derived from RuntimeException or a checked exception.JUnit classes are excluded.

Jakarta Commons logging

UseCorrectExceptionLogging

To make sure the full stacktrace is printed out, use the logging statement with two arguments : a String and a Throwable.

ProperLogger

A logger should normally be defined private static final and be associated with the correct class.

Private final Log log; is also allowed for rare cases where loggers need to be passed around, with the restriction that the logger needs to be passed into the constructor.

GuardDebugLogging

When log messages are composed by concatenating strings, the whole section should be guarded by a isDebugEnabled() check to avoid performance and memory issues.

Security Code Guidelines

MethodReturnsInternalArray

Exposing internal arrays to the caller violates object encapsulation since elements can be removed or replaced outside of the object that owns it.

It is safer to return a copy of the array.

ArrayIsStoredDirectly

Constructors and methods receiving arrays should clone objects and store the copy.

This prevents future changes from the user from affecting the original array.