Error Prone

AssignmentInOperand

Since: PMD 1.03

Priority: Medium (3)

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.errorprone.AssignmentInOperandRule

Example(s):

public void bar() {
    int x = 2;
    if ((x = getX()) == 3) {
      System.out.println("3!");
    }
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/AssignmentInOperand" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/AssignmentInOperand">
    <properties>
        <property name="allowIf" value="false" />
        <property name="allowFor" value="false" />
        <property name="allowWhile" value="false" />
        <property name="allowIncrementDecrement" value="false" />
    </properties>
</rule>

AssignmentToNonFinalStatic

Since: PMD 2.2

Priority: Medium (3)

Identifies a possible unsafe usage of a static field.

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

Example(s):

public class StaticField {
   static int x;
   public FinalFields(int y) {
    x = y; // unsafe
   }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AssignmentToNonFinalStatic" />

AvoidAccessibilityAlteration

Since: PMD 4.1

Priority: Medium (3)

Methods such as getDeclaredConstructors(), getDeclaredMethods(), and getDeclaredFields() also return private constructors, methods and fields. These can be made accessible by calling setAccessible(true). This gives access to normally protected data which violates the principle of encapsulation.

This rule detects calls to setAccessible and finds possible accessibility alterations. If the call to setAccessible is wrapped within a PrivilegedAction, then the access alteration is assumed to be deliberate and is not reported.

Note that with Java 17 the Security Manager, which is used for PrivilegedAction execution, is deprecated: JEP 411: Deprecate the Security Manager for Removal. For future-proof code, deliberate access alteration should be suppressed using the usual suppression methods (e.g. by using @SuppressWarnings annotation).

This rule is defined by the following XPath expression:

//PrimaryPrefix
    [Name[ends-with(@Image, '.setAccessible')]]
    [not(following-sibling::PrimarySuffix/Arguments//BooleanLiteral[@True = false()])]
    [pmd-java:typeIs('java.lang.reflect.AccessibleObject')]
    (: exclude anonymous privileged action classes :)
    [not(ancestor::AllocationExpression[pmd-java:typeIs('java.security.PrivilegedAction')])]
    (: exclude inner privileged action classes :)
    [not(ancestor::ClassOrInterfaceDeclaration[1][pmd-java:typeIs('java.security.PrivilegedAction')])]

Example(s):

import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.security.AccessController;
import java.security.PrivilegedAction;

public class Violation {
    private void invalidSetAccessCalls() throws NoSuchMethodException, SecurityException {
        Constructor<?> constructor = this.getClass().getDeclaredConstructor(String.class);
        // call to forbidden setAccessible
        constructor.setAccessible(true);

        Method privateMethod = this.getClass().getDeclaredMethod("aPrivateMethod");
        // call to forbidden setAccessible
        privateMethod.setAccessible(true);

        // deliberate accessibility alteration
        String privateField = AccessController.doPrivileged(new PrivilegedAction<String>() {
            @Override
            public String run() {
                try {
                    Field field = Violation.class.getDeclaredField("aPrivateField");
                    field.setAccessible(true);
                    return (String) field.get(null);
                } catch (ReflectiveOperationException | SecurityException e) {
                    throw new RuntimeException(e);
                }
            }
        });
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidAccessibilityAlteration" />

AvoidAssertAsIdentifier

Since: PMD 3.4

Priority: Medium High (2)

Use of the term ‘assert’ will conflict with newer versions of Java since it is a reserved word.

This rule is defined by the following XPath expression:

//VariableDeclaratorId[@Name='assert']

Example(s):

public class A {
    public class Foo {
        String assert = "foo";
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidAssertAsIdentifier" />

AvoidBranchingStatementAsLastInLoop

Since: PMD 5.0

Priority: Medium High (2)

Using a branching statement as the last part of a loop may be a bug, and/or is confusing. Ensure that the usage is not a bug, or consider using another approach.

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

Example(s):

// unusual use of branching statement in a loop
for (int i = 0; i < 10; i++) {
    if (i*i <= 25) {
        continue;
    }
    break;
}

// this makes more sense...
for (int i = 0; i < 10; i++) {
    if (i*i > 25) {
        break;
    }
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop">
    <properties>
        <property name="checkBreakLoopTypes" value="for|do|while" />
        <property name="checkContinueLoopTypes" value="for|do|while" />
        <property name="checkReturnLoopTypes" value="for|do|while" />
    </properties>
</rule>

AvoidCallingFinalize

Since: PMD 3.0

Priority: Medium (3)

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.

Note that Oracle has declared Object.finalize() as deprecated since JDK 9.

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

Example(s):

void foo() {
    Bar b = new Bar();
    b.finalize();
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidCallingFinalize" />

AvoidCatchingNPE

Since: PMD 1.8

Priority: Medium (3)

Code should never throw NullPointerExceptions under normal circumstances. A catch block may hide the original error, causing other, more subtle problems later on.

This rule is defined by the following XPath expression:

//CatchStatement/FormalParameter/Type
 /ReferenceType/ClassOrInterfaceType[@Image='NullPointerException']

Example(s):

public class Foo {
    void bar() {
        try {
            // do something
        } catch (NullPointerException npe) {
        }
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidCatchingNPE" />

AvoidCatchingThrowable

Since: PMD 1.2

Priority: Medium (3)

Catching Throwable errors is not recommended since its scope is very broad. It includes runtime issues such as OutOfMemoryError that should be exposed and managed separately.

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

Example(s):

public void bar() {
    try {
        // do something
    } catch (Throwable th) {  // should not catch Throwable
        th.printStackTrace();
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidCatchingThrowable" />

AvoidDecimalLiteralsInBigDecimalConstructor

Since: PMD 3.4

Priority: Medium (3)

One might assume that the result of "new BigDecimal(0.1)" is exactly equal to 0.1, but it is actually equal to .1000000000000000055511151231257827021181583404541015625. This is because 0.1 cannot be represented exactly as a double (or 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 0.1, appearances notwithstanding.

The (String) constructor, on the other hand, is perfectly predictable: ‘new BigDecimal("0.1")’ is exactly equal to 0.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[pmd-java:typeIs('java.math.BigDecimal')]
[Arguments/ArgumentList/Expression/PrimaryExpression
    [
        pmd-java:typeIs('float') or
        pmd-java:typeIs('java.lang.Float') or
        pmd-java:typeIs('double') or
        pmd-java:typeIs('java.lang.Double')
    ]
]

Example(s):

BigDecimal bd = new BigDecimal(1.123);       // loss of precision, this would trigger the rule

BigDecimal bd = new BigDecimal("1.123");     // preferred approach

BigDecimal bd = new BigDecimal(12);          // preferred approach, ok for integer values

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidDecimalLiteralsInBigDecimalConstructor" />

AvoidDuplicateLiterals

Since: PMD 1.0

Priority: Medium (3)

Code containing duplicate String literals can usually be improved by declaring the String as a constant field.

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

Example(s):

private void bar() {
     buz("Howdy");
     buz("Howdy");
     buz("Howdy");
     buz("Howdy");
}
private void buz(String x) {}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/AvoidDuplicateLiterals" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/AvoidDuplicateLiterals">
    <properties>
        <property name="separator" value="," />
        <property name="maxDuplicateLiterals" value="4" />
        <property name="minimumLength" value="3" />
        <property name="skipAnnotations" value="false" />
        <property name="exceptionList" value="" />
    </properties>
</rule>

AvoidEnumAsIdentifier

Since: PMD 3.4

Priority: Medium High (2)

Use of the term ‘enum’ will conflict with newer versions of Java since it is a reserved word.

This rule is defined by the following XPath expression:

//VariableDeclaratorId[@Name='enum']

Example(s):

public class A {
    public class Foo {
        String enum = "foo";
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidEnumAsIdentifier" />

AvoidFieldNameMatchingMethodName

Since: PMD 3.0

Priority: Medium (3)

It can be confusing to have a field name with the same name as a method. While this is permitted, having information (field) and actions (method) is not clear naming. Developers versed in Smalltalk often prefer this approach as the methods denote accessor methods.

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

Example(s):

public class Foo {
    Object bar;
    // bar is data or an action or both?
    void bar() {
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidFieldNameMatchingMethodName" />

AvoidFieldNameMatchingTypeName

Since: PMD 3.0

Priority: Medium (3)

It is somewhat confusing to have a field name matching the declaring type name. This probably means that type and/or field names should be chosen more carefully.

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

Example(s):

public class Foo extends Bar {
    int foo;    // There is probably a better name that can be used
}
public interface Operation {
    int OPERATION = 1; // There is probably a better name that can be used
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidFieldNameMatchingTypeName" />

AvoidInstanceofChecksInCatchClause

Since: PMD 3.0

Priority: Medium (3)

Each caught exception type should be handled in its own catch clause.

This rule is defined by the following XPath expression:

//CatchStatement/FormalParameter
 /following-sibling::Block//InstanceOfExpression/PrimaryExpression/PrimaryPrefix
  /Name[
   @Image = ./ancestor::Block/preceding-sibling::FormalParameter
    /VariableDeclaratorId/@Name
  ]

Example(s):

try { // Avoid this
    // do something
} catch (Exception ee) {
    if (ee instanceof IOException) {
        cleanup();
    }
}

try {  // Prefer this:
    // do something
} catch (IOException ee) {
    cleanup();
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidInstanceofChecksInCatchClause" />

AvoidLiteralsInIfCondition

Since: PMD 4.2.6

Priority: Medium (3)

Avoid using hard-coded literals in conditional statements. By declaring them as static variables or private members with descriptive names maintainability is enhanced. By default, the literals "-1" and "0" are ignored. More exceptions can be defined with the property "ignoreMagicNumbers".

The rule doesn’t consider deeper expressions by default, but this can be enabled via the property ignoreExpressions. With this property set to false, if-conditions like i == 1 + 5 are reported as well. Note that in that case, the property ignoreMagicNumbers is not taken into account, if there are multiple literals involved in such an expression.

This rule is defined by the following XPath expression:

(: simple case - no deep expressions :)
//IfStatement[$ignoreExpressions = true()]/Expression/*/PrimaryExpression/PrimaryPrefix/Literal
    [not(NullLiteral)]
    [not(BooleanLiteral)]
    [empty(index-of(tokenize($ignoreMagicNumbers, '\s*,\s*'), @Image))]
|
(: consider also deeper expressions :)
//IfStatement[$ignoreExpressions = false()]/Expression//*[local-name() != 'UnaryExpression' or @Operator != '-']/PrimaryExpression/PrimaryPrefix/Literal
    [not(NullLiteral)]
    [not(BooleanLiteral)]
    [empty(index-of(tokenize($ignoreMagicNumbers, '\s*,\s*'), @Image))]
|
(: consider negative literals :)
//IfStatement[$ignoreExpressions = false()]/Expression//UnaryExpression[@Operator = '-']/PrimaryExpression/PrimaryPrefix/Literal
    [not(NullLiteral)]
    [not(BooleanLiteral)]
    [empty(index-of(tokenize($ignoreMagicNumbers, '\s*,\s*'), concat('-', @Image)))]
|
(: consider multiple literals in expressions :)
//IfStatement[$ignoreExpressions = false()]/Expression[count(*/PrimaryExpression/PrimaryPrefix/Literal
    [not(NullLiteral)]
    [not(BooleanLiteral)]) > 1]

Example(s):

private static final int MAX_NUMBER_OF_REQUESTS = 10;

public void checkRequests() {

    if (i == 10) {                        // magic number, buried in a method
      doSomething();
    }

    if (i == MAX_NUMBER_OF_REQUESTS) {    // preferred approach
      doSomething();
    }

    if (aString.indexOf('.') != -1) {}     // magic number -1, by default ignored
    if (aString.indexOf('.') >= 0) { }     // alternative approach

    if (aDouble > 0.0) {}                  // magic number 0.0
    if (aDouble >= Double.MIN_VALUE) {}    // preferred approach

    // with rule property "ignoreExpressions" set to "false"
    if (i == pos + 5) {}  // violation: magic number 5 within an (additive) expression
    if (i == pos + SUFFIX_LENGTH) {} // preferred approach
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/AvoidLiteralsInIfCondition" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/AvoidLiteralsInIfCondition">
    <properties>
        <property name="ignoreMagicNumbers" value="-1,0" />
        <property name="ignoreExpressions" value="true" />
    </properties>
</rule>

AvoidLosingExceptionInformation

Since: PMD 4.2.6

Priority: Medium High (2)

Statements in a catch block that invoke accessors on the exception without using the information only add to code size. Either remove the invocation, or use the return result.

This rule is defined by the following XPath expression:

//CatchStatement/Block/BlockStatement/Statement/StatementExpression/PrimaryExpression/PrimaryPrefix/Name
[
   @Image = concat(../../../../../../../FormalParameter/VariableDeclaratorId/@Name, '.getMessage')
   or
   @Image = concat(../../../../../../../FormalParameter/VariableDeclaratorId/@Name, '.getLocalizedMessage')
   or
   @Image = concat(../../../../../../../FormalParameter/VariableDeclaratorId/@Name, '.getCause')
   or
   @Image = concat(../../../../../../../FormalParameter/VariableDeclaratorId/@Name, '.getStackTrace')
   or
   @Image = concat(../../../../../../../FormalParameter/VariableDeclaratorId/@Name, '.toString')
]

Example(s):

public void bar() {
    try {
        // do something
    } catch (SomeException se) {
        se.getMessage();
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidLosingExceptionInformation" />

AvoidMultipleUnaryOperators

Since: PMD 4.2

Priority: Medium High (2)

The use of multiple unary operators may be problematic, and/or confusing. Ensure that the intended usage is not a bug, or consider simplifying the expression.

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

Example(s):

// 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;

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/AvoidMultipleUnaryOperators" />

AvoidUsingOctalValues

Since: PMD 3.9

Priority: Medium (3)

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

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

Example(s):

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

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/AvoidUsingOctalValues" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/AvoidUsingOctalValues">
    <properties>
        <property name="strict" value="false" />
    </properties>
</rule>

BadComparison

Deprecated

This rule has been renamed. Use instead: ComparisonWithNaN

Deprecated

Since: PMD 6.36.0

Priority: Medium (3)

Reports comparisons with double and float NaN (Not-a-Number) values. These are specified to have unintuitive behavior: NaN is considered unequal to itself. This means a check like someDouble == Double.NaN will always return false, even if someDouble is really the NaN value. To test whether a value is the NaN value, one should instead use Double.isNaN(someDouble) (or Float.isNaN). The != operator should be treated similarly. Finally, comparisons like someDouble <= Double.NaN are nonsensical and will always evaluate to false.

This rule has been renamed from "BadComparison" with PMD 6.36.0.

This rule is defined by the following XPath expression:

(//EqualityExpression | //RelationalExpression)/PrimaryExpression/PrimaryPrefix/Name[@Image='Double.NaN' or @Image='Float.NaN']

Example(s):

boolean x = (y == Double.NaN);

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/BadComparison" />

BeanMembersShouldSerialize

Since: PMD 1.1

Priority: Medium (3)

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.

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

Example(s):

private transient int someFoo;  // good, it's transient
private static int otherFoo;    // also OK
private int moreFoo;            // OK, has proper accessors, see below
private int badFoo;             // bad, should be marked transient

private void setMoreFoo(int moreFoo){
      this.moreFoo = moreFoo;
}

private int getMoreFoo(){
      return this.moreFoo;
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/BeanMembersShouldSerialize" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/BeanMembersShouldSerialize">
    <properties>
        <property name="ignoredAnnotations" value="lombok.Data|lombok.Getter|lombok.Value" />
        <property name="prefix" value="" />
    </properties>
</rule>

BrokenNullCheck

Since: PMD 3.8

Priority: Medium High (2)

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.lang.java.rule.errorprone.BrokenNullCheckRule

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/BrokenNullCheck" />

CallSuperFirst

Since: PMD 4.2.5

Priority: Medium (3)

Super should be called at the start of the method

This rule is defined by the following XPath expression:

//MethodDeclaration[
  @Name='onCreate' or
  @Name='onConfigurationChanged' or
  @Name='onPostCreate' or
  @Name='onPostResume' or
  @Name='onRestart' or
  @Name='onRestoreInstanceState' or
  @Name='onResume' or
  @Name='onStart'
  ]
    /Block[not(
      (BlockStatement[1]/Statement/StatementExpression/PrimaryExpression[./PrimaryPrefix[@SuperModifier= true()]]/PrimarySuffix[@Image= ancestor::MethodDeclaration/@Name]))]
[ancestor::ClassOrInterfaceDeclaration[ExtendsList/ClassOrInterfaceType[
  pmd-java:typeIs('android.app.Activity') or
  pmd-java:typeIs('android.app.Application') or
  pmd-java:typeIs('android.app.Service')
]]]

Example(s):

public class DummyActivity extends Activity {
    public void onCreate(Bundle bundle) {
        // missing call to super.onCreate(bundle)
        foo();
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/CallSuperFirst" />

CallSuperLast

Since: PMD 4.2.5

Priority: Medium (3)

Super should be called at the end of the method

This rule is defined by the following XPath expression:

//MethodDeclaration[
  @Name='finish' or
  @Name='onDestroy' or
  @Name='onPause' or
  @Name='onSaveInstanceState' or
  @Name='onStop' or
  @Name='onTerminate'
  ]
   /Block/BlockStatement[last()]
    [not(Statement/StatementExpression/PrimaryExpression[./PrimaryPrefix[@SuperModifier= true()]]/PrimarySuffix[@Image= ancestor::MethodDeclaration/@Name])]
[ancestor::ClassOrInterfaceDeclaration[ExtendsList/ClassOrInterfaceType[
  pmd-java:typeIs('android.app.Activity') or
  pmd-java:typeIs('android.app.Application') or
  pmd-java:typeIs('android.app.Service')
]]]

Example(s):

public class DummyActivity extends Activity {
    public void onPause() {
        foo();
        // missing call to super.onPause()
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/CallSuperLast" />

CheckSkipResult

Since: PMD 5.0

Priority: Medium (3)

The skip() method may skip a smaller number of bytes than requested. Check the returned value to find out if it was the case or not.

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

Example(s):

public class Foo {

   private FileInputStream _s = new FileInputStream("file");

   public void skip(int n) throws IOException {
      _s.skip(n); // You are not sure that exactly n bytes are skipped
   }

   public void skipExactly(int n) throws IOException {
      while (n != 0) {
         long skipped = _s.skip(n);
         if (skipped == 0)
            throw new EOFException();
         n -= skipped;
      }
   }

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/CheckSkipResult" />

ClassCastExceptionWithToArray

Since: PMD 3.4

Priority: Medium (3)

When deriving an array of a specific class from your Collection, one should provide an array of the same class as the parameter of the toArray() method. Doing otherwise you will will result in a ClassCastException.

This rule is defined by the following XPath expression:

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

Example(s):

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 is fine and will not trigger the rule
Integer[] b = (Integer [])c.toArray(new Integer[c.size()]);

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/ClassCastExceptionWithToArray" />

CloneMethodMustBePublic

Since: PMD 5.4.0

Priority: Medium (3)

The java Manual says "By convention, classes that implement this interface should override Object.clone (which is protected) with a public method."

This rule is defined by the following XPath expression:

//MethodDeclaration[@Public= false()]
  [@Name = 'clone']
  [@Arity = 0]

Example(s):

public class Foo implements Cloneable {
    @Override
    protected Object clone() throws CloneNotSupportedException { // Violation, must be public
    }
}

public class Foo implements Cloneable {
    @Override
    protected Foo clone() { // Violation, must be public
    }
}

public class Foo implements Cloneable {
    @Override
    public Object clone() // Ok
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/CloneMethodMustBePublic" />

CloneMethodMustImplementCloneable

Since: PMD 1.9

Priority: Medium (3)

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.

The rule can also detect, if the class implements or extends a Cloneable class.

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

Example(s):

public class MyClass {
 public Object clone() throws CloneNotSupportedException {
  return foo;
 }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/CloneMethodMustImplementCloneable" />

CloneMethodReturnTypeMustMatchClassName

Since: PMD 5.4.0

Priority: Medium (3)

Minimum Language Version: Java 1.5

If a class implements cloneable the return type of the method clone() must be the class name. That way, the caller of the clone method doesn’t need to cast the returned clone to the correct type.

Note: This is only possible with Java 1.5 or higher.

This rule is defined by the following XPath expression:

//MethodDeclaration
[
@Name = 'clone'
and @Arity = 0
and not (ResultType//ClassOrInterfaceType/@Image = ancestor::ClassOrInterfaceDeclaration[1]/@SimpleName)
]

Example(s):

public class Foo implements Cloneable {
    @Override
    protected Object clone() { // Violation, Object must be Foo
    }
}

public class Foo implements Cloneable {
    @Override
    public Foo clone() { //Ok
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/CloneMethodReturnTypeMustMatchClassName" />

CloneThrowsCloneNotSupportedException

Deprecated

Since: PMD 1.9

Priority: Medium (3)

The method clone() should throw a CloneNotSupportedException.

This rule is deprecated since PMD 6.35.0 without replacement. The rule has no real value as CloneNotSupportedException is a checked exception and therefore you need to deal with it while implementing the clone() method. You either need to declare the exception or catch it. If you catch it, then subclasses can’t throw it themselves explicitly. However, Object.clone() will still throw this exception if the Cloneable interface is not implemented.

This rule is defined by the following XPath expression:

//MethodDeclaration
[
@Name = 'clone'
and @Arity = 0
and count(NameList/Name[contains
(@Image,'CloneNotSupportedException')]) = 0
]
[
../../../../ClassOrInterfaceDeclaration[@Final = false()]
]

Example(s):

public class MyClass implements Cloneable{
    public Object clone() { // will cause an error
         MyClass clone = (MyClass)super.clone();
         return clone;
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/CloneThrowsCloneNotSupportedException" />

CloseResource

Since: PMD 1.2.2

Priority: Medium (3)

Ensure that resources (like java.sql.Connection, java.sql.Statement, and java.sql.ResultSet objects and any subtype of java.lang.AutoCloseable) are always closed after use. Failing to do so might result in resource leaks.

Note: It suffices to configure the super type, e.g. java.lang.AutoClosable, so that this rule automatically triggers on any subtype (e.g. java.io.FileInputStream). Additionally specifying java.sql.Connection helps in detecting the types, if the type resolution / auxclasspath is not correctly setup.

Note: Since PMD 6.16.0 the default value for the property types contains java.lang.AutoCloseable and detects now cases where the standard java.io.*Stream classes are involved. In order to restore the old behaviour, just remove "AutoCloseable" from the types.

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

Example(s):

public class Bar {
    public void withSQL() {
        Connection c = pool.getConnection();
        try {
            // do stuff
        } catch (SQLException ex) {
           // handle exception
        } finally {
            // oops, should close the connection using 'close'!
            // c.close();
        }
    }

    public void withFile() {
        InputStream file = new FileInputStream(new File("/tmp/foo"));
        try {
            int c = file.in();
        } catch (IOException e) {
            // handle exception
        } finally {
            // TODO: close file
        }
    }
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/CloseResource" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/CloseResource">
    <properties>
        <property name="closeTargets" value="" />
        <property name="types" value="java.lang.AutoCloseable,java.sql.Connection,java.sql.Statement,java.sql.ResultSet" />
        <property name="closeAsDefaultTarget" value="true" />
        <property name="allowedResourceTypes" value="java.io.ByteArrayOutputStream|java.io.ByteArrayInputStream|java.io.StringWriter|java.io.CharArrayWriter|java.util.stream.Stream|java.util.stream.IntStream|java.util.stream.LongStream|java.util.stream.DoubleStream" />
        <property name="closeNotInFinally" value="false" />
    </properties>
</rule>

CompareObjectsWithEquals

Since: PMD 3.2

Priority: Medium (3)

Use equals() to compare object references; avoid comparing them with ==.

Since comparing objects with named constants is useful in some cases (eg, when defining constants for sentinel values), the rule ignores comparisons against fields with all-caps name (eg this == SENTINEL), which is a common naming convention for constant fields.

You may allow some types to be compared by reference by listing the exceptions in the typesThatCompareByReference property.

This rule is defined by the following XPath expression:

//EqualityExpression
    [count(
        PrimaryExpression[pmd-java:typeIs('java.lang.Object')]
        [not(some $t in $typesThatCompareByReference satisfies pmd-java:typeIs($t))]
       ) = 2
    ]
    [not(PrimaryExpression[PrimaryPrefix/@ThisModifier = true()]
                          [not(PrimarySuffix)]
                          [ancestor::MethodDeclaration[@Name = 'equals']])
    ]
    (: Is not a field access with an all-caps identifier :)
    [not(PrimaryExpression[not(PrimarySuffix) and PrimaryPrefix/Name[upper-case(@Image)=@Image]
     or PrimaryExpression/PrimarySuffix[last()][upper-case(@Image)=@Image]])]

Example(s):

class Foo {
  boolean bar(String a, String b) {
    return a == b;
  }
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/CompareObjectsWithEquals" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/CompareObjectsWithEquals">
    <properties>
        <property name="typesThatCompareByReference" value="java.lang.Enum,java.lang.Class" />
    </properties>
</rule>

ComparisonWithNaN

Since: PMD 6.36.0

Priority: Medium (3)

Reports comparisons with double and float NaN (Not-a-Number) values. These are specified to have unintuitive behavior: NaN is considered unequal to itself. This means a check like someDouble == Double.NaN will always return false, even if someDouble is really the NaN value. To test whether a value is the NaN value, one should instead use Double.isNaN(someDouble) (or Float.isNaN). The != operator should be treated similarly. Finally, comparisons like someDouble <= Double.NaN are nonsensical and will always evaluate to false.

This rule has been renamed from "BadComparison" with PMD 6.36.0.

This rule is defined by the following XPath expression:

(//EqualityExpression | //RelationalExpression)/PrimaryExpression/PrimaryPrefix/Name[@Image='Double.NaN' or @Image='Float.NaN']

Example(s):

boolean x = (y == Double.NaN);

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/ComparisonWithNaN" />

ConstructorCallsOverridableMethod

Since: PMD 1.04

Priority: High (1)

Calling overridable methods during construction poses a risk of invoking methods on an incompletely constructed object and can be difficult to debug. It may leave the sub-class unable to construct its superclass or forced to replicate the construction process completely within itself, losing the ability to call super(). If the default constructor contains a call to an overridable method, the subclass may be completely uninstantiable. Note that this includes method calls throughout the control flow graph - i.e., if a constructor Foo() calls a private method bar() that calls a public method buz(), this denotes a problem.

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

Example(s):

public class SeniorClass {
  public SeniorClass(){
      toString(); //may throw NullPointerException if overridden
  }
  public String toString(){
    return "IAmSeniorClass";
  }
}
public class JuniorClass extends SeniorClass {
  private String name;
  public JuniorClass(){
    super(); //Automatic call leads to NullPointerException
    name = "JuniorClass";
  }
  public String toString(){
    return name.toUpperCase();
  }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/ConstructorCallsOverridableMethod" />

DataflowAnomalyAnalysis

Deprecated

Since: PMD 3.9

Priority: Low (5)

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. DU - Anomaly: A recently defined variable is undefined. These anomalies may appear in normal source text.
2. DD - Anomaly: A recently defined variable is redefined. This is ominous but don’t have to be a bug.

This rule is deprecated. Use UnusedAssignment in category bestpractices instead.

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

Example(s):

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

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/DataflowAnomalyAnalysis" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/DataflowAnomalyAnalysis">
    <properties>
        <property name="maxPaths" value="1000" />
        <property name="maxViolations" value="100" />
    </properties>
</rule>

DetachedTestCase

Since: PMD 6.13.0

Priority: Medium (3)

The method appears to be a test case since it has public or default visibility, non-static access, no arguments, no return value, has no annotations, but is a member of a class that has one or more JUnit test cases. If it is a utility method, it should likely have private visibility. If it is an ignored test, it should be annotated with @Test and @Ignore.

This rule is defined by the following XPath expression:

//ClassOrInterfaceBodyDeclaration
[../ClassOrInterfaceBodyDeclaration/Annotation/*/Name
        [pmd-java:typeIs('org.junit.Test')
         or pmd-java:typeIs('org.junit.jupiter.api.Test')
         or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest')
         or pmd-java:typeIs('org.junit.jupiter.api.TestFactory')
         or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate')
         or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest')]
]
[not(Annotation)]
[MethodDeclaration[(@Public = true() or @PackagePrivate = true()) and @Static = false() and
        ResultType[@Void = true()] and
        MethodDeclarator/FormalParameters[@Size = 0]
    ]
]

Example(s):

public class MyTest {
    @Test
    public void someTest() {
    }

    // violation: Not annotated
    public void someOtherTest () {
    }

}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DetachedTestCase" />

DoNotCallGarbageCollectionExplicitly

Since: PMD 4.2

Priority: Medium High (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.

This rule is defined by the following XPath expression:

//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();
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DoNotCallGarbageCollectionExplicitly" />

DoNotCallSystemExit

Deprecated

This rule has been renamed. Use instead: DoNotTerminateVM

Deprecated

Since: PMD 4.1

Priority: Medium (3)

Web applications should not call System.exit(), since only the web container or the application server should stop the JVM. Otherwise a web application would terminate all other applications running on the same application server.

This rule also checks for the equivalent calls Runtime.getRuntime().exit() and Runtime.getRuntime().halt().

This rule was called DoNotCallSystemExit until PMD 6.29.0.

This rule is defined by the following XPath expression:

//Name[
    starts-with(@Image,'System.exit')
    or
    (starts-with(@Image,'Runtime.getRuntime') and ../../PrimarySuffix[ends-with(@Image,'exit') or ends-with(@Image,'halt')])
][not(ancestor::MethodDeclaration[1][@Name="main" and @Static = true()])]

Example(s):

public void bar() {
    System.exit(0);                 // never call this when running in an application server!
}
public void foo() {
    Runtime.getRuntime().exit(0);   // never stop the JVM manually, the container will do this.
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DoNotCallSystemExit" />

DoNotExtendJavaLangThrowable

Since: PMD 6.0.0

Priority: Medium (3)

Extend Exception or RuntimeException instead of Throwable.

This rule is defined by the following XPath expression:

//ClassOrInterfaceDeclaration/ExtendsList/ClassOrInterfaceType
  [@Image="Throwable" or @Image="java.lang.Throwable"]

Example(s):

public class Foo extends Throwable { }

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DoNotExtendJavaLangThrowable" />

DoNotHardCodeSDCard

Since: PMD 4.2.6

Priority: Medium (3)

Use Environment.getExternalStorageDirectory() instead of "/sdcard"

This rule is defined by the following XPath expression:

//Literal[starts-with(@Image,'"/sdcard')]

Example(s):

public class MyActivity extends Activity {
    protected void foo() {
        String storageLocation = "/sdcard/mypackage";   // hard-coded, poor approach

       storageLocation = Environment.getExternalStorageDirectory() + "/mypackage"; // preferred approach
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DoNotHardCodeSDCard" />

DoNotTerminateVM

Since: PMD 4.1

Priority: Medium (3)

Web applications should not call System.exit(), since only the web container or the application server should stop the JVM. Otherwise a web application would terminate all other applications running on the same application server.

This rule also checks for the equivalent calls Runtime.getRuntime().exit() and Runtime.getRuntime().halt().

This rule was called DoNotCallSystemExit until PMD 6.29.0.

This rule is defined by the following XPath expression:

//Name[
    starts-with(@Image,'System.exit')
    or
    (starts-with(@Image,'Runtime.getRuntime') and ../../PrimarySuffix[ends-with(@Image,'exit') or ends-with(@Image,'halt')])
][not(ancestor::MethodDeclaration[1][@Name="main" and @Static = true()])]

Example(s):

public void bar() {
    System.exit(0);                 // never call this when running in an application server!
}
public void foo() {
    Runtime.getRuntime().exit(0);   // never stop the JVM manually, the container will do this.
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DoNotTerminateVM" />

DoNotThrowExceptionInFinally

Since: PMD 4.2

Priority: Medium Low (4)

Throwing exceptions within a ‘finally’ block is confusing since they may mask other exceptions or code defects. Note: This is a PMD implementation of the Lint4j rule "A throw in a finally block"

This rule is defined by the following XPath expression:

//FinallyStatement[descendant::ThrowStatement]

Example(s):

public class Foo {
    public void bar() {
        try {
            // Here do some stuff
        } catch( Exception e) {
            // Handling the issue
        } finally {
            // is this really a good idea ?
            throw new Exception();
        }
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DoNotThrowExceptionInFinally" />

DontImportSun

Since: PMD 1.5

Priority: Medium Low (4)

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.errorprone.DontImportSunRule

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DontImportSun" />

DontUseFloatTypeForLoopIndices

Since: PMD 4.3

Priority: Medium (3)

Don’t use floating point for loop indices. If you must use floating point, use double unless you’re certain that float provides enough precision and you have a compelling performance need (space or time).

This rule is defined by the following XPath expression:

//ForStatement/ForInit/LocalVariableDeclaration
/Type/PrimitiveType[@Image="float"]

Example(s):

public class Count {
  public static void main(String[] args) {
    final int START = 2000000000;
    int count = 0;
    for (float f = START; f < START + 50; f++)
      count++;
      //Prints 0 because (float) START == (float) (START + 50).
      System.out.println(count);
      //The termination test misbehaves due to floating point granularity.
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/DontUseFloatTypeForLoopIndices" />

EmptyCatchBlock

Since: PMD 0.1

Priority: Medium (3)

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
 [not(Block/BlockStatement)]
 [$allowCommentedBlocks != true() or Block/@containsComment = false()]
 [FormalParameter/Type/ReferenceType
   /ClassOrInterfaceType[@Image != 'InterruptedException' and @Image != 'CloneNotSupportedException']
 ]
 [FormalParameter/VariableDeclaratorId[not(matches(@Name, $allowExceptionNameRegex))]]

Example(s):

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

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/errorprone.xml/EmptyCatchBlock" />

Use this rule and customize it:

<rule ref="category/java/errorprone.xml/EmptyCatchBlock">
    <properties>
        <property name="allowCommentedBlocks" value="false" />
        <property name="allowExceptionNameRegex" value="^(ignored|expected)$" />
    </properties>
</rule>

EmptyFinalizer

Since: PMD 1.5

Priority: Medium (3)

Empty finalize methods serve no purpose and should be removed. Note that Oracle has declared Object.finalize() as deprecated since JDK 9.

This rule is defined by the following XPath expression:

//MethodDeclaration[@Name='finalize'][@Arity = 0]
  /Block[not(*)]

Example(s):

public class Foo {
   protected void finalize() {}
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyFinalizer" />

EmptyFinallyBlock

Since: PMD 0.4

Priority: Medium (3)

Empty finally blocks serve no purpose and should be removed.

This rule is defined by the following XPath expression:

//FinallyStatement[not(Block/BlockStatement)]

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyFinallyBlock" />

EmptyIfStmt

Since: PMD 0.1

Priority: Medium (3)

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[not(*)]]

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyIfStmt" />

EmptyInitializer

Since: PMD 5.0

Priority: Medium (3)

Empty initializers serve no purpose and should be removed.

This rule is defined by the following XPath expression:

//Initializer/Block[not(*)]

Example(s):

public class Foo {

   static {} // Why ?

   {} // Again, why ?

}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyInitializer" />

EmptyStatementBlock

Since: PMD 5.0

Priority: Medium (3)

Empty block statements serve no purpose and should be removed.

This rule is defined by the following XPath expression:

//BlockStatement/Statement/Block[not(*)]

Example(s):

public class Foo {

   private int _bar;

   public void setBar(int bar) {
      { _bar = bar; } // Why not?
      {} // But remove this.
   }

}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyStatementBlock" />

EmptyStatementNotInLoop

Since: PMD 1.5

Priority: Medium (3)

An empty statement (or a semicolon by itself) that is not used as the sole body of a ‘for’ or ‘while’ loop is probably a bug. It could also be a double semicolon, which has no purpose 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(s):

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");;
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyStatementNotInLoop" />

EmptySwitchStatements

Since: PMD 1.0

Priority: Medium (3)

Empty switch statements serve no purpose and should be removed.

This rule is defined by the following XPath expression:

//SwitchStatement[count(*) = 1]

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptySwitchStatements" />

EmptySynchronizedBlock

Since: PMD 1.3

Priority: Medium (3)

Empty synchronized blocks serve no purpose and should be removed.

This rule is defined by the following XPath expression:

//SynchronizedStatement/Block[1][not(*)]

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptySynchronizedBlock" />

EmptyTryBlock

Since: PMD 0.4

Priority: Medium (3)

Avoid empty try blocks - what’s the point?

This rule is defined by the following XPath expression:

//TryStatement[not(ResourceSpecification)]/Block[1][not(*)]

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyTryBlock" />

EmptyWhileStmt

Since: PMD 0.2

Priority: Medium (3)

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 is 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[not(*)] or EmptyStatement]

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EmptyWhileStmt" />

EqualsNull

Since: PMD 1.9

Priority: High (1)

Tests for null should not use the equals() method. The ‘==’ operator should be used instead.

This rule is defined by the following XPath expression:

//PrimaryExpression
  [
    PrimaryPrefix[Name[ends-with(@Image, 'equals')]]
      [following-sibling::node()/Arguments/ArgumentList[count(Expression)=1]
          /Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]

    or

    PrimarySuffix[ends-with(@Image, 'equals')]
      [following-sibling::node()/Arguments/ArgumentList[count(Expression)=1]
          /Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]

  ]

Example(s):

String x = "foo";

if (x.equals(null)) {   // bad form
    doSomething();
}

if (x == null) {        // preferred
    doSomething();
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/EqualsNull" />

FinalizeDoesNotCallSuperFinalize

Since: PMD 1.5

Priority: Medium (3)

If the finalize() is implemented, its last action should be to call super.finalize. Note that Oracle has declared Object.finalize() as deprecated since JDK 9.

This rule is defined by the following XPath expression:

//MethodDeclaration[@Name='finalize'][@Arity = 0]
   /Block
      /BlockStatement[last()]
      [not(Statement/StatementExpression/PrimaryExpression
            [./PrimaryPrefix[@SuperModifier= true()]]
            [./PrimarySuffix[@Image='finalize']]
          )
      ]
      [not(Statement/TryStatement/FinallyStatement
       /Block/BlockStatement/Statement/StatementExpression/PrimaryExpression
            [./PrimaryPrefix[@SuperModifier= true()]]
            [./PrimarySuffix[@Image='finalize']]
          )
      ]

Example(s):

protected void finalize() {
    something();
    // neglected to call super.finalize()
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/FinalizeDoesNotCallSuperFinalize" />

FinalizeOnlyCallsSuperFinalize

Since: PMD 1.5

Priority: Medium (3)

If the finalize() is implemented, it should do something besides just calling super.finalize(). Note that Oracle has declared Object.finalize() as deprecated since JDK 9.

This rule is defined by the following XPath expression:

//MethodDeclaration[@Name='finalize'][@Arity = 0]
   /Block[count(BlockStatement)=1]
     /BlockStatement[
       Statement/StatementExpression/PrimaryExpression
       [./PrimaryPrefix[@SuperModifier= true()]]
       [./PrimarySuffix[@Image='finalize']]
     ]

Example(s):

protected void finalize() {
    super.finalize();
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/FinalizeOnlyCallsSuperFinalize" />

FinalizeOverloaded

Since: PMD 1.5

Priority: Medium (3)

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.

Note that Oracle has declared Object.finalize() as deprecated since JDK 9.

This rule is defined by the following XPath expression:

//MethodDeclaration[@Name='finalize'][@Arity > 0]

Example(s):

public class Foo {
    // this is confusing and probably a bug
    protected void finalize(int a) {
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/FinalizeOverloaded" />

FinalizeShouldBeProtected

Since: PMD 1.1

Priority: Medium (3)

When overriding the finalize(), the new method should be set as protected. If made public, other classes may invoke it at inappropriate times.

Note that Oracle has declared Object.finalize() as deprecated since JDK 9.

This rule is defined by the following XPath expression:

//MethodDeclaration[@Protected=false()][@Name='finalize'][@Arity = 0]

Example(s):

public void finalize() {
    // do something
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/FinalizeShouldBeProtected" />

IdempotentOperations

Since: PMD 2.0

Priority: Medium (3)

Avoid idempotent operations - they have no effect.

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

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/IdempotentOperations" />

ImplicitSwitchFallThrough

Since: PMD 3.0

Priority: Medium (3)

Switch statements without break or return statements for each case option may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through.

This rule has been renamed from "MissingBreakInSwitch" with PMD 6.37.0.

This rule is defined by the following XPath expression:

//SwitchStatement
[(count(.//BreakStatement)
 + count(BlockStatement//Statement/ReturnStatement)
 + count(BlockStatement//Statement/ContinueStatement)
 + count(BlockStatement//Statement/ThrowStatement)
 + count(BlockStatement//Statement/IfStatement[@Else= true() and Statement[2][ReturnStatement|ContinueStatement|ThrowStatement]]/Statement[1][ReturnStatement|ContinueStatement|ThrowStatement])
 + count(SwitchLabel[ following-sibling::node()[1][self::SwitchLabel] ])
 + count(SwitchLabel[count(following-sibling::node()) = 0])
  < count (SwitchLabel))]

Example(s):

public void bar(int status) {
    switch(status) {
      case CANCELLED:
        doCancelled();
        // break; hm, should this be commented out?
      case NEW:
        doNew();
        // is this really a fall-through?
      case REMOVED:
        doRemoved();
        // what happens if you add another case after this one?
      case OTHER: // empty case - this is interpreted as an intentional fall-through
      case ERROR:
        doErrorHandling();
        break;
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/ImplicitSwitchFallThrough" />

ImportFromSamePackage

Deprecated

Since: PMD 1.02

Priority: Medium (3)

There is no need to import a type that lives in the same package.

This rule is deprecated since PMD 6.34.0. Use the rule UnnecessaryImport from category codestyle instead.

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

Example(s):

package foo;

import foo.Buz;     // no need for this
import foo.*;       // or this

public class Bar{}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/ImportFromSamePackage" />

InstantiationToGetClass

Since: PMD 2.0

Priority: Medium Low (4)

Avoid instantiating an object just to call getClass() on it; use the .class public member instead.

This rule is defined by the following XPath expression:

//PrimarySuffix
 [@Image='getClass']
 [parent::PrimaryExpression
  [PrimaryPrefix/AllocationExpression]
  [count(PrimarySuffix) = 2]
 ]

Example(s):

// replace this
Class c = new String().getClass();

// with this:
Class c = String.class;

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/InstantiationToGetClass" />

InvalidLogMessageFormat

Since: PMD 5.5.0

Priority: Low (5)

Check for messages in slf4j and log4j2 (since 6.19.0) loggers with non matching number of arguments and placeholders.

Since 6.32.0 in addition to parameterized message placeholders ({}) also format specifiers of string formatted messages are supported (%s).

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

Example(s):

LOGGER.error("forget the arg {}");
LOGGER.error("forget the arg %s");
LOGGER.error("too many args {}", "arg1", "arg2");
LOGGER.error("param {}", "arg1", new IllegalStateException("arg")); //The exception is shown separately, so is correct.

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/InvalidLogMessageFormat" />

InvalidSlf4jMessageFormat

Deprecated

This rule has been renamed. Use instead: InvalidLogMessageFormat

Deprecated

Since: PMD 5.5.0

Priority: Low (5)

Check for messages in slf4j and log4j2 (since 6.19.0) loggers with non matching number of arguments and placeholders.

Since 6.32.0 in addition to parameterized message placeholders ({}) also format specifiers of string formatted messages are supported (%s).

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

Example(s):

LOGGER.error("forget the arg {}");
LOGGER.error("forget the arg %s");
LOGGER.error("too many args {}", "arg1", "arg2");
LOGGER.error("param {}", "arg1", new IllegalStateException("arg")); //The exception is shown separately, so is correct.

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/InvalidSlf4jMessageFormat" />

JumbledIncrementer

Since: PMD 1.0

Priority: Medium (3)

Avoid jumbled loop incrementers - its usually a mistake, and is confusing even if intentional.

This rule is defined by the following XPath expression:

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

Example(s):

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

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/JumbledIncrementer" />

JUnitSpelling

Since: PMD 1.0

Priority: Medium (3)

In JUnit 3, the setUp method is used to set up all data entities required in running tests. The tearDown method is used to clean up all data entities required in running tests. You should not misspell method name if you want your test to set up and clean up everything correctly.

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

Example(s):

import junit.framework.*;

public class Foo extends TestCase {
    public void setup() {}    // oops, should be setUp
    public void TearDown() {} // oops, should be tearDown
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/JUnitSpelling" />

JUnitStaticSuite

Since: PMD 1.0

Priority: Medium (3)

The suite() method in a JUnit test needs to be both public and static.

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

Example(s):

import junit.framework.*;

public class Foo extends TestCase {
    public void suite() {}         // oops, should be static
    private static void suite() {} // oops, should be public
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/JUnitStaticSuite" />

LoggerIsNotStaticFinal

Deprecated

Since: PMD 2.0

Priority: Medium High (2)

In most cases, the Logger reference can be declared as static and final.

This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced by ProperLogger.

This rule is defined by the following XPath expression:

//VariableDeclarator
 [parent::FieldDeclaration]
 [../Type/ReferenceType
  /ClassOrInterfaceType[@Image='Logger']
   and
  (..[@Final= false()] or ..[@Static = false()] ) ]

Example(s):

public class Foo{
    Logger log = Logger.getLogger(Foo.class.getName());                 // not recommended

    static final Logger log = Logger.getLogger(Foo.class.getName());    // preferred approach
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/LoggerIsNotStaticFinal" />

MethodWithSameNameAsEnclosingClass

Since: PMD 1.5

Priority: Medium (3)

Non-constructor methods should not have the same name as the enclosing class.

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

Example(s):

public class MyClass {

    public MyClass() {}         // this is OK because it is a constructor

    public void MyClass() {}    // this is bad because it is a method
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/MethodWithSameNameAsEnclosingClass" />

MisplacedNullCheck

Since: PMD 3.5

Priority: Medium (3)

The null check here is misplaced. If the variable is null a NullPointerException will be thrown. Either the check is useless (the variable will never be "null") or it is incorrect.

This rule is defined by the following XPath expression:

//ConditionalAndExpression
  /EqualityExpression
    [@Image = '!=']
    (: one side is null :)
    [PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
    (: other side checks for the variable used somewhere in the first child of conditional and expression :)
    [some $var in preceding-sibling::PrimaryExpression//Name
      [not(ancestor::ConditionalOrExpression/EqualityExpression[@Image = '=='])]
      /@Image
      satisfies starts-with($var, concat(PrimaryExpression/PrimaryPrefix/Name/@Image, '.'))]
  /PrimaryExpression/PrimaryPrefix/Name
|
//ConditionalOrExpression
  /EqualityExpression
    [@Image = '==']
    (: one side is null :)
    [PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
    (: other side checks for the variable used somewhere in the first child of conditional or expression :)
    [some $var in preceding-sibling::PrimaryExpression//Name
      [not(ancestor::ConditionalAndExpression/EqualityExpression[@Image = '!='])]
      /@Image
      satisfies starts-with($var, concat(PrimaryExpression/PrimaryPrefix/Name/@Image, '.'))]
  /PrimaryExpression/PrimaryPrefix/Name

Example(s):

public class Foo {
    void bar() {
        if (a.equals(baz) && a != null) {} // a could be null, misplaced null check

        if (a != null && a.equals(baz)) {} // correct null check
    }
}

public class Foo {
    void bar() {
        if (a.equals(baz) || a == null) {} // a could be null, misplaced null check

        if (a == null || a.equals(baz)) {} // correct null check
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/MisplacedNullCheck" />

MissingBreakInSwitch

Deprecated

This rule has been renamed. Use instead: ImplicitSwitchFallThrough

Deprecated

Since: PMD 3.0

Priority: Medium (3)

Switch statements without break or return statements for each case option may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through.

This rule has been renamed from "MissingBreakInSwitch" with PMD 6.37.0.

This rule is defined by the following XPath expression:

//SwitchStatement
[(count(.//BreakStatement)
 + count(BlockStatement//Statement/ReturnStatement)
 + count(BlockStatement//Statement/ContinueStatement)
 + count(BlockStatement//Statement/ThrowStatement)
 + count(BlockStatement//Statement/IfStatement[@Else= true() and Statement[2][ReturnStatement|ContinueStatement|ThrowStatement]]/Statement[1][ReturnStatement|ContinueStatement|ThrowStatement])
 + count(SwitchLabel[ following-sibling::node()[1][self::SwitchLabel] ])
 + count(SwitchLabel[count(following-sibling::node()) = 0])
  < count (SwitchLabel))]

Example(s):

public void bar(int status) {
    switch(status) {
      case CANCELLED:
        doCancelled();
        // break; hm, should this be commented out?
      case NEW:
        doNew();
        // is this really a fall-through?
      case REMOVED:
        doRemoved();
        // what happens if you add another case after this one?
      case OTHER: // empty case - this is interpreted as an intentional fall-through
      case ERROR:
        doErrorHandling();
        break;
    }
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/MissingBreakInSwitch" />

MissingSerialVersionUID

Since: PMD 3.0

Priority: Medium (3)

Serializable classes should provide a serialVersionUID field. The serialVersionUID field is also needed for abstract base classes. Each individual class in the inheritance chain needs an own serialVersionUID field. See also Should an abstract class have a serialVersionUID.

This rule is defined by the following XPath expression:

//ClassOrInterfaceDeclaration
    [@Interface = false()]
    [count(ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration
        /FieldDeclaration/VariableDeclarator/VariableDeclaratorId[@Name='serialVersionUID']) = 0]
    [(ImplementsList | ExtendsList)/ClassOrInterfaceType[pmd-java:typeIs('java.io.Serializable')]]

Example(s):

public class Foo implements java.io.Serializable {
    String name;
    // Define serialization id to avoid serialization related bugs
    // i.e., public static final long serialVersionUID = 4328743;
}

Use this rule by referencing it:

<rule ref="category/java/errorprone.xml/MissingSerialVersionUID" />

MissingStaticMethodInNonInstantiatableClass

Since: PMD 3.0

Priority: Medium (3)

A class that has private constructors and does not have any static methods or fields cannot be used.

When one of the private constructors is annotated with one of the annotations, then the class is not considered non-instantiatable anymore and no violation will be reported. See the property annotations.

This rule is defined by the following XPath expression:

//ClassOrInterfaceDeclaration[@Nested= false()]
[
  (
    (: at least one constructor :)
    ./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration
    and
    (: only private constructors :)count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration) = count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private= true()])
    and
    (: all constructors must not be annotated :)
    (every $x in $annotations satisfies
      not(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration/
            ../Annotation/MarkerAnnotation/Name[pmd-java:typeIs($x)]))
  )
  and
  (: no static methods :)not(.//MethodDeclaration[@Static= true()])
  and
  (: no (public, package-private, protected) static fields :)not(.//FieldDeclaration[@Private= false()][@Static= true()])
  and
  (: no nested classes, that are public and static, and have no constructors at all or a public constructor :)
  (: and have a method returning the outer class type :)
  (: or the inner class extends the outer class :)not(.//ClassOrInterfaceDeclaration[@Nested= true()]
           [@Public= true()]
           [@Static= true()]
           [not(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration) or ./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Public= true()]]
           [(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/MethodDeclaration
                [@Public= true()]
                [./ResultType/Type/ReferenceType/ClassOrInterfaceType
                    [@Image = //ClassOrInterfaceDeclaration[@Nested= false()]/@SimpleName]
                ]
            ) or (
                ./ExtendsList/ClassOrInterfaceType[@Image = //ClassOrInterfaceDeclaration[@Nested=false()]/@SimpleName]
            )]
        )
]