Basic Rules

The Basic Ruleset contains a collection of good practices which everyone should follow.

EmptyCatchBlock

Since: PMD 0.1

Empty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this swallows an exception which should either be acted on or reported.

This rule is defined by the following XPath expression:

    
//CatchStatement
 [count(Block/BlockStatement) = 0 and ($allowCommentedBlocks != 'true' or Block/@containsComment = 'false')]
 [FormalParameter/Type/ReferenceType
   /ClassOrInterfaceType[@Image != 'InterruptedException' and @Image != 'CloneNotSupportedException']
 ]
 
             

Example:

                
  
public void doSomething() {
  try {
    FileInputStream fis = new FileInputStream("/tmp/bugger");
  } catch (IOException ioe) {
      // not good
  }
}
 
      
            

This rule has the following properties:

NameDefault valueDescription
allowCommentedBlocks Empty blocks containing comments will be skipped

EmptyIfStmt

Since: PMD 0.1

Empty If Statement finds instances where a condition is checked but nothing is done about it.

This rule is defined by the following XPath expression:


//IfStatement/Statement
 [EmptyStatement or Block[count(*) = 0]]
 
              

Example:

                
    
public class Foo {
 void bar(int x) {
  if (x == 0) {
   // empty!
  }
 }
}
 
       
            

EmptyWhileStmt

Since: PMD 0.2

Empty While Statement finds all instances where a while statement does nothing. If it is a timing loop, then you should use Thread.sleep() for it; if it's a while loop that does a lot in the exit expression, rewrite it to make it clearer.

This rule is defined by the following XPath expression:


//WhileStatement/Statement[./Block[count(*) = 0]  or ./EmptyStatement]

              

Example:

                
  
public class Foo {
 void bar(int a, int b) {
  while (a == b) {
   // empty!
  }
 }
}
 
       
            

EmptyTryBlock

Since: PMD 0.4

Avoid empty try blocks - what's the point?

This rule is defined by the following XPath expression:


//TryStatement/Block[1][count(*) = 0]

              

Example:

                
  
public class Foo {
 public void bar() {
  try {
  } catch (Exception e) {
    e.printStackTrace();
  }
 }
}

      
            

EmptyFinallyBlock

Since: PMD 0.4

Avoid empty finally blocks - these can be deleted.

This rule is defined by the following XPath expression:


//FinallyStatement[count(Block/BlockStatement) = 0]
 
              

Example:

                
  
public class Foo {
 public void bar() {
  try {
    int x=2;
   } finally {
    // empty!
   }
 }
}
 
      
            

EmptySwitchStatements

Since: PMD 1.0

Avoid empty switch statements.

This rule is defined by the following XPath expression:


//SwitchStatement[count(*) = 1]
 
              

Example:

                
  
public class Foo {
 public void bar() {
  int x = 2;
  switch (x) {
   // once there was code here
   // but it's been commented out or something
  }
 }
}
      
            

JumbledIncrementer

Since: PMD 1.0

Avoid jumbled loop incrementers - it's usually a mistake, and it's confusing even if it's what's intended.

This rule is defined by the following XPath expression:

 
//ForStatement
 [
  ForUpdate/StatementExpressionList/StatementExpression/PostfixExpression/PrimaryExpression/PrimaryPrefix/Name/@Image
  =
  ancestor::ForStatement/ForInit//VariableDeclaratorId/@Image
 ]
 
             

Example:

                
 
public class JumbledIncrementerRule1 {
  public void foo() {
   for (int i = 0; i < 10; i++) {
    for (int k = 0; k < 20; i++) {
     System.out.println("Hello");
    }
   }
  }
 }
 
     
            

ForLoopShouldBeWhileLoop

Since: PMD 1.02

Some for loops can be simplified to while loops - this makes them more concise.

This rule is defined by the following XPath expression:

                
//ForStatement
 [count(*) &gt; 1]
 [not(ForInit)]
 [not(ForUpdate)]
 [not(Type and Expression and Statement)]
 
            

Example:

                
  
public class Foo {
 void bar() {
  for (;true;) true; // No Init or Update part, may as well be: while (true)
 }
}
 
      
            

UnnecessaryConversionTemporary

Since: PMD 0.1

Avoid unnecessary temporaries when converting primitives to Strings

This rule is defined by the following Java class: net.sourceforge.pmd.rules.UnnecessaryConversionTemporary

Example:

                
  
public String convert(int x) {
  // this wastes an object
  String foo = new Integer(x).toString();
  // this is better
  return Integer.toString(x);
}
 
      
            

OverrideBothEqualsAndHashcode

Since: PMD 0.4

Override both public boolean Object.equals(Object other), and public int Object.hashCode(), or override neither. Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitly delegating to your superclass.

This rule is defined by the following Java class: net.sourceforge.pmd.rules.OverrideBothEqualsAndHashcode

Example:

                
  
// this is bad
public class Bar {
  public boolean equals(Object o) {
      // do some comparison
  }
}

// and so is this
public class Baz {
  public int hashCode() {
      // return some hash value
  }
}

// this is OK
public class Foo {
  public boolean equals(Object other) {
      // do some comparison
  }
  public int hashCode() {
      // return some hash value
  }
}
 
      
            

DoubleCheckedLocking

Since: PMD 1.04

Partially created objects can be returned by the Double Checked Locking pattern when used in Java. An optimizing JRE may assign a reference to the baz variable before it creates the object the reference is intended to point to. For more details see http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html.

This rule is defined by the following Java class: net.sourceforge.pmd.rules.DoubleCheckedLocking

Example:

                
  
public class Foo {
  Object baz;
  Object bar() {
    if(baz == null) { //baz may be non-null yet not fully created
      synchronized(this){
        if(baz == null){
          baz = new Object();
        }
      }
    }
    return baz;
  }
}
 
      
            

ReturnFromFinallyBlock

Since: PMD 1.05

Avoid returning from a finally block - this can discard exceptions.

This rule is defined by the following XPath expression:


//FinallyStatement//ReturnStatement

              

Example:

                
  
public class Bar {
 public String foo() {
  try {
   throw new Exception( "My Exception" );
  } catch (Exception e) {
   throw e;
  } finally {
   return "A. O. K."; // Very bad.
  }
 }
}

      
            

EmptySynchronizedBlock

Since: PMD 1.3

Avoid empty synchronized blocks - they're useless.

This rule is defined by the following XPath expression:


//SynchronizedStatement/Block[1][count(*) = 0]

              

Example:

                

public class Foo {
 public void bar() {
  synchronized (this) {
   // empty!
  }
 }
}

      
            

UnnecessaryReturn

Since: PMD 1.3

Avoid unnecessary return statements

This rule is defined by the following Java class: net.sourceforge.pmd.rules.basic.UnnecessaryReturn

Example:

                
  
public class Foo {
 public void bar() {
  int x = 42;
  return;
 }
}
 
      
            

EmptyStaticInitializer

Since: PMD 1.5

An empty static initializer was found.

This rule is defined by the following XPath expression:


//Initializer[@Static='true']/Block[count(*)=0]

                 

Example:

                
   
public class Foo {
 static {
  // empty
 }
 }

       
            

UnconditionalIfStatement

Since: PMD 1.5

Do not use "if" statements that are always true or always false.

This rule is defined by the following XPath expression:

 
//IfStatement/Expression
 [count(PrimaryExpression)=1]
 /PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral

                

Example:

                
  
public class Foo {
 public void close() {
  if (true) {
       // ...
   }
 }
}

      
            

EmptyStatementNotInLoop

Since: PMD 1.5

An empty statement (aka a semicolon by itself) that is not used as the sole body of a for loop or while loop is probably a bug. It could also be a double semicolon, which is useless and should be removed.

This rule is defined by the following XPath expression:


//EmptyStatement
 [not(
       ../../../ForStatement
       or ../../../WhileStatement
       or ../../../BlockStatement/ClassOrInterfaceDeclaration
       or ../../../../../../ForStatement/Statement[1]
        /Block[1]/BlockStatement[1]/Statement/EmptyStatement
       or ../../../../../../WhileStatement/Statement[1]
        /Block[1]/BlockStatement[1]/Statement/EmptyStatement)
 ]

                

Example:

                

public class MyClass {
   public void doit() {
      // this is probably not what you meant to do
      ;
      // the extra semicolon here this is not necessary
      System.out.println("look at the extra semicolon");;
   }
}

       
            

BooleanInstantiation

Since: PMD 1.2

Avoid instantiating Boolean objects; you can reference Boolean.TRUE, Boolean.FALSE, or call Boolean.valueOf() instead.

This rule is defined by the following Java class: net.sourceforge.pmd.rules.basic.BooleanInstantiation

Example:

                
   
public class Foo {
 Boolean bar = new Boolean("true"); // just do a Boolean bar = Boolean.TRUE;
 Boolean buz = Boolean.valueOf(false); // just do a Boolean buz = Boolean.FALSE;
}
   
   
            

UnnecessaryFinalModifier

Since: PMD 3.0

When a class has the final modifier, all the methods are automatically final.

This rule is defined by the following XPath expression:

    
//ClassOrInterfaceDeclaration[@Final='true' and @Interface='false']
/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/MethodDeclaration[@Final='true']
    
              

Example:

                

public final class Foo {
    // This final modifier is not necessary, since the class is final
    // and thus, all methods are final
    private final void foo() {
    }
}


      
            

CollapsibleIfStatements

Since: PMD 3.1

Sometimes two 'if' statements can be consolidated by separating their conditions with a boolean short-circuit operator.

This rule is defined by the following XPath expression:

                
//IfStatement[@Else='false']/Statement
 /IfStatement[@Else='false']
 |
//IfStatement[@Else='false']/Statement
 /Block[count(BlockStatement)=1]/BlockStatement
  /Statement/IfStatement[@Else='false']
            

Example:

                
  
public class Foo {
 void bar() {
  if (x) {
   if (y) {
    // do stuff
   }
  }
 }
}
 
      
            

UselessOverridingMethod

Since: PMD 3.3

The overriding method merely calls the same method defined in a superclass

This rule is defined by the following Java class: net.sourceforge.pmd.rules.UselessOverridingMethod

Example:

                
public void foo(String bar) {
    super.foo(bar);      //Why bother overriding?
}
        
            

Example:

                
public String foo() {
    return super.foo();  //Why bother overriding?
}
        
            

Example:

                
@Id
public Long getId() {
    return super.getId();  //OK if 'ignoreAnnotations' is false, which is the default behavior
}
        
            

This rule has the following properties:

NameDefault valueDescription
ignoreAnnotations false Ignore annotations

ClassCastExceptionWithToArray

Since: PMD 3.4

if you need to get an array of a class from your Collection, you should pass an array of the desidered class as the parameter of the toArray method. Otherwise you will get a ClassCastException.

This rule is defined by the following XPath expression:


//CastExpression[Type/ReferenceType/ClassOrInterfaceType[@Image !=
"Object"]]//PrimaryExpression
[
 PrimaryPrefix/Name[ends-with(@Image, '.toArray')]
 and
 PrimarySuffix/Arguments[count(*) = 0]
and
count(PrimarySuffix) = 1
]

    

Example:

                

import java.util.ArrayList;
import java.util.Collection;

public class Test {

    public static void main(String[] args) {
        Collection c=new ArrayList();
        Integer obj=new Integer(1);
        c.add(obj);

        // this would trigger the rule (and throw a ClassCastException
if executed)
        Integer[] a=(Integer [])c.toArray();

        // this wouldn't trigger the rule
        Integer[] b=(Integer [])c.toArray(new Integer[c.size()]);
    }
}

  
            

AvoidDecimalLiteralsInBigDecimalConstructor

Since: PMD 3.4

One might assume that "new BigDecimal(.1)" is exactly equal to .1, but it is actually equal to .1000000000000000055511151231257827021181583404541015625. This is so because .1 cannot be represented exactly as a double (or, for that matter, as a binary fraction of any finite length). Thus, the long value that is being passed in to the constructor is not exactly equal to .1, appearances notwithstanding. The (String) constructor, on the other hand, is perfectly predictable: 'new BigDecimal(".1")' is exactly equal to .1, as one would expect. Therefore, it is generally recommended that the (String) constructor be used in preference to this one.

This rule is defined by the following XPath expression:


//AllocationExpression[ClassOrInterfaceType[@Image="BigDecimal"]
and
./Arguments/ArgumentList
/Expression/PrimaryExpression/PrimaryPrefix/Literal[(not
(ends-with
(@Image,'"'))) and contains(@Image,".")]]
 
    

Example:

                

import java.math.BigDecimal;
public class Test {

    public static void main(String[] args) {
      // this would trigger the rule
     BigDecimal bd=new BigDecimal(1.123);
      // this wouldn't trigger the rule
     BigDecimal bd=new BigDecimal("1.123");
      // this wouldn't trigger the rule
     BigDecimal bd=new BigDecimal(12);
    }
}

  
            

UselessOperationOnImmutable

Since: PMD 3.5

An operation on an Immutable object (String, BigDecimal or BigInteger) won't change the object itself. The result of the operation is a new object. Therefore, ignoring the operation result is an error.

This rule is defined by the following Java class: net.sourceforge.pmd.rules.UselessOperationOnImmutable

Example:

                
    
import java.math.*;
class Test {
 void method1() {
  BigDecimal bd=new BigDecimal(10);
  bd.add(new BigDecimal(5)); // this will trigger the rule
 }
 void method2() {
  BigDecimal bd=new BigDecimal(10);
  bd = bd.add(new BigDecimal(5)); // this won't trigger the rule
 }
}
    
      
            

MisplacedNullCheck

Since: PMD 3.5

The null check here is misplaced. if the variable is null you'll get a NullPointerException. Either the check is useless (the variable will never be "null") or it's incorrect.

This rule is defined by the following XPath expression:

    
//Expression
    /*[self::ConditionalOrExpression or self::ConditionalAndExpression]
     /descendant::PrimaryExpression/PrimaryPrefix
      /Name[starts-with(@Image,
      concat(ancestor::PrimaryExpression/following-sibling::EqualityExpression
       [./PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
     /PrimaryExpression/PrimaryPrefix
      /Name[count(../../PrimarySuffix)=0]/@Image,"."))
    ]
    
        

Example:

                
    
public class Foo {
 void bar() {
  if (a.equals(baz) && a != null) {}
 }
}
    
      
            

Example:

                
public class Foo {
 void bar() {
  if (a.equals(baz) || a == null) {}
 }
}
   
            

UnusedNullCheckInEquals

Since: PMD 3.5

After checking an object reference for null, you should invoke equals() on that object rather than passing it to another object's equals() method.

This rule is defined by the following XPath expression:

        
//PrimarySuffix[@Image='equals' and not(../PrimaryPrefix/Literal)]
 /following-sibling::PrimarySuffix/Arguments/ArgumentList/Expression
 /PrimaryExpression[count(PrimarySuffix)=0]/PrimaryPrefix
 /Name[@Image = ./../../../../../../../../../../Expression/ConditionalAndExpression
 /EqualityExpression[@Image="!=" and count(./preceding-sibling::*)=0 and
 ./PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
  /PrimaryExpression/PrimaryPrefix/Name/@Image]
        
        

Example:

                

public class Test {

public String method1() { return "ok";}
public String method2() { return null;}

public void method(String a) {
String b;
/*
I don't know it method1() can be "null"
but I know "a" is not null..
I'd better write a.equals(method1())
*/
if (a!=null && method1().equals(a)) { // will
trigger the rule
//whatever
}

if (method1().equals(a) && a != null) { //
won't trigger the rule
//whatever
}

if (a!=null && method1().equals(b)) { // won't
trigger the rule
//whatever
}

if (a!=null && "LITERAL".equals(a)) { // won't
trigger the rule
//whatever
}

if (a!=null && !a.equals("go")) { // won't
trigger the rule
a=method2();
if (method1().equals(a)) {
//whatever
}
}
}
}


            

AvoidThreadGroup

Since: PMD 3.6

Avoid using ThreadGroup; although it is intended to be used in a threaded environment it contains methods that are not thread safe.

This rule is defined by the following XPath expression:


//AllocationExpression/ClassOrInterfaceType[contains(@Image,'ThreadGroup')] |
//PrimarySuffix[contains(@Image, 'getThreadGroup')]

        

Example:

                
    
    public class Bar {
     void buz() {
      ThreadGroup tg = new ThreadGroup("My threadgroup") ;
      tg = new ThreadGroup(tg, "my thread group");
      tg = Thread.currentThread().getThreadGroup();
      tg = System.getSecurityManager().getThreadGroup();
     }
    }
    
      
            

BrokenNullCheck

Since: PMD 3.8

The null check is broken since it will throw a NullPointerException itself. It is likely that you used || instead of && or vice versa.

This rule is defined by the following Java class: net.sourceforge.pmd.rules.basic.BrokenNullCheck

Example:

                

class Foo {
 String bar(String string) {
  // should be &&
  if (string!=null || !string.equals(""))
    return string;
  // should be ||
  if (string==null && string.equals(""))
    return string;
 }
}
        
        
            

BigIntegerInstantiation

Since: PMD 3.9

Don't create instances of already existing BigInteger (BigInteger.ZERO, BigInteger.ONE) and for 1.5 on, BigInteger.TEN and BigDecimal (BigDecimal.ZERO, BigDecimal.ONE, BigDecimal.TEN)

This rule is defined by the following Java class: net.sourceforge.pmd.rules.basic.BigIntegerInstantiation

Example:

                

public class Test {

 public static void main(String[] args) {
   BigInteger bi=new BigInteger(1);
   BigInteger bi2=new BigInteger("0");
   BigInteger bi3=new BigInteger(0.0);
   BigInteger bi4;
   bi4=new BigInteger(0);
 }
}

  
            

AvoidUsingOctalValues

Since: PMD 3.9

Integer literals should not start with zero. Zero means that the rest of literal will be interpreted as an octal value.

This rule is defined by the following Java class: net.sourceforge.pmd.rules.basic.AvoidUsingOctalValues

Example:

                
		    
		public class Foo {
		  int i = 012; // set i with 10 not 12
		  int j = 010; // set j with 8 not 10
		  k = i * j; // set k with 80 not 120
		}
		    
    
            

AvoidUsingHardCodedIP

Since: PMD 4.1

An application with hard coded IP may become impossible to deploy in some case. It never hurts to externalize IP adresses.

This rule is defined by the following Java class: net.sourceforge.pmd.rules.basic.AvoidUsingHardCodedIP

Example:

                
	    
	public class Foo {
	  String ip = "127.0.0.1"; // This is a really bad idea !
	}
	    
	    
            

This rule has the following properties:

NameDefault valueDescription
pattern ^"[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}"$ Regular Expression

CheckResultSet

Since: PMD 4.1

Always check the return of one of the navigation method (next,previous,first,last) of a ResultSet. Indeed, if the value return is 'false', the developer should deal with it !

This rule is defined by the following XPath expression:

		        	
//Type/ReferenceType/ClassOrInterfaceType[
        (@Image = 'ResultSet')
        and
        (../../../descendant::Name[ends-with(@Image,'executeQuery')])
        and
        (
	(not (contains(
                        (./ancestor::Block/descendant::WhileStatement/descendant::Name/attribute::Image),
                        concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.next')
		)  ) )
	and ( not ( contains(
                        (./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
                        concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.next')
		) ) )
	and (not (contains(
                        (./ancestor::Block/descendant::WhileStatement/descendant::Name/attribute::Image),
                        concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.previous')
		)  ) )
	and ( not ( contains(
                        (./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
                        concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.previous')
		) ) )
	and ( not ( contains(
                        (./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
                        concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.last')
		) ) )
	and ( not ( contains(
                        (./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
                        concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.first')
		) ) )

         )
]
		        	
            	

Example:

                
            
            // This is NOT appropriate !
            Statement stat = conn.createStatement();
            ResultSet rst = stat.executeQuery("SELECT name FROM person");
            rst.next(); // what if it returns a 'false' ?
            String firstName = rst.getString(1);

            // This is appropriate...
            Statement stat = conn.createStatement();
            ResultSet rst = stat.executeQuery("SELECT name FROM person");
            if (rst.next())
            {
                String firstName = rst.getString(1);
            }
            else
            {
                // here you deal with the error ( at least log it)
            }
            
        
            

AvoidMultipleUnaryOperators

Since: PMD 4.2

Using multiple unary operators may be a bug, and/or is confusing. Check the usage is not a bug, or consider simplifying the expression.

This rule is defined by the following XPath expression:

		        	
//UnaryExpression[
		./UnaryExpression
		or ./UnaryExpressionNotPlusMinus
		or ./PrimaryExpression/PrimaryPrefix/Expression/UnaryExpression
		or ./PrimaryExpression/PrimaryPrefix/Expression/UnaryExpressionNotPlusMinus
	]
|
//UnaryExpressionNotPlusMinus[
		./UnaryExpression
		or ./UnaryExpressionNotPlusMinus
		or ./PrimaryExpression/PrimaryPrefix/Expression/UnaryExpression
		or ./PrimaryExpression/PrimaryPrefix/Expression/UnaryExpressionNotPlusMinus
	]
		        	
            	

Example:

                
            
            // These are typo bugs, or at best needlessly complex and confusing:
            int i = - -1;
            int j = + - +1;
            int z = ~~2;
            boolean b = !!true;
            boolean c = !!!true;

            // These are better:
            int i = 1;
            int j = -1;
            int z = 2;
            boolean b = true;
            boolean c = false;

            // And these just make your brain hurt:
            int i = ~-2;
            int j = -~7;
            
        
            

EmptyInitializer

Since: PMD 5.0

An empty initializer was found.

This rule is defined by the following XPath expression:


//Initializer/Block[count(*)=0]

                 

Example:

                
   
public class Foo {

   static {} // Why ?

   {} // Again, why ?

}