Controversial Rules

The Controversial Ruleset contains rules that, for whatever reason, are considered controversial. They are separated out here to allow people to include as they see fit via custom rulesets. This ruleset was initially created in response to discussions over UnnecessaryConstructorRule which Tom likes but most people really dislike :-)

UnnecessaryConstructor

Since: PMD1.0

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

                    
//ClassOrInterfaceBody[count(ClassOrInterfaceBodyDeclaration/ConstructorDeclaration)=1]
/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration
[@Public='true']
[not(FormalParameters/*)]
[not(BlockStatement)]
[not(NameList)]
[count(ExplicitConstructorInvocation/Arguments/ArgumentList/Expression)=0]
Example(s): 
  
public class Foo {
 public Foo() {}
}

NullAssignment

Since: PMD1.02

Assigning a "null" to a variable (outside of its declaration) is usually bad form. Some times, the assignment is an indication that the programmer doesn't completely understand what is going on in the code. NOTE: This sort of assignment may in rare cases be useful to encourage garbage collection. If that's what you're using it for, by all means, disregard this rule :-)

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.controversial.NullAssignmentRule

Example(s): 
 
 public class Foo {
   public void bar() {
     Object x = null; // This is OK.
     x = new Object();
     // Big, complex piece of code here.
     x = null; // This is BAD.
     // Big, complex piece of code here.
   }
 }

OnlyOneReturn

Since: PMD1.0

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

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.controversial.OnlyOneReturnRule

Example(s): 
 
 public class OneReturnOnly1 {
  public void foo(int x) {
   if (x > 0) {
    return "hey";   // oops, multiple exit points!
   }
   return "hi";
  }
 }

AssignmentInOperand

Since: PMD1.03

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

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.controversial.AssignmentInOperandRule

Example(s): 
  
public class Foo {
 public void bar() {
  int x = 2;
  if ((x = getX()) == 3) {
   System.out.println("3!");
  }
 }
 private int getX() {
  return 3;
 }
}

AtLeastOneConstructor

Since: PMD1.04

Each class should declare at least one constructor. 

              
//ClassOrInterfaceDeclaration[
	not(ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration)
	and
	(@Static = 'false')
	and
	(count(./descendant::MethodDeclaration[@Static]) < 1)
]
	[@Interface='false']
Example(s): 
  
public class Foo {
 // no constructor!  not good!
}

DontImportSun

Since: PMD1.5

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

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.controversial.DontImportSunRule

Example(s): 

import sun.misc.foo;
public class Foo {}

SuspiciousOctalEscape

Since: PMD1.5

A suspicious octal escape sequence was found inside a String literal. The Java language specification (section 3.10.6) 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 by the literal character "8".

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.controversial.SuspiciousOctalEscapeRule

Example(s): 

public class Foo {
 public void foo() {
  // interpreted as octal 12, followed by character '8'
  System.out.println("suspicious: \128");
 }
}

CallSuperInConstructor

Since: PMD3.0

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. 

    
//ClassOrInterfaceDeclaration[ count (ExtendsList/*) > 0 ]
/ClassOrInterfaceBody
 /ClassOrInterfaceBodyDeclaration
 /ConstructorDeclaration[ count (.//ExplicitConstructorInvocation)=0 ]
Example(s): 

public class Foo extends Bar{
 public Foo() {
  // call the constructor of Bar
  super();
 }
 public Foo(int code) {
  // do something with code
  this();
  // no problem with this
 }
}

UnnecessaryParentheses

Since: PMD3.1

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

                  
          //Expression
           /PrimaryExpression
            /PrimaryPrefix
             /Expression[count(*)=1]
              /PrimaryExpression
              /PrimaryPrefix
Example(s): 
  public class Foo {
      boolean bar() {
          return (true);
      }
  }

DefaultPackage

Since: PMD3.4

Use explicit scoping instead of the default package private level. 

//ClassOrInterfaceDeclaration[@Interface='false']
/ClassOrInterfaceBody
/ClassOrInterfaceBodyDeclaration
[
FieldDeclaration[@PackagePrivate='true']
or MethodDeclaration[@PackagePrivate='true']
]

BooleanInversion

Since: PMD3.5

Use bitwise inversion to invert boolean values - it's the fastest way to do this. See http://www.javaspecialists.co.za/archive/newsletter.do?issue=042&locale=en_US for specific details 

    
//AssignmentOperator[@Image="="]/../Expression/UnaryExpressionNotPlusMinus[@Image="!"]
Example(s): 
  
public class Foo {
 public void main(bar) {
  boolean b = true;
  b = !b; // slow
  b ^= true; // fast
 }
}

DataflowAnomalyAnalysis

Since: PMD3.9

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.

This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.controversial.DataflowAnomalyAnalysisRule

Example(s): 

public class Foo {
    public void foo() {
	 int buz = 5;
	 buz = 6; // redefinition of buz -> dd-anomaly
	 foo(buz);
	 buz = 2;
    } // buz is undefined when leaving scope -> du-anomaly
}

AvoidFinalLocalVariable

Since: PMD4.1

Avoid using final local variables, turn them into fields. 

//LocalVariableDeclaration[@Final = 'true']
Example(s): 
public class MyClass {
    public void foo() {
        final String finalLocalVariable;
    }
}

AvoidUsingShortType

Since: PMD4.1

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 caculation and convert the int back to a short. So, the use of the 'short' type may have a greater impact than memory usage. 

                    
            //PrimitiveType[@Image = 'short']
Example(s): 
            
    public class UsingShort
    {
        private short doNotUseShort = 0;

		public UsingShort() {
			short shouldNotBeUsed = 1;
			doNotUseShort += shouldNotBeUsed;
		}
 	}

AvoidUsingVolatile

Since: PMD4.1

Use of the keyword 'volatile' is general 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. 

                    
                        //FieldDeclaration[
                                contains(@Volatile,'true')
                        ]
Example(s): 
			
				public class ThrDeux {
					private volatile String var;

				}

AvoidUsingNativeCode

Since: PMD4.1

As JVM and Java language offer already many help in creating application, it should be very rare to have to rely on non-java code. Even though, it is rare to actually have to use Java Native Interface (JNI). As the use of JNI make application less portable, and harder to maintain, it is not recommended. 

                    
                        //Name[starts-with(@Image,'System.loadLibrary')]
Example(s): 
            
                public class SomeJNIClass {
                        public SomeJNIClass() {
                                System.loadLibrary("nativelib");
                        }

                        static {
                             System.loadLibrary("nativelib");
                        }

                        public void invalidCallsInMethod() throws SecurityException, NoSuchMethodException {
                                System.loadLibrary("nativelib");
                        }
                }

AvoidAccessibilityAlteration

Since: PMD4.1

Methods such as getDeclaredConstructors(), getDeclaredConstructor(Class[]) and setAccessible(), as the interface PrivilegedAction, allow to alter, at runtime, the visilibilty of variable, classes, or methods, even if they are private. Obviously, no one should do so, as such behavior is against everything encapsulation principal stands for. 

                   
                        //PrimaryExpression[
                        (
                        (PrimarySuffix[
                                ends-with(@Image,'getDeclaredConstructors')
                                        or
                                ends-with(@Image,'getDeclaredConstructor')
                                        or
                                ends-with(@Image,'setAccessible')
                                ])
                        or
                        (PrimaryPrefix/Name[
                                ends-with(@Image,'getDeclaredConstructor')
                                or
                                ends-with(@Image,'getDeclaredConstructors')
                                or
                                starts-with(@Image,'AccessibleObject.setAccessible')
                                ])
                        )
                        and
                        (//ImportDeclaration/Name[
                                contains(@Image,'java.security.PrivilegedAction')])
                ]
Example(s): 
            

			import java.lang.reflect.AccessibleObject;
			import java.lang.reflect.Method;
			import java.security.PrivilegedAction;

			public class Violation {
				public void invalidCallsInMethod() throws SecurityException, NoSuchMethodException {
					// Possible call to forbidden getDeclaredConstructors
					Class[] arrayOfClass = new Class[1];
					this.getClass().getDeclaredConstructors();
					this.getClass().getDeclaredConstructor(arrayOfClass);
					Class clazz = this.getClass();
					clazz.getDeclaredConstructor(arrayOfClass);
					clazz.getDeclaredConstructors();

					// Possible call to forbidden setAccessible
					clazz.getMethod("", arrayOfClass).setAccessible(false);
					AccessibleObject.setAccessible(null, false);
					Method.setAccessible(null, false);
					Method[] methodsArray = clazz.getMethods();
					int nbMethod;
					for ( nbMethod = 0; nbMethod < methodsArray.length; nbMethod++ ) {
						methodsArray[nbMethod].setAccessible(false);
					}

					// Possible call to forbidden PrivilegedAction
					PrivilegedAction priv = (PrivilegedAction) new Object(); priv.run();
				}
			}

DoNotCallGarbageCollectionExplicitly

Since: PMD4.2

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. 

                    
//Name[
(starts-with(@Image, 'System.') and
(starts-with(@Image, 'System.gc') or
starts-with(@Image, 'System.runFinalization'))) or
(
starts-with(@Image,'Runtime.getRuntime') and
../../PrimarySuffix[ends-with(@Image,'gc')]
)
]
Example(s): 
            
            public class GCCall
            {
     	public GCCall()
                	{
                        // Explicit gc call !
                        System.gc();
                	}
	public void doSomething()
	 {
		// Explicit gc call !
		Runtime.getRuntime().gc();
	}

public explicitGCcall() { // Explicit gc call ! System.gc(); }

public void doSomething() { // Explicit gc call ! Runtime.getRuntime().gc(); } }

OneDeclarationPerLine

Since: PMD5.0

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 declaration on the same line and report any of them. 

                    
//LocalVariableDeclaration[count(VariableDeclarator) > 1]
Example(s): 
        	
String name ; 	
String lastname ;
Example(s): 
        	
// This should trigger a violation
String lastname,name;

AvoidPrefixingMethodParameters

Since: PMD5.0

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, it's better to document method behavior with Javadoc. 

                    
//MethodDeclaration/MethodDeclarator/FormalParameters/FormalParameter/VariableDeclaratorId[
        pmd:matches(@Image,'^in[A-Z].*','^out[A-Z].*','^in$','^out$')
]
Example(s): 
// Not really clear
public class Foo {
  public void bar(
      int inLeftOperand,
      Result outRightOperand) {
      outRightOperand.setValue(inLeftOperand * outRightOperand.getValue());
  }
}
Example(s): 
// Far more useful
public class Foo {
	/**
	 * 
	 * @param leftOperand, (purpose), not modified by method.
	 * @param rightOperand (purpose), will be modified by the method: contains the result.
	 */
	public void bar(
	      int leftOperand,
	      Result rightOperand) {
	      rightOperand.setValue(leftOperand * rightOperand.getValue());
	}
}