Rules which enforce generally accepted best practices.
Edit me

AbstractClassWithoutAbstractMethod

Since: PMD 3.0

Priority: Medium (3)

The abstract class does not contain any abstract methods. An abstract class suggests an incomplete implementation, which is to be completed by subclasses implementing the abstract methods. If the class is intended to be used as a base class only (not to be instantiated directly) a protected constructor can be provided prevent direct instantiation.

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

Example(s):

public abstract class Foo {
  void int method1() { ... }
  void int method2() { ... }
  // consider using abstract methods or removing
  // the abstract modifier and adding protected constructors
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AbstractClassWithoutAbstractMethod" />

AccessorClassGeneration

Since: PMD 1.04

Priority: Medium (3)

Instantiation by way of private constructors from outside of the constructor’s class often causes the generation of an accessor. A factory method, or non-privatization of the constructor can eliminate this situation. The generated class file is actually an interface. It gives the accessing class the ability to invoke a new hidden package scope constructor that takes the interface as a supplementary parameter. This turns a private constructor effectively into one with package scope, and is challenging to discern.

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

Example(s):

public class Outer {
 void method(){
  Inner ic = new Inner();//Causes generation of accessor class
 }
 public class Inner {
  private Inner(){}
 }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AccessorClassGeneration" />

AccessorMethodGeneration

Since: PMD 5.5.4

Priority: Medium (3)

When accessing private fields / methods from another class, the Java compiler will generate accessor methods with package-private visibility. This adds overhead, and to the dex method count on Android. This situation can be avoided by changing the visibility of the field / method from private to package-private.

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

Example(s):

public class OuterClass {
    private int counter;
    /* package */ int id;

    public class InnerClass {
        InnerClass() {
            OuterClass.this.counter++; // wrong accessor method will be generated
        }

        public int getOuterClassId() {
            return OuterClass.this.id; // id is package-private, no accessor method needed
        }
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AccessorMethodGeneration" />

ArrayIsStoredDirectly

Since: PMD 2.2

Priority: Medium (3)

Constructors and methods receiving arrays should clone objects and store the copy. This prevents future changes from the user from affecting the original array.

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

Example(s):

public class Foo {
    private String [] x;
        public void foo (String [] param) {
        // Don't do this, make a copy of the array at least
        this.x=param;
    }
}

This rule has the following properties:

Name Default Value Description Multivalued
allowPrivate true If true, allow private methods/constructors to store arrays directly no

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

<rule ref="category/java/bestpractices.xml/ArrayIsStoredDirectly" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/ArrayIsStoredDirectly">
    <properties>
        <property name="allowPrivate" value="true" />
    </properties>
</rule>

AvoidMessageDigestField

Since: PMD 6.18.0

Priority: Medium (3)

Declaring a MessageDigest instance as a field make this instance directly available to multiple threads. Such sharing of MessageDigest instances should be avoided if possible since it leads to wrong results if the access is not synchronized correctly. Just create a new instance and use it locally, where you need it. Creating a new instance is easier than synchronizing access to a shared instance.

This rule is defined by the following XPath expression:

//FieldDeclaration/ClassOrInterfaceType[pmd-java:typeIs('java.security.MessageDigest')]

Example(s):

import java.security.MessageDigest;
public class AvoidMessageDigestFieldExample {
    private final MessageDigest sharedMd;
    public AvoidMessageDigestFieldExample() throws Exception {
        sharedMd = MessageDigest.getInstance("SHA-256");
    }
    public byte[] calculateHashShared(byte[] data) {
        // sharing a MessageDigest like this without synchronizing access
        // might lead to wrong results
        sharedMd.reset();
        sharedMd.update(data);
        return sharedMd.digest();
    }

    // better
    public byte[] calculateHash(byte[] data) throws Exception {
        MessageDigest md = MessageDigest.getInstance("SHA-256");
        md.update(data);
        return md.digest();
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AvoidMessageDigestField" />

AvoidPrintStackTrace

Since: PMD 3.2

Priority: Medium (3)

Avoid printStackTrace(); use a logger call instead.

This rule is defined by the following XPath expression:

//MethodCall[ pmd-java:matchesSig("java.lang.Throwable#printStackTrace()") ]

Example(s):

class Foo {
    void bar() {
        try {
            // do something
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AvoidPrintStackTrace" />

AvoidReassigningCatchVariables

Since: PMD 6.27.0

Priority: Medium (3)

Reassigning exception variables caught in a catch statement should be avoided because of:

1) If it is needed, multi catch can be easily added and code will still compile.

2) Following the principle of least surprise we want to make sure that a variable caught in a catch statement is always the one thrown in a try block.

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

Example(s):

public class Foo {
    public void foo() {
        try {
            // do something
        } catch (Exception e) {
            e = new NullPointerException(); // not recommended
        }

        try {
            // do something
        } catch (MyException | ServerException e) {
            e = new RuntimeException(); // won't compile
        }
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AvoidReassigningCatchVariables" />

AvoidReassigningLoopVariables

Since: PMD 6.11.0

Priority: Medium (3)

Reassigning loop variables can lead to hard-to-find bugs. Prevent or limit how these variables can be changed.

In foreach-loops, configured by the foreachReassign property:

  • deny: Report any reassignment of the loop variable in the loop body. This is the default.
  • allow: Don’t check the loop variable.
  • firstOnly: Report any reassignments of the loop variable, except as the first statement in the loop body. This is useful if some kind of normalization or clean-up of the value before using is permitted, but any other change of the variable is not.

In for-loops, configured by the forReassign property:

  • deny: Report any reassignment of the control variable in the loop body. This is the default.
  • allow: Don’t check the control variable.
  • skip: Report any reassignments of the control variable, except conditional increments/decrements (++, --, +=, -=). This prevents accidental reassignments or unconditional increments of the control variable.

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

Example(s):

public class Foo {
  private void foo() {
    for (String s : listOfStrings()) {
      s = s.trim(); // OK, when foreachReassign is "firstOnly" or "allow"
      doSomethingWith(s);

      s = s.toUpper(); // OK, when foreachReassign is "allow"
      doSomethingElseWith(s);
    }

    for (int i=0; i < 10; i++) {
      if (check(i)) {
        i++; // OK, when forReassign is "skip" or "allow"
      }

      i = 5;  // OK, when forReassign is "allow"

      doSomethingWith(i);
    }
  }
}

This rule has the following properties:

Name Default Value Description Multivalued
foreachReassign deny how/if foreach control variables may be reassigned no
forReassign deny how/if for control variables may be reassigned no

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

<rule ref="category/java/bestpractices.xml/AvoidReassigningLoopVariables" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/AvoidReassigningLoopVariables">
    <properties>
        <property name="foreachReassign" value="deny" />
        <property name="forReassign" value="deny" />
    </properties>
</rule>

AvoidReassigningParameters

Since: PMD 1.0

Priority: Medium High (2)

Reassigning values to incoming parameters is not recommended. Use temporary local variables instead.

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

Example(s):

public class Foo {
  private void foo(String bar) {
    bar = "something else";
  }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AvoidReassigningParameters" />

AvoidStringBufferField

Since: PMD 4.2

Priority: Medium (3)

StringBuffers/StringBuilders can grow considerably, and so may become a source of memory leaks if held within objects with long lifetimes.

This rule is defined by the following XPath expression:

//FieldDeclaration/ClassOrInterfaceType[pmd-java:typeIs('java.lang.StringBuffer') or pmd-java:typeIs('java.lang.StringBuilder')]

Example(s):

public class Foo {
    private StringBuffer buffer;    // potential memory leak as an instance variable;
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/AvoidStringBufferField" />

AvoidUsingHardCodedIP

Since: PMD 4.1

Priority: Medium (3)

Application with hard-coded IP addresses can become impossible to deploy in some cases. Externalizing IP adresses is preferable.

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

Example(s):

public class Foo {
    private String ip = "127.0.0.1";     // not recommended
}

This rule has the following properties:

Name Default Value Description Multivalued
checkAddressTypes IPV4 | IPV6 | IPV4_MAPPED_IPV6 Check for IP address types. yes. Delimiter is ‘|’.

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

<rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP">
    <properties>
        <property name="checkAddressTypes" value="IPV4|IPV6|IPV4_MAPPED_IPV6" />
    </properties>
</rule>

CheckResultSet

Since: PMD 4.1

Priority: Medium (3)

Always check the return values of navigation methods (next, previous, first, last) of a ResultSet. If the value return is ‘false’, it should be handled properly.

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

Example(s):

Statement stat = conn.createStatement();
ResultSet rst = stat.executeQuery("SELECT name FROM person");
rst.next();     // what if it returns false? bad form
String firstName = rst.getString(1);

Statement stat = conn.createStatement();
ResultSet rst = stat.executeQuery("SELECT name FROM person");
if (rst.next()) {    // result is properly examined and used
    String firstName = rst.getString(1);
    } else  {
        // handle missing data
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/CheckResultSet" />

ConstantsInInterface

Since: PMD 5.5

Priority: Medium (3)

Avoid constants in interfaces. Interfaces should define types, constants are implementation details better placed in classes or enums. See Effective Java, item 19.

This rule is defined by the following XPath expression:

//ClassOrInterfaceDeclaration[@Interface = true()][$ignoreIfHasMethods= false() or not(ClassOrInterfaceBody/MethodDeclaration)]/ClassOrInterfaceBody/FieldDeclaration

Example(s):

public interface ConstantInterface {
    public static final int CONST1 = 1; // violation, no fields allowed in interface!
    static final int CONST2 = 1;        // violation, no fields allowed in interface!
    final int CONST3 = 1;               // violation, no fields allowed in interface!
    int CONST4 = 1;                     // violation, no fields allowed in interface!
}

// with ignoreIfHasMethods = false
public interface AnotherConstantInterface {
    public static final int CONST1 = 1; // violation, no fields allowed in interface!

    int anyMethod();
}

// with ignoreIfHasMethods = true
public interface YetAnotherConstantInterface {
    public static final int CONST1 = 1; // no violation

    int anyMethod();
}

This rule has the following properties:

Name Default Value Description Multivalued
ignoreIfHasMethods true Whether to ignore constants in interfaces if the interface defines any methods no

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

<rule ref="category/java/bestpractices.xml/ConstantsInInterface" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/ConstantsInInterface">
    <properties>
        <property name="ignoreIfHasMethods" value="true" />
    </properties>
</rule>

DefaultLabelNotLastInSwitchStmt

Since: PMD 1.5

Priority: Medium (3)

By convention, the default label should be the last label in a switch statement.

This rule is defined by the following XPath expression:

//SwitchLabel[@Default = true() and not(.. is ../../*[last()])]

Example(s):

public class Foo {
  void bar(int a) {
   switch (a) {
    case 1:  // do something
       break;
    default:  // the default case should be last, by convention
       break;
    case 2:
       break;
   }
  }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/DefaultLabelNotLastInSwitchStmt" />

DoubleBraceInitialization

Since: PMD 6.16.0

Priority: Medium (3)

Double brace initialisation is a pattern to initialise eg collections concisely. But it implicitly generates a new .class file, and the object holds a strong reference to the enclosing object. For those reasons, it is preferable to initialize the object normally, even though it’s verbose.

This rule counts any anonymous class which only has a single initializer as an instance of double-brace initialization. There is currently no way to find out whether a method called in the initializer is not accessible from outside the anonymous class, and those legit cases should be suppressed for the time being.

This rule is defined by the following XPath expression:

//ConstructorCall/AnonymousClassDeclaration/ClassOrInterfaceBody[count(*)=1]/Initializer[@Static=false()]

Example(s):

// this is double-brace initialization
return new ArrayList<String>(){{
    add("a");
    add("b");
    add("c");
}};

// the better way is to not create an anonymous class:
List<String> a = new ArrayList<>();
a.add("a");
a.add("b");
a.add("c");
return a;

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/DoubleBraceInitialization" />

ForLoopCanBeForeach

Since: PMD 6.0.0

Priority: Medium (3)

Minimum Language Version: Java 1.5

Reports loops that can be safely replaced with the foreach syntax. The rule considers loops over lists, arrays and iterators. A loop is safe to replace if it only uses the index variable to access an element of the list or array, only has one update statement, and loops through every element of the list or array left to right.

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

Example(s):

public class MyClass {
  void loop(List<String> l) {
    for (int i = 0; i < l.size(); i++) { // pre Java 1.5
      System.out.println(l.get(i));
    }

    for (String s : l) {        // post Java 1.5
      System.out.println(s);
    }
  }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/ForLoopCanBeForeach" />

ForLoopVariableCount

Since: PMD 6.11.0

Priority: Medium (3)

Having a lot of control variables in a ‘for’ loop makes it harder to see what range of values the loop iterates over. By default this rule allows a regular ‘for’ loop with only one variable.

This rule is defined by the following XPath expression:

//ForInit/LocalVariableDeclaration[count(VariableDeclarator) > $maximumVariables]

Example(s):

// this will be reported with the default setting of at most one control variable in a for loop
for (int i = 0, j = 0; i < 10; i++, j += 2) {
   foo();

This rule has the following properties:

Name Default Value Description Multivalued
maximumVariables 1 A regular for statement will have 1 control variable no

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

<rule ref="category/java/bestpractices.xml/ForLoopVariableCount" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/ForLoopVariableCount">
    <properties>
        <property name="maximumVariables" value="1" />
    </properties>
</rule>

GuardLogStatement

Since: PMD 5.1.0

Priority: Medium High (2)

Whenever using a log level, one should check if the loglevel is actually enabled, or otherwise skip the associate String creation and manipulation.

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

Example(s):

// Add this for performance
    if (log.isDebugEnabled() { ...
        log.debug("log something" + " and " + "concat strings");

This rule has the following properties:

Name Default Value Description Multivalued
logLevels trace , debug , info , warn , error , log , finest , finer , fine , info , warning , severe LogLevels to guard yes. Delimiter is ‘,’.
guardsMethods isTraceEnabled , isDebugEnabled , isInfoEnabled , isWarnEnabled , isErrorEnabled , isLoggable Method use to guard the log statement yes. Delimiter is ‘,’.

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

<rule ref="category/java/bestpractices.xml/GuardLogStatement" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/GuardLogStatement">
    <properties>
        <property name="logLevels" value="trace,debug,info,warn,error,log,finest,finer,fine,info,warning,severe" />
        <property name="guardsMethods" value="isTraceEnabled,isDebugEnabled,isInfoEnabled,isWarnEnabled,isErrorEnabled,isLoggable" />
    </properties>
</rule>

JUnit4SuitesShouldUseSuiteAnnotation

Since: PMD 4.0

Priority: Medium (3)

In JUnit 3, test suites are indicated by the suite() method. In JUnit 4, suites are indicated through the @RunWith(Suite.class) annotation.

This rule is defined by the following XPath expression:

//ClassOrInterfaceBodyDeclaration
[MethodDeclaration[@Name='suite']/ResultType/Type/ReferenceType/ClassOrInterfaceType[pmd-java:typeIs('junit.framework.Test')]]
[not(MethodDeclaration/Block/BlockStatement/Statement/ReturnStatement//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.JUnit4TestAdapter')])]

Example(s):

public class BadExample extends TestCase{

    public static Test suite(){
        return new Suite();
    }
}

@RunWith(Suite.class)
@SuiteClasses( { TestOne.class, TestTwo.class })
public class GoodTest {
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/JUnit4SuitesShouldUseSuiteAnnotation" />

JUnit4TestShouldUseAfterAnnotation

Since: PMD 4.0

Priority: Medium (3)

In JUnit 3, the tearDown method was used to clean up all data entities required in running tests. JUnit 4 skips the tearDown method and executes all methods annotated with @After after running each test. JUnit 5 introduced @AfterEach and @AfterAll annotations to execute methods after each test or after all tests in the class, respectively.

This rule is defined by the following XPath expression:

//ClassOrInterfaceBodyDeclaration
    [MethodDeclaration[@Name='tearDown']]
    [not(Annotation/*/Name[
           pmd-java:typeIs('org.junit.After')
        or pmd-java:typeIs('org.junit.jupiter.api.AfterEach')
        or pmd-java:typeIs('org.junit.jupiter.api.AfterAll')
        or pmd-java:typeIs('org.testng.annotations.AfterMethod')])]

Example(s):

public class MyTest {
    public void tearDown() {
        bad();
    }
}
public class MyTest2 {
    @After public void tearDown() {
        good();
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/JUnit4TestShouldUseAfterAnnotation" />

JUnit4TestShouldUseBeforeAnnotation

Since: PMD 4.0

Priority: Medium (3)

In JUnit 3, the setUp method was used to set up all data entities required in running tests. JUnit 4 skips the setUp method and executes all methods annotated with @Before before all tests. JUnit 5 introduced @BeforeEach and @BeforeAll annotations to execute methods before each test or before all tests in the class, respectively.

This rule is defined by the following XPath expression:

//ClassOrInterfaceBodyDeclaration
    [MethodDeclaration[@Name='setUp']]
    [not(Annotation/*/Name[
           pmd-java:typeIs('org.junit.Before')
        or pmd-java:typeIs('org.junit.jupiter.api.BeforeEach')
        or pmd-java:typeIs('org.junit.jupiter.api.BeforeAll')
        or pmd-java:typeIs('org.testng.annotations.BeforeMethod')])]

Example(s):

public class MyTest {
    public void setUp() {
        bad();
    }
}
public class MyTest2 {
    @Before public void setUp() {
        good();
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/JUnit4TestShouldUseBeforeAnnotation" />

JUnit4TestShouldUseTestAnnotation

Since: PMD 4.0

Priority: Medium (3)

In JUnit 3, the framework executed all methods which started with the word test as a unit test. In JUnit 4, only methods annotated with the @Test annotation are executed. In JUnit 5, one of the following annotations should be used for tests: @Test, @RepeatedTest, @TestFactory, @TestTemplate or @ParameterizedTest.

This rule is defined by the following XPath expression:

//ClassOrInterfaceDeclaration[
       matches(@SimpleName, $testClassPattern)
        or ExtendsList/ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')]]

    /ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration[MethodDeclaration[@Public=true() and starts-with(@Name, 'test')]]
    [not(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')
    ])]

Example(s):

public class MyTest {
    public void testBad() {
        doSomething();
    }

    @Test
    public void testGood() {
        doSomething();
    }
}

This rule has the following properties:

Name Default Value Description Multivalued
testClassPattern Test The regex pattern used to identify test classes no

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

<rule ref="category/java/bestpractices.xml/JUnit4TestShouldUseTestAnnotation" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/JUnit4TestShouldUseTestAnnotation">
    <properties>
        <property name="testClassPattern" value="Test" />
    </properties>
</rule>

JUnitAssertionsShouldIncludeMessage

Since: PMD 1.04

Priority: Medium (3)

JUnit assertions should include an informative message - i.e., use the three-argument version of assertEquals(), not the two-argument version.

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

Example(s):

public class Foo extends TestCase {
    public void testSomething() {
        assertEquals("foo", "bar");
        // Use the form:
        // assertEquals("Foo does not equals bar", "foo", "bar");
        // instead
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/JUnitAssertionsShouldIncludeMessage" />

JUnitTestContainsTooManyAsserts

Since: PMD 5.0

Priority: Medium (3)

Unit tests should not contain too many asserts. Many asserts are indicative of a complex test, for which it is harder to verify correctness. Consider breaking the test scenario into multiple, shorter test scenarios. Customize the maximum number of assertions used by this Rule to suit your needs.

This rule checks for JUnit4, JUnit5 and TestNG Tests, as well as methods starting with "test".

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

Example(s):

public class MyTestCase extends TestCase {
    // Ok
    public void testMyCaseWithOneAssert() {
        boolean myVar = false;
        assertFalse("should be false", myVar);
    }

    // Bad, too many asserts (assuming max=1)
    public void testMyCaseWithMoreAsserts() {
        boolean myVar = false;
        assertFalse("myVar should be false", myVar);
        assertEquals("should equals false", false, myVar);
    }
}

This rule has the following properties:

Name Default Value Description Multivalued
maximumAsserts 1 Maximum number of assert calls in a test method no

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

<rule ref="category/java/bestpractices.xml/JUnitTestContainsTooManyAsserts" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/JUnitTestContainsTooManyAsserts">
    <properties>
        <property name="maximumAsserts" value="1" />
    </properties>
</rule>

JUnitTestsShouldIncludeAssert

Since: PMD 2.0

Priority: Medium (3)

JUnit tests should include at least one assertion. This makes the tests more robust, and using assert with messages provide the developer a clearer idea of what the test does.

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

Example(s):

public class Foo extends TestCase {
   public void testSomething() {
      Bar b = findBar();
   // This is better than having a NullPointerException
   // assertNotNull("bar not found", b);
   b.work();
   }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/JUnitTestsShouldIncludeAssert" />

JUnitUseExpected

Since: PMD 4.0

Priority: Medium (3)

In JUnit4, use the @Test(expected) annotation to denote tests that should throw exceptions.

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

Example(s):

public class MyTest {
    @Test
    public void testBad() {
        try {
            doSomething();
            fail("should have thrown an exception");
        } catch (Exception e) {
        }
    }

    @Test(expected=Exception.class)
    public void testGood() {
        doSomething();
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/JUnitUseExpected" />

LiteralsFirstInComparisons

Since: PMD 6.24.0

Priority: Medium (3)

Position literals first in all String comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. Note that switching literal positions for compareTo and compareToIgnoreCase may change the result, see examples.

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

Example(s):

class Foo {
    boolean bar(String x) {
        return x.equals("2"); // should be "2".equals(x)
    }
    boolean bar(String x) {
        return x.equalsIgnoreCase("2"); // should be "2".equalsIgnoreCase(x)
    }
    boolean bar(String x) {
        return (x.compareTo("bar") > 0); // should be: "bar".compareTo(x) < 0
    }
    boolean bar(String x) {
        return (x.compareToIgnoreCase("bar") > 0); // should be: "bar".compareToIgnoreCase(x) < 0
    }
    boolean bar(String x) {
        return x.contentEquals("bar"); // should be "bar".contentEquals(x)
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/LiteralsFirstInComparisons" />

LooseCoupling

Since: PMD 0.7

Priority: Medium (3)

The use of implementation types (i.e., HashSet) as object references limits your ability to use alternate implementations in the future as requirements change. Whenever available, referencing objects by their interface types (i.e, Set) provides much more flexibility.

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

Example(s):

import java.util.ArrayList;
import java.util.HashSet;

public class Bar {
    // sub-optimal approach
    private ArrayList<SomeType> list = new ArrayList<>();

    public HashSet<SomeType> getFoo() {
        return new HashSet<SomeType>();
    }

    // preferred approach
    private List<SomeType> list = new ArrayList<>();

    public Set<SomeType> getFoo() {
        return new HashSet<SomeType>();
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/LooseCoupling" />

MethodReturnsInternalArray

Since: PMD 2.2

Priority: Medium (3)

Exposing internal arrays to the caller violates object encapsulation since elements can be removed or replaced outside of the object that owns it. It is safer to return a copy of the array.

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

Example(s):

public class SecureSystem {
    UserData [] ud;
    public UserData [] getUserData() {
        // Don't return directly the internal array, return a copy
        return ud;
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/MethodReturnsInternalArray" />

MissingOverride

Since: PMD 6.2.0

Priority: Medium (3)

Minimum Language Version: Java 1.5

Annotating overridden methods with @Override ensures at compile time that the method really overrides one, which helps refactoring and clarifies intent.

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

Example(s):

public class Foo implements Runnable {
                // This method is overridden, and should have an @Override annotation
                public void run() {

                }
            }

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/MissingOverride" />

OneDeclarationPerLine

Since: PMD 5.0

Priority: Medium Low (4)

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

This rule is defined by the following XPath expression:

//LocalVariableDeclaration
   [not(parent::ForInit)]
   [count(VariableDeclarator) > 1]
   [$strictMode or count(distinct-values(VariableDeclarator/@BeginLine)) != count(VariableDeclarator)]
|
//FieldDeclaration
   [count(VariableDeclarator) > 1]
   [$strictMode or count(distinct-values(VariableDeclarator/@BeginLine)) != count(VariableDeclarator)]

Example(s):

String name;            // separate declarations
String lastname;

String name, lastname;  // combined declaration, a violation

String name,
       lastname;        // combined declaration on multiple lines, no violation by default.
                        // Set property strictMode to true to mark this as violation.

This rule has the following properties:

Name Default Value Description Multivalued
strictMode false If true, mark combined declaration even if the declarations are on separate lines. no

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

<rule ref="category/java/bestpractices.xml/OneDeclarationPerLine" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/OneDeclarationPerLine">
    <properties>
        <property name="strictMode" value="false" />
    </properties>
</rule>

PreserveStackTrace

Since: PMD 3.7

Priority: Medium (3)

Throwing a new exception from a catch block without passing the original exception into the new exception will cause the original stack trace to be lost making it difficult to debug effectively.

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

Example(s):

public class Foo {
    void good() {
        try{
            Integer.parseInt("a");
        } catch (Exception e) {
            throw new Exception(e); // first possibility to create exception chain
        }
        try {
            Integer.parseInt("a");
        } catch (Exception e) {
            throw (IllegalStateException)new IllegalStateException().initCause(e); // second possibility to create exception chain.
        }
    }
    void bad() {
        try{
            Integer.parseInt("a");
        } catch (Exception e) {
            throw new Exception(e.getMessage());
        }
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/PreserveStackTrace" />

ReplaceEnumerationWithIterator

Since: PMD 3.4

Priority: Medium (3)

Consider replacing Enumeration usages with the newer java.util.Iterator

This rule is defined by the following XPath expression:

//ImplementsList/ClassOrInterfaceType[@Image='Enumeration']

Example(s):

public class Foo implements Enumeration {
    private int x = 42;
    public boolean hasMoreElements() {
        return true;
    }
    public Object nextElement() {
        return String.valueOf(i++);
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/ReplaceEnumerationWithIterator" />

ReplaceHashtableWithMap

Since: PMD 3.4

Priority: Medium (3)

Consider replacing Hashtable usage with the newer java.util.Map if thread safety is not required.

This rule is defined by the following XPath expression:

//Type/ReferenceType/ClassOrInterfaceType[@Image='Hashtable']

Example(s):

public class Foo {
    void bar() {
        Hashtable h = new Hashtable();
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/ReplaceHashtableWithMap" />

ReplaceVectorWithList

Since: PMD 3.4

Priority: Medium (3)

Consider replacing Vector usages with the newer java.util.ArrayList if expensive thread-safe operations are not required.

This rule is defined by the following XPath expression:

//Type/ReferenceType/ClassOrInterfaceType[@Image='Vector']

Example(s):

public class Foo {
    void bar() {
        Vector v = new Vector();
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/ReplaceVectorWithList" />

SwitchStmtsShouldHaveDefault

Since: PMD 1.0

Priority: Medium (3)

All switch statements should include a default option to catch any unspecified values.

This rule is defined by the following XPath expression:

//SwitchStatement[@DefaultCase = false() and @ExhaustiveEnumSwitch = false()]

Example(s):

public void bar() {
    int x = 2;
    switch (x) {
      case 1: int j = 6;
      case 2: int j = 8;
          // missing default: here
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/SwitchStmtsShouldHaveDefault" />

SystemPrintln

Since: PMD 2.1

Priority: Medium High (2)

References to System.(out|err).print are usually intended for debugging purposes and can remain in the codebase even in production code. By using a logger one can enable/disable this behaviour at will (and by priority) and avoid clogging the Standard out log.

This rule is defined by the following XPath expression:

//MethodCall[ starts-with(@MethodName, 'print') ]
  /FieldAccess[ @Name = ('err', 'out') ]
  /TypeExpression[ pmd-java:typeIsExactly('java.lang.System') ]

Example(s):

class Foo{
    Logger log = Logger.getLogger(Foo.class.getName());
    public void testA () {
        System.out.println("Entering test");
        // Better use this
        log.fine("Entering test");
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/SystemPrintln" />

UnusedAssignment

Since: PMD 6.26.0

Priority: Medium (3)

Reports assignments to variables that are never used before the variable is overwritten, or goes out of scope. Unused assignments are those for which

  1. The variable is never read after the assignment, or
  2. The assigned value is always overwritten by other assignments before the next read of the variable.

The rule doesn’t consider assignments to fields except for those of this in a constructor, or static fields of the current class in static initializers.

The rule may be suppressed with the standard @SuppressWarnings("unused") tag.

The rule subsumes UnusedLocalVariable, and UnusedFormalParameter. Those violations are filtered out by default, in case you already have enabled those rules, but may be enabled with the property reportUnusedVariables. Variables whose name starts with ignored or unused are filtered out, as is standard practice for exceptions.

Limitations:

  • The rule currently cannot know which method calls throw exceptions, or which exceptions they throw. In the body of a try block, every method or constructor call is assumed to throw. This may cause false-negatives. The only other language construct that is assumed to throw is the throw statement, in particular, things like assert statements, or NullPointerExceptions on dereference are ignored.
  • The rule cannot resolve assignments across constructors, when they’re called with the special this(...) syntax. This may cause false-negatives.

Both of those limitations may be partly relaxed in PMD 7.

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

Example(s):

class A {
                // this field initializer is redundant,
                // it is always overwritten in the constructor
                int f = 1;

                A(int f) {
                    this.f = f;
                }
            }
class B {

    int method(int i, int j) {
        // this initializer is redundant,
        // it is overwritten in all branches of the `if`
        int k = 0;

        // Both the assignments to k are unused, because k is
        // not read after the if/else
        // This may hide a bug: the programmer probably wanted to return k
        if (i < j)
            k = i;
        else
            k = j;

        return j;
    }

}
class C {

    int method() {
        int i = 0;

        checkSomething(++i);
        checkSomething(++i);
        checkSomething(++i);
        checkSomething(++i);

        // That last increment is not reported unless
        // the property `checkUnusedPrefixIncrement` is
        // set to `true`
        // Technically it could be written (i+1), but it
        // is not very important
    }

}
class C {

    // variables that are truly unused (at most assigned to, but never accessed)
    // are only reported if property `reportUnusedVariables` is true

    void method(int param) { } // for example this method parameter

    // even then, you can suppress the violation with an annotation:

    void method(@SuppressWarning("unused") int param) { } // no violation, even if `reportUnusedVariables` is true

    // For catch parameters, or for resources which don't need to be used explicitly,
    // you can give a name that starts with "ignored" to ignore such warnings

    {
        try (Something ignored = Something.create()) {
            // even if ignored is unused, it won't be flagged
            // its purpose might be to side-effect in the create/close routines

        } catch (Exception e) { // this is unused and will cause a warning if `reportUnusedVariables` is true
            // you should choose a name that starts with "ignored"
            return;
        }
    }

}

This rule has the following properties:

Name Default Value Description Multivalued
checkUnusedPrefixIncrement false Report expressions like ++i that may be replaced with (i + 1) no
reportUnusedVariables false Report variables that are only initialized, and never read at all. The rule UnusedVariable already cares for that, but you can enable it if needed no

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

<rule ref="category/java/bestpractices.xml/UnusedAssignment" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/UnusedAssignment">
    <properties>
        <property name="checkUnusedPrefixIncrement" value="false" />
        <property name="reportUnusedVariables" value="false" />
    </properties>
</rule>

UnusedFormalParameter

Since: PMD 0.8

Priority: Medium (3)

Reports parameters of methods and constructors that are not referenced them in the method body. Parameters whose name starts with ignored or unused are filtered out.

Removing unused formal parameters from public methods could cause a ripple effect through the code base. Hence, by default, this rule only considers private methods. To include non-private methods, set the checkAll property to true.

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

Example(s):

public class Foo {
    private void bar(String howdy) {
        // howdy is not used
    }
}

This rule has the following properties:

Name Default Value Description Multivalued
checkAll false Check all methods, including non-private ones no

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

<rule ref="category/java/bestpractices.xml/UnusedFormalParameter" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/UnusedFormalParameter">
    <properties>
        <property name="checkAll" value="false" />
    </properties>
</rule>

UnusedImports

Since: PMD 1.0

Priority: Medium Low (4)

Avoid unused import statements to prevent unwanted dependencies. This rule will also find unused on demand imports, i.e. import com.foo.*.

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

Example(s):

import java.io.File;  // not referenced or required
import java.util.*;   // not referenced or required

public class Foo {}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UnusedImports" />

UnusedLocalVariable

Since: PMD 0.1

Priority: Medium (3)

Detects when a local variable is declared and/or assigned, but not used. Variables whose name starts with ignored or unused are filtered out.

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

Example(s):

public class Foo {
    public void doSomething() {
        int i = 5; // Unused
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UnusedLocalVariable" />

UnusedPrivateField

Since: PMD 0.1

Priority: Medium (3)

Detects when a private field is declared and/or assigned a value, but not used.

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

Example(s):

public class Something {
    private static int FOO = 2; // Unused
    private int i = 5; // Unused
    private int j = 6;
    public int addOne() {
        return j++;
    }
}

This rule has the following properties:

Name Default Value Description Multivalued
ignoredAnnotations lombok.Setter | lombok.Getter | lombok.Builder | lombok.Data | lombok.RequiredArgsConstructor | lombok.AllArgsConstructor | lombok.Value | lombok.NoArgsConstructor | java.lang.Deprecated | javafx.fxml.FXML | lombok.experimental.Delegate | lombok.EqualsAndHashCode Fully qualified names of the annotation types that should be ignored by this rule yes. Delimiter is ‘|’.

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

<rule ref="category/java/bestpractices.xml/UnusedPrivateField" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/UnusedPrivateField">
    <properties>
        <property name="ignoredAnnotations" value="lombok.Setter|lombok.Getter|lombok.Builder|lombok.Data|lombok.RequiredArgsConstructor|lombok.AllArgsConstructor|lombok.Value|lombok.NoArgsConstructor|java.lang.Deprecated|javafx.fxml.FXML|lombok.experimental.Delegate|lombok.EqualsAndHashCode" />
    </properties>
</rule>

UnusedPrivateMethod

Since: PMD 0.7

Priority: Medium (3)

Unused Private Method detects when a private method is declared but is unused.

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

Example(s):

public class Something {
    private void foo() {} // unused
}

This rule has the following properties:

Name Default Value Description Multivalued
ignoredAnnotations java.lang.Deprecated Fully qualified names of the annotation types that should be ignored by this rule yes. Delimiter is ‘|’.

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

<rule ref="category/java/bestpractices.xml/UnusedPrivateMethod" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/UnusedPrivateMethod">
    <properties>
        <property name="ignoredAnnotations" value="java.lang.Deprecated" />
    </properties>
</rule>

UseAssertEqualsInsteadOfAssertTrue

Since: PMD 3.1

Priority: Medium (3)

This rule detects JUnit assertions in object equality. These assertions should be made by more specific methods, like assertEquals.

This rule is defined by the following XPath expression:

// MethodCall[ pmd-java:matchesSig('org.junit.Assert#assertTrue(_*)')
            or pmd-java:matchesSig('org.junit.jupiter.api.Assertions#assertTrue(_*)')
            or pmd-java:matchesSig('junit.framework.TestCase#assertTrue(_*)') ]
             [ArgumentList/*[last()][self::MethodCall[pmd-java:matchesSig('_#equals(java.lang.Object)')]]]

Example(s):

public class FooTest extends TestCase {
    void testCode() {
        Object a, b;
        assertTrue(a.equals(b));                    // bad usage
        assertEquals("a should equals b", a, b);    // good usage
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UseAssertEqualsInsteadOfAssertTrue" />

UseAssertNullInsteadOfAssertTrue

Since: PMD 3.5

Priority: Medium (3)

This rule detects JUnit assertions in object references equality. These assertions should be made by more specific methods, like assertNull, assertNotNull.

This rule is defined by the following XPath expression:

// MethodCall[ @MethodName = ('assertTrue', 'assertFalse')
       and (pmd-java:matchesSig('org.junit.Assert#_(_*)')
         or pmd-java:matchesSig('org.junit.jupiter.api.Assertions#_(_*)')
         or pmd-java:matchesSig('junit.framework.TestCase#_(_*)'))
      ]
      [ArgumentList/*[last()][self::InfixExpression[@Operator = ("==", "!=") and NullLiteral]]]

Example(s):

public class FooTest extends TestCase {
    void testCode() {
        Object a = doSomething();
        assertTrue(a==null);    // bad usage
        assertNull(a);          // good usage
        assertTrue(a != null);  // bad usage
        assertNotNull(a);       // good usage
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UseAssertNullInsteadOfAssertTrue" />

UseAssertSameInsteadOfAssertTrue

Since: PMD 3.1

Priority: Medium (3)

This rule detects JUnit assertions in object references equality. These assertions should be made by more specific methods, like assertSame, assertNotSame.

This rule is defined by the following XPath expression:

// MethodCall[ @MethodName = ('assertTrue', 'assertFalse')
       and (pmd-java:matchesSig('org.junit.Assert#_(_*)')
         or pmd-java:matchesSig('org.junit.jupiter.api.Assertions#_(_*)')
         or pmd-java:matchesSig('junit.framework.TestCase#_(_*)'))
      ]
      [ArgumentList/*[last()][self::InfixExpression[@Operator = ("==", "!=") and not(NullLiteral)]]]

Example(s):

public class FooTest extends TestCase {
    void testCode() {
        Object a, b;
        assertTrue(a == b); // bad usage
        assertSame(a, b);   // good usage
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UseAssertSameInsteadOfAssertTrue" />

UseAssertTrueInsteadOfAssertEquals

Since: PMD 5.0

Priority: Medium (3)

When asserting a value is the same as a literal or Boxed boolean, use assertTrue/assertFalse, instead of assertEquals.

This rule is defined by the following XPath expression:

// MethodCall[ @MethodName = 'assertEquals'
       and (pmd-java:matchesSig('org.junit.Assert#_(_*)')
         or pmd-java:matchesSig('org.junit.jupiter.api.Assertions#_(_*)')
         or pmd-java:matchesSig('junit.framework.TestCase#_(_*)'))
      ]
      [ArgumentList/*[self::BooleanLiteral or self::FieldAccess[@Name=("TRUE", "FALSE") and pmd-java:typeIs('java.lang.Boolean')]]]

Example(s):

public class MyTestCase extends TestCase {
    public void testMyCase() {
        boolean myVar = true;
        // Ok
        assertTrue("myVar is true", myVar);
        // Bad
        assertEquals("myVar is true", true, myVar);
        // Bad
        assertEquals("myVar is false", false, myVar);
        // Bad
        assertEquals("myVar is true", Boolean.TRUE, myVar);
        // Bad
        assertEquals("myVar is false", Boolean.FALSE, myVar);
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UseAssertTrueInsteadOfAssertEquals" />

UseCollectionIsEmpty

Since: PMD 3.9

Priority: Medium (3)

The isEmpty() method on java.util.Collection is provided to determine if a collection has any elements. Comparing the value of size() to 0 does not convey intent as well as the isEmpty() method.

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

Example(s):

public class Foo {
    void good() {
        List foo = getList();
        if (foo.isEmpty()) {
            // blah
        }
    }

    void bad() {
        List foo = getList();
        if (foo.size() == 0) {
            // blah
        }
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty" />

UseTryWithResources

Since: PMD 6.12.0

Priority: Medium (3)

Minimum Language Version: Java 1.7

Java 7 introduced the try-with-resources statement. This statement ensures that each resource is closed at the end of the statement. It avoids the need of explicitly closing the resources in a finally block. Additionally exceptions are better handled: If an exception occurred both in the try block and finally block, then the exception from the try block was suppressed. With the try-with-resources statement, the exception thrown from the try-block is preserved.

This rule is defined by the following XPath expression:

//TryStatement[FinallyStatement//Name[
    tokenize(@Image, '\.')[last()] = $closeMethods
][
    pmd-java:typeIs('java.lang.AutoCloseable')
    or
    ../../PrimarySuffix/Arguments[@ArgumentCount = 1]//PrimaryPrefix[pmd-java:typeIs('java.lang.AutoCloseable')]
]]

Example(s):

public class TryWithResources {
    public void run() {
        InputStream in = null;
        try {
            in = openInputStream();
            int i = in.read();
        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            try {
                if (in != null) in.close();
            } catch (IOException ignored) {
                // ignored
            }
        }

        // better use try-with-resources
        try (InputStream in2 = openInputStream()) {
            int i = in2.read();
        }
    }
}

This rule has the following properties:

Name Default Value Description Multivalued
closeMethods close , closeQuietly Method names in finally block, which trigger this rule yes. Delimiter is ‘,’.

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

<rule ref="category/java/bestpractices.xml/UseTryWithResources" />

Use this rule and customize it:

<rule ref="category/java/bestpractices.xml/UseTryWithResources">
    <properties>
        <property name="closeMethods" value="close,closeQuietly" />
    </properties>
</rule>

UseVarargs

Since: PMD 5.0

Priority: Medium Low (4)

Minimum Language Version: Java 1.5

Java 5 introduced the varargs parameter declaration for methods and constructors. This syntactic sugar provides flexibility for users of these methods and constructors, allowing them to avoid having to deal with the creation of an array.

This rule is defined by the following XPath expression:

//FormalParameters/FormalParameter
    [position()=last()]
    [VariableDeclaratorId/@ArrayType=true()]
    [@Varargs=false()]
    [not (./Type[@ArrayType=true()]/ReferenceType[PrimitiveType[@Image='byte']])]
    [not (./Type/ReferenceType[ClassOrInterfaceType[@Image='Byte']])]
    [not (./Type/PrimitiveType[@Image='byte'])]
    [not (ancestor::MethodDeclaration/preceding-sibling::Annotation/*/Name[@Image='Override'])]
    [not(
         ancestor::MethodDeclaration
            [    @Public=true()
             and @Static=true()
             and child::ResultType[@Void=true()]
             and @Name = 'main'
             and @Arity = 1
            ]
         (: Type of the formal parameter here. :)
         and pmd-java:typeIs('java.lang.String[]')
    )]

Example(s):

public class Foo {
    public void foo(String s, Object[] args) {
        // Do something here...
    }

    public void bar(String s, Object... args) {
        // Ahh, varargs tastes much better...
    }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/UseVarargs" />

WhileLoopWithLiteralBoolean

Since: PMD 6.13.0

Priority: Medium (3)

do {} while (true); requires reading the end of the statement before it is apparent that it loops forever, whereas while (true) {} is easier to understand.

do {} while (false); is redundant, and if an inner variable scope is required, a block {} is sufficient.

while (false) {} will never execute the block and can be removed in its entirety.

This rule is defined by the following XPath expression:

//DoStatement/BooleanLiteral
| //WhileStatement/BooleanLiteral[@True = false()]

Example(s):

public class Example {
  {
    while (true) { } // allowed
    while (false) { } // disallowed
    do { } while (true); // disallowed
    do { } while (false); // disallowed
  }
}

Use this rule by referencing it:

<rule ref="category/java/bestpractices.xml/WhileLoopWithLiteralBoolean" />