# Best Practices

Rules which enforce generally accepted best practices.
Table of Contents

## 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 to 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)

Maximum Language Version: Java 10

Instantiation by way of private constructors from outside 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.

Note: This rule is only executed for Java 10 or lower. Since Java 11, JEP 181: Nest-Based Access Control has been implemented. This means that in Java 11 and above accessor classes are not generated anymore.

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)

Maximum Language Version: Java 10

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.

Note: This rule is only executed for Java 10 or lower. Since Java 11, JEP 181: Nest-Based Access Control has been implemented. This means that in Java 11 and above accessor classes are not generated anymore.

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
--- | --- | ---
allowPrivate | true | If true, allow private methods/constructors to store arrays directly

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/ClassType[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
--- | --- | ---
foreachReassign | deny | how/if foreach control variables may be reassigned One of: `allow`, `deny`, `firstOnly`
forReassign | deny | how/if for control variables may be reassigned One of: `allow`, `deny`, `skip`

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 of a method or constructor is not recommended, as this can make the code more difficult to understand. The code is often read with the assumption that parameter values don’t change and an assignment violates therefore the principle of least astonishment. This is especially a problem if the parameter is documented e.g. in the method’s javadoc and the new content differs from the original documented content.

Use temporary local variables instead. This allows you to assign a new name, which makes the code better understandable.

Note that this rule considers both methods and constructors. If there are multiple assignments for a formal parameter, then only the first assignment is reported.

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

Example(s):

    public class Hello {
      private void greet(String name) {
        name = name.trim();
        System.out.println("Hello " + name);

        // preferred
        String trimmedName = name.trim();
        System.out.println("Hello " + trimmedName);
      }
    }

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/ClassType[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 addresses 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
--- | --- | ---
checkAddressTypes | ipv4 , ipv6 , ipv4MappedIpv6 | Check for IP address types. Zero or more of: `ipv4MappedIpv6`, `ipv4`, `ipv6`

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,ipv4MappedIpv6" />
        </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)

Using constants in interfaces is a bad practice. Interfaces define types, constants are implementation details better placed in classes or enums. If the constants are best viewed as members of an enumerated type, you should export them with an enum type. For other scenarios, consider using a utility class. See Effective Java’s ‘Use interfaces only to define types’.

This rule is defined by the following XPath expression:

    //ClassDeclaration[@Interface = true()][$ignoreIfHasMethods= false() or not(ClassBody/MethodDeclaration)]/ClassBody/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
--- | --- | ---
ignoreIfHasMethods | true | Whether to ignore constants in interfaces if the interface defines any methods

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>

## DefaultLabelNotLastInSwitch

Since: PMD 1.5

Priority: Medium (3)

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

Note: This rule has been renamed from "DefaultLabelNotLastInSwitchStmt" with PMD 7.7.0.

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/DefaultLabelNotLastInSwitch" />

## DefaultLabelNotLastInSwitchStmt

Deprecated

This rule has been renamed. Use instead: DefaultLabelNotLastInSwitch

Deprecated

Since: PMD 1.5

Priority: Medium (3)

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

Note: This rule has been renamed from "DefaultLabelNotLastInSwitchStmt" with PMD 7.7.0.

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/ClassBody[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" />

## EnumComparison

Since: PMD 7.19.0

Priority: Medium (3)

When comparing enums, `equals()` should be avoided and `==` should be preferred.

Using `==` has some advantages:

- same semantic as `equals()`
- less problematic with possible null pointer exceptions
- supports static type check: If you compare two incompatible enum types, the compiler will tell you. When using equals, you’ll only get noticed at runtime or not at all.

This rule implements SonarSource rule S4551.

Note, that only primitive types and enums should be compared using `==`. To compare other objects, `equals()` is the correct way. See `CompareObjectsWithEquals` and `UseEqualsToCompareStrings`.

This rule is defined by the following XPath expression:

    //MethodCall[pmd-java:matchesSig("_#equals(java.lang.Object)")]
      [*[pmd-java:typeIs("java.lang.Enum")] or
       ArgumentList[*[pmd-java:typeIs("java.lang.Enum")]]]

Example(s):

    enum Color { RED, GREEN, BLUE }
    class ColorTester {
      boolean isRed(Color color) {
        return color.equals(Color.RED); // violation
      }

      boolean isGreen(Color color) {
        return color == Color.GREEN; // preferred
      }
    }

Use this rule by referencing it:

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

## ExhaustiveSwitchHasDefault

Since: PMD 7.10.0

Priority: Medium (3)

When switching over an enum or sealed class, the compiler will ensure that all possible cases are covered. If a case is missing, this will result in a compilation error. But if a default case is added, this compiler check is not performed anymore, leading to difficulties in noticing bugs at runtime.

Not using a default case makes sure, a compiler error is introduced whenever a new enum constant or a new subclass to the sealed class hierarchy is added. We will discover this problem at compile time rather than at runtime (if at all).

Note: The fix it not necessarily just removing the default case. Maybe a case is missing which needs to be implemented.

This rule is defined by the following XPath expression:

    //(SwitchStatement | SwitchExpression)
                          [@Exhaustive = true()]
                          [@DefaultCase = true()]

Example(s):

    class Foo {
        enum MyEnum { A, B };

        void doSomething(MyEnum e) {
            switch(e) {
                case A -> System.out.println("a");
                case B -> System.out.println("b");
                default -> System.out.println("unnecessary default");
            };
        }
    }

Use this rule by referencing it:

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

## 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
--- | --- | ---
maximumVariables | 1 | A regular for statement will have 1 control variable

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 it is actually enabled, or otherwise skip the associate String creation and manipulation, as well as any method calls.

An alternative to checking the log level are substituting parameters, formatters or lazy logging with lambdas. The available alternatives depend on the actual logging framework.

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

Example(s):

    // Add this for performance - avoid manipulating strings if the logger may drop it
    if (log.isDebugEnabled()) {
        log.debug("log something" + param1 + " and " + param2 + "concat strings");
    }

    // Avoid the guarding if statement with substituting parameters
    log.debug("log something {} and {}", param1, param2);

    // Avoid the guarding if statement with formatters
    log.debug("log something %s and %s", param1, param2);

    // This is still an issue, method invocations may be expensive / have side-effects
    log.debug("log something expensive: {}", calculateExpensiveLoggingText());

    // Avoid the guarding if statement with lazy logging and lambdas
    log.debug("log something expensive: {}", () -> calculateExpensiveLoggingText());

    // … alternatively use method references
    log.debug("log something expensive: {}", this::calculateExpensiveLoggingText);

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
logLevels | trace , debug , info , warn , error , log , finest , finer , fine , info , warning , severe | LogLevels to guard
guardsMethods | isTraceEnabled , isDebugEnabled , isInfoEnabled , isWarnEnabled , isErrorEnabled , isLoggable | Method use to guard the log statement

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>

## ImplicitFunctionalInterface

Since: PMD 7.12.0

Priority: Medium High (2)

Reports functional interfaces that were not explicitly declared as such with the annotation `@FunctionalInterface`. If an interface is accidentally a functional interface, then it should bear a `@SuppressWarnings("PMD.ImplicitFunctionalInterface")` annotation to make this clear.

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

Example(s):

    // The intent on this declaration is unclear, and the rule will report it.
                public interface MyInterface {
                    void doSomething();
                }

                // This is clearly intended as a functional interface.
                @FunctionalInterface
                public interface MyInterface {
                    void doSomething();
                }

                // This is clearly NOT intended as a functional interface.
                @SuppressWarnings("PMD.ImplicitFunctionalInterface")
                public interface MyInterface {
                    void doSomething();
                }

Use this rule by referencing it:

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

## 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:

    //MethodDeclaration[@Name='suite' and ClassType[pmd-java:typeIs('junit.framework.Test')]]
                       [not(.//ReturnStatement/*[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

Deprecated

This rule has been renamed. Use instead: UnitTestShouldUseAfterAnnotation

Deprecated

Since: PMD 4.0

Priority: Medium (3)

This rule detects methods called `tearDown()` that are not properly annotated as a cleanup method. This is primarily intended to assist in upgrading from JUnit 3, where tear down methods were required to be called `tearDown()`. To a lesser extent, this may help detect omissions even under newer JUnit versions or under TestNG, as long as you are following this convention to name the methods.

- JUnit 4 will only execute 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.
- TestNG provides the annotations `@AfterMethod` and `@AfterClass` to execute methods after each test or after tests in the class, respectively.

Note: This rule was named JUnit4TestShouldUseAfterAnnotation before PMD 7.7.0.

This rule is defined by the following XPath expression:

    //MethodDeclaration[@Name='tearDown' and @Arity=0]
        [not(ModifierList/Annotation[
               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.AfterClass')
            or pmd-java:typeIs('org.testng.annotations.AfterMethod')
        ])]
        (: Make sure this is a JUnit 4/5 or TestNG class :)
        [../MethodDeclaration[
                pmd-java:hasAnnotation('org.junit.Test')
             or pmd-java:hasAnnotation('org.junit.jupiter.api.Test')
             or pmd-java:hasAnnotation('org.testng.annotations.Test')
        ]]

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

Deprecated

This rule has been renamed. Use instead: UnitTestShouldUseBeforeAnnotation

Deprecated

Since: PMD 4.0

Priority: Medium (3)

This rule detects methods called `setUp()` that are not properly annotated as a setup method. This is primarily intended to assist in upgrading from JUnit 3, where setup methods were required to be called `setUp()`. To a lesser extent, this may help detect omissions even under newer JUnit versions or under TestNG, as long as you are following this convention to name the methods.

- JUnit 4 will only execute 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.
- TestNG provides the annotations `@BeforeMethod` and `@BeforeClass` to execute methods before each test or before tests in the class, respectively.

Note: This rule was named JUnit4TestShouldUseBeforeAnnotation before PMD 7.7.0.

This rule is defined by the following XPath expression:

    //MethodDeclaration[@Name='setUp' and @Arity=0]
        [not(ModifierList/Annotation[
               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')
            or pmd-java:typeIs('org.testng.annotations.BeforeClass')
        ])]
        (: Make sure this is a JUnit 4/5 or TestNG class :)
        [../MethodDeclaration[
                   pmd-java:hasAnnotation('org.junit.Test')
                or pmd-java:hasAnnotation('org.junit.jupiter.api.Test')
                or pmd-java:hasAnnotation('org.testng.annotations.Test')
        ]]

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

Deprecated

This rule has been renamed. Use instead: UnitTestShouldUseTestAnnotation

Deprecated

Since: PMD 4.0

Priority: Medium (3)

The rule will detect any test method starting with "test" that is not properly annotated, and will therefore not be run.

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`. In TestNG, only methods annotated with the `@Test` annotation are executed.

Note: This rule was named JUnit4TestShouldUseTestAnnotation before PMD 7.7.0.

This rule is defined by the following XPath expression:

    //ClassDeclaration[matches(@SimpleName, $testClassPattern) or pmd-java:typeIs('junit.framework.TestCase')]
        (: a junit 3 method :)
        /ClassBody/MethodDeclaration[
            @Visibility="public"
            and starts-with(@Name, 'test')
            and not(ModifierList/Annotation[
              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')
              or pmd-java:typeIs('org.testng.annotations.Test')
              ]
            )
        ]

Example(s):

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

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

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
testClassPattern | Test | The regex pattern used to identify test classes

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>

## JUnit5TestShouldBePackagePrivate

Since: PMD 6.35.0

Priority: Medium (3)

Reports JUnit 5 test classes and methods that are not package-private. Contrary to JUnit 4 tests, which required public visibility to be run by the engine, JUnit 5 tests can also be run if they’re package-private. Marking them as such is a good practice to limit their visibility.

Test methods are identified as those which use `@Test`, `@RepeatedTest`, `@TestFactory`, `@TestTemplate` or `@ParameterizedTest`.

This rule is defined by the following XPath expression:

    //ClassDeclaration[
        (: a Junit 5 test class, ie, it has methods with the annotation :)
        @Interface = false() and
        ClassBody/MethodDeclaration
        [ModifierList/Annotation[
                   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')
        ]]
    ]/(
           self::*[@Abstract = false() and @Visibility = ("public", "protected")]
    |      ClassBody/MethodDeclaration
           [@Visibility = ("public", "protected")]
           [ModifierList/Annotation[
                   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):

    class MyTest { // not public, that's fine
        @Test
        public void testBad() { } // should not have a public modifier

        @Test
        protected void testAlsoBad() { } // should not have a protected modifier

        @Test
        private void testNoRun() { } // should not have a private modifier

        @Test
        void testGood() { } // package private as expected
    }

Use this rule by referencing it:

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

## JUnitAssertionsShouldIncludeMessage

Deprecated

This rule has been renamed. Use instead: UnitTestAssertionsShouldIncludeMessage

Deprecated

Since: PMD 1.04

Priority: Medium (3)

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

This rule supports tests using JUnit (3, 4 and 5) and TestNG.

Note: This rule was named JUnitAssertionsShouldIncludeMessage before PMD 7.7.0.

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

Example(s):

    public class Foo {
        @Test
        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

Deprecated

This rule has been renamed. Use instead: UnitTestContainsTooManyAsserts

Deprecated

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 JUnit (3, 4 and 5) and TestNG Tests.

Note: This rule was named JUnitTestContainsTooManyAsserts before PMD 7.7.0.

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

Example(s):

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

        // Bad, too many asserts (assuming max=1)
        @Test
        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
--- | --- | ---
maximumAsserts | 1 | Maximum number of assert calls in a test method
extraAssertMethodNames |  | Extra valid assertion methods names

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" />
            <property name="extraAssertMethodNames" value="" />
        </properties>
    </rule>

## JUnitTestsShouldIncludeAssert

Deprecated

This rule has been renamed. Use instead: UnitTestShouldIncludeAssert

Deprecated

Since: PMD 2.0

Priority: Medium (3)

Unit 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 checks for JUnit (3, 4 and 5) and TestNG Tests.

Note: This rule was named JUnitTestsShouldIncludeAssert before PMD 7.7.0.

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

Example(s):

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

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
extraAssertMethodNames |  | Extra valid assertion methods names

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

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

Use this rule and customize it:

    <rule ref="category/java/bestpractices.xml/JUnitTestsShouldIncludeAssert">
        <properties>
            <property name="extraAssertMethodNames" value="" />
        </properties>
    </rule>

## 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" />

## LabeledStatement

Since: PMD 7.18.0

Priority: Medium (3)

This rule detects the use of labeled statements. By default, it allows labeled loops so that you can use `break` / `continue` with labels. This can be changed with the property `allowLoops` to flag any labels.

Labels make control flow difficult to understand and should be avoided. They can be confused with the goto statement and make code harder to read and maintain.

If you really need to jump out of an inner loop, think about refactoring the multiple loops into a function - that way, you can replace the break with a return.

This rule implements SonarSource rule S1119.

To detect unused labels, use the rule `UnusedLabel`.

This rule is defined by the following XPath expression:

    if ($allowLoops) then
        //LabeledStatement[not(DoStatement|WhileStatement|ForStatement|ForeachStatement)][let $label := @Label return
              (.//BreakStatement | .//ContinueStatement)[@Label = $label]
        ]
    else
        //LabeledStatement[let $label := @Label return
              (.//BreakStatement | .//ContinueStatement)[@Label = $label]
        ]

Example(s):

    class Scratch {
        public static void main(String[] args) {
            int x = 1;
            lbl1: while (true) {          // violation: labeled statement on loop (when property allowLoops=false)
                lbl2: if (x == 3) {       // violation: labeled statement
                    x++;
                    break lbl2;
                }
                lbl3: if (x == 4) {
                    break lbl1;
                }
                System.out.println(x);
                x++;
            }
        }
    }

    // Bad
    outer:
    for (int i = 0; i < 10; i++) {
        for (int j = 0; j < 10; j++) {
            break outer; // violation - labeled break (when allowLoops=false)
            continue outer; // violation - labeled continue (when allowLoops=false)
        }
    }

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
allowLoops | true | Whether to allow or not allow labels before loop statements (do, while, for)

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

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

Use this rule and customize it:

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

## 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.

Note that compile-time constant strings are treated like literals. This is because they are inlined into the class file, are necessarily non-null, and therefore cannot cause an NPE at runtime.

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

        static final String CONSTANT = "const";
        {
            CONSTANT.equals("literal"); // not reported, this is effectively the same as writing "const".equals("foo")
        }
    }

Use this rule by referencing it:

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

## LooseCoupling

Since: PMD 0.7

Priority: Medium (3)

Excessive coupling to implementation types (e.g., `HashSet`) limits your ability to use alternate implementations in the future as requirements change. Whenever available, declare variables and parameters using a more general type (e.g, `Set`).

This rule reports uses of concrete collection types. User-defined types that should be treated the same as interfaces can be configured with the property `allowedTypes`.

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

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
allowedTypes | java.util.Properties | Exceptions to the rule

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

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

Use this rule and customize it:

    <rule ref="category/java/bestpractices.xml/LooseCoupling">
        <properties>
            <property name="allowedTypes" value="java.util.Properties" />
        </properties>
    </rule>

## 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" />

## NonExhaustiveSwitch

Since: PMD 1.0

Priority: Medium (3)

Switch statements should be exhaustive, to make their control flow easier to follow. This can be achieved by adding a `default` case, or, if the switch is on an enum type, by ensuring there is one switch branch for each enum constant.

This rule doesn’t consider Switch Statements, that use Pattern Matching, since for these the compiler already ensures that all cases are covered. The same is true for Switch Expressions, which are also not considered by this rule.

This rule is defined by the following XPath expression:

    //SwitchStatement
                        (: exclude empty switches :)
                        [count(*) > 1]
                        [@DefaultCase = false()]
                        [@ExhaustiveEnumSwitch = false()]
                        (: exclude pattern tests - for these, the compiler will ensure exhaustiveness :)
                        [not(*/SwitchLabel[@PatternLabel = true()])]

Example(s):

    class Foo {{
        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/NonExhaustiveSwitch" />

## 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
--- | --- | ---
strictMode | false | If true, mark combined declaration even if the declarations are on separate lines.

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)

Reports exceptions that are thrown from within a catch block, yet don’t refer to the exception parameter declared by that catch block. The stack trace of the original exception could be lost, which makes the thrown exception less informative.

To preserve the stack trace, the original exception may be used as the cause of the new exception, using `Throwable#initCause`, or passed as a constructor argument to the new exception. It may also be preserved using `Throwable#addSuppressed`. The rule actually assumes that any method or constructor that takes the original exception as argument preserves the original stack trace.

The rule allows `InvocationTargetException` and `PrivilegedActionException` to be replaced by their cause exception. The discarded part of the stack trace is in those cases only JDK-internal code, which is not very useful. The rule also ignores exceptions whose name starts with `ignored`.

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); // Ok, this initializes the cause of the new exception
            }
            try {
                Integer.parseInt("a");
            } catch (Exception e) {
                throw (IllegalStateException)new IllegalStateException().initCause(e); // second possibility to create exception chain.
            }
        }
        void wrong() {
            try{
                Integer.parseInt("a");
            } catch (Exception e) {
                // Violation: this only preserves the message and not the stack trace
                throw new Exception(e.getMessage());
            }
        }
    }

Use this rule by referencing it:

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

## PrimitiveWrapperInstantiation

Since: PMD 6.37.0

Priority: Medium (3)

Reports usages of primitive wrapper constructors. They are deprecated since Java 9 and should not be used. Even before Java 9, they can be replaced with usage of the corresponding static `valueOf` factory method (which may be automatically inserted by the compiler since Java 1.5). This has the advantage that it may reuse common instances instead of creating a new instance each time.

Note that for `Boolean`, the named constants `Boolean.TRUE` and `Boolean.FALSE` are preferred instead of `Boolean.valueOf`.

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

Example(s):

    public class Foo {
                    private Integer ZERO = new Integer(0);      // violation
                    private Integer ZERO1 = Integer.valueOf(0); // better
                    private Integer ZERO1 = 0;                  // even better
                }

Use this rule by referencing it:

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

## RelianceOnDefaultCharset

Since: PMD 7.17.0

Priority: Medium (3)

Be sure to specify a character set for APIs that use the JVM’s default character set to ensure stable encoding behavior between different JVMs, programs, and servers. Using the platform’s default charset makes the code less portable and might lead to unexpected behavior when running on different systems.

Additional, since Java 18, the default charset for these APIs is consistently UTF-8 (see JEP 400). While this reduces unexpected behavior on different systems, it is still advised to explicitly specify a character set, especially if UTF-8 is not the desired charset.

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

Example(s):

    public class Foo {
        void bad() throws IOException {
            new InputStreamReader(inputStream);  // violation
            new OutputStreamWriter(outputStream);  // violation
            URLEncoder.encode("test string");  // violation (deprecated)
            new PrintStream(outputStream);  // violation
            new PrintWriter("output.txt");  // violation
            new Scanner(inputStream);  // violation
            new Formatter();  // violation
            "test".getBytes();  // violation
            new ByteArrayOutputStream().toString();  // violation
            new FileReader("input.txt");  // violation
            new FileWriter("output.txt");  // violation
        }

        void good() throws IOException {
            new InputStreamReader(inputStream, StandardCharsets.UTF_8);  // ok
            new OutputStreamWriter(outputStream, StandardCharsets.UTF_8);  // ok
            URLEncoder.encode("test string", StandardCharsets.UTF_8);  // ok
            new PrintStream(outputStream, true, StandardCharsets.UTF_8);  // ok
            new PrintWriter("output.txt", StandardCharsets.UTF_8);  // ok
            new Scanner(inputStream, StandardCharsets.UTF_8);  // ok
            new Formatter(Locale.US);  // ok
            "test".getBytes(StandardCharsets.UTF_8);  // ok
            new ByteArrayOutputStream().toString(StandardCharsets.UTF_8);  // ok
            new FileReader("input.txt", StandardCharsets.UTF_8);  // ok
            new FileWriter("output.txt", StandardCharsets.UTF_8);  // ok
        }
    }

Use this rule by referencing it:

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

## 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/ClassType[pmd-java:typeIsExactly('java.util.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:

    //ClassType[pmd-java:typeIsExactly('java.util.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:

    //ClassType[pmd-java:typeIsExactly('java.util.Vector')]

Example(s):

    import java.util.Vector;
    public class Foo {
        void bar() {
            Vector v = new Vector();
        }
    }

Use this rule by referencing it:

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

## SimplifiableTestAssertion

Since: PMD 6.37.0

Priority: Medium (3)

Reports test assertions that may be simplified using a more specific assertion method. This enables better error messages, and makes the assertions more readable.

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

Example(s):

    import org.junit.Test;
    import static org.junit.Assert.*;

    class SomeTestClass {
        Object a,b;
        @Test
        void testMethod() {
            assertTrue(a.equals(b)); // could be assertEquals(a, b);
            assertTrue(!a.equals(b)); // could be assertNotEquals(a, b);

            assertTrue(!something); // could be assertFalse(something);
            assertFalse(!something); // could be assertTrue(something);

            assertTrue(a == b); // could be assertSame(a, b);
            assertTrue(a != b); // could be assertNotSame(a, b);

            assertTrue(a == null); // could be assertNull(a);
            assertTrue(a != null); // could be assertNotNull(a);
        }
    }

Use this rule by referencing it:

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

## SwitchStmtsShouldHaveDefault

Deprecated

This rule has been renamed. Use instead: NonExhaustiveSwitch

Deprecated

Since: PMD 1.0

Priority: Medium (3)

Switch statements should be exhaustive, to make their control flow easier to follow. This can be achieved by adding a `default` case, or, if the switch is on an enum type, by ensuring there is one switch branch for each enum constant.

This rule doesn’t consider Switch Statements, that use Pattern Matching, since for these the compiler already ensures that all cases are covered. The same is true for Switch Expressions, which are also not considered by this rule.

This rule is defined by the following XPath expression:

    //SwitchStatement
                        (: exclude empty switches :)
                        [count(*) > 1]
                        [@DefaultCase = false()]
                        [@ExhaustiveEnumSwitch = false()]
                        (: exclude pattern tests - for these, the compiler will ensure exhaustiveness :)
                        [not(*/SwitchLabel[@PatternLabel = true()])]

Example(s):

    class Foo {{
        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" />

## UnitTestAssertionsShouldIncludeMessage

Since: PMD 1.04

Priority: Medium (3)

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

This rule supports tests using JUnit (3, 4 and 5) and TestNG.

Note: This rule was named JUnitAssertionsShouldIncludeMessage before PMD 7.7.0.

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

Example(s):

    public class Foo {
        @Test
        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/UnitTestAssertionsShouldIncludeMessage" />

## UnitTestContainsTooManyAsserts

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 JUnit (3, 4 and 5) and TestNG Tests.

Note: This rule was named JUnitTestContainsTooManyAsserts before PMD 7.7.0.

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

Example(s):

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

        // Bad, too many asserts (assuming max=1)
        @Test
        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
--- | --- | ---
maximumAsserts | 1 | Maximum number of assert calls in a test method
extraAssertMethodNames |  | Extra valid assertion methods names

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

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

Use this rule and customize it:

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

## UnitTestShouldIncludeAssert

Since: PMD 2.0

Priority: Medium (3)

Unit 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 checks for JUnit (3, 4 and 5) and TestNG Tests.

Note: This rule was named JUnitTestsShouldIncludeAssert before PMD 7.7.0.

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

Example(s):

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

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
extraAssertMethodNames |  | Extra valid assertion methods names

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

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

Use this rule and customize it:

    <rule ref="category/java/bestpractices.xml/UnitTestShouldIncludeAssert">
        <properties>
            <property name="extraAssertMethodNames" value="" />
        </properties>
    </rule>

## UnitTestShouldUseAfterAnnotation

Since: PMD 4.0

Priority: Medium (3)

This rule detects methods called `tearDown()` that are not properly annotated as a cleanup method. This is primarily intended to assist in upgrading from JUnit 3, where tear down methods were required to be called `tearDown()`. To a lesser extent, this may help detect omissions even under newer JUnit versions or under TestNG, as long as you are following this convention to name the methods.

- JUnit 4 will only execute 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.
- TestNG provides the annotations `@AfterMethod` and `@AfterClass` to execute methods after each test or after tests in the class, respectively.

Note: This rule was named JUnit4TestShouldUseAfterAnnotation before PMD 7.7.0.

This rule is defined by the following XPath expression:

    //MethodDeclaration[@Name='tearDown' and @Arity=0]
        [not(ModifierList/Annotation[
               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.AfterClass')
            or pmd-java:typeIs('org.testng.annotations.AfterMethod')
        ])]
        (: Make sure this is a JUnit 4/5 or TestNG class :)
        [../MethodDeclaration[
                pmd-java:hasAnnotation('org.junit.Test')
             or pmd-java:hasAnnotation('org.junit.jupiter.api.Test')
             or pmd-java:hasAnnotation('org.testng.annotations.Test')
        ]]

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/UnitTestShouldUseAfterAnnotation" />

## UnitTestShouldUseBeforeAnnotation

Since: PMD 4.0

Priority: Medium (3)

This rule detects methods called `setUp()` that are not properly annotated as a setup method. This is primarily intended to assist in upgrading from JUnit 3, where setup methods were required to be called `setUp()`. To a lesser extent, this may help detect omissions even under newer JUnit versions or under TestNG, as long as you are following this convention to name the methods.

- JUnit 4 will only execute 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.
- TestNG provides the annotations `@BeforeMethod` and `@BeforeClass` to execute methods before each test or before tests in the class, respectively.

Note: This rule was named JUnit4TestShouldUseBeforeAnnotation before PMD 7.7.0.

This rule is defined by the following XPath expression:

    //MethodDeclaration[@Name='setUp' and @Arity=0]
        [not(ModifierList/Annotation[
               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')
            or pmd-java:typeIs('org.testng.annotations.BeforeClass')
        ])]
        (: Make sure this is a JUnit 4/5 or TestNG class :)
        [../MethodDeclaration[
                   pmd-java:hasAnnotation('org.junit.Test')
                or pmd-java:hasAnnotation('org.junit.jupiter.api.Test')
                or pmd-java:hasAnnotation('org.testng.annotations.Test')
        ]]

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/UnitTestShouldUseBeforeAnnotation" />

## UnitTestShouldUseTestAnnotation

Since: PMD 4.0

Priority: Medium (3)

The rule will detect any test method starting with "test" that is not properly annotated, and will therefore not be run.

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`. In TestNG, only methods annotated with the `@Test` annotation are executed.

Note: This rule was named JUnit4TestShouldUseTestAnnotation before PMD 7.7.0.

This rule is defined by the following XPath expression:

    //ClassDeclaration[matches(@SimpleName, $testClassPattern) or pmd-java:typeIs('junit.framework.TestCase')]
        (: a junit 3 method :)
        /ClassBody/MethodDeclaration[
            @Visibility="public"
            and starts-with(@Name, 'test')
            and not(ModifierList/Annotation[
              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')
              or pmd-java:typeIs('org.testng.annotations.Test')
              ]
            )
        ]

Example(s):

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

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

This rule has the following properties:

Name | Default Value | Description
--- | --- | ---
testClassPattern | Test | The regex pattern used to identify test classes

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

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

Use this rule and customize it:

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

## UnnecessaryVarargsArrayCreation

Since: PMD 7.1.0

Priority: Medium (3)

Reports explicit array creation when a varargs is expected. For instance:

    Arrays.asList(new String[] { "foo", "bar", });

can be replaced by:

    Arrays.asList("foo", "bar");

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

Example(s):

    import java.util.Arrays;

    class C {
        static {
            Arrays.asList(new String[]{"foo", "bar",});
            // should be
            Arrays.asList("foo", "bar");
        }
    }

Use this rule by referencing it:

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

## UnnecessaryWarningSuppression

Since: PMD 7.14.0

Priority: Medium (3)

This rule reports suppression comments and annotations that did not suppress any PMD violation. Note that violations of this rule cannot be suppressed.

Please note:

- The rule will report those suppressions comments/annotations that did not suppress a violation during the current run. That means you cannot run this rule separately from other rules, it must always be run with all the rules that could produce a warning. This is most likely not a problem, as you can just include this rule in your regular ruleset.
- The rule for now only reports annotations specific to PMD, like `@SuppressWarnings("PMD")`. For instance `@SuppressWarnings("all")` is never reported as we cannot know if another tool is producing a warning there that must be suppressed. In the future we might be able to check for other common ones like `@SuppressWarnings("unchecked")` or `"fallthrough"`.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.rule.impl.UnnecessaryPmdSuppressionRule

Example(s):

    public class Something {
                    // Unless some rule triggered on the following line, this rule will report the comment:
                    private void foo() {} // NOPMD
                }

Use this rule by referencing it:

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

## 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 tracks assignments to fields of `this`, and static fields of the current class. This may cause some false positives in timing-sensitive concurrent code, which the rule cannot detect.

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
--- | --- | ---
checkUnusedPrefixIncrement | false | Report expressions like ++i that may be replaced with (i + 1)
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

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`. The same applies to public constructors.

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
--- | --- | ---
checkAll | false | Check all methods, including non-private ones

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>

## UnusedLabel

Since: PMD 7.18.0

Priority: Medium (3)

Unused labeled are unnecessary and may be confusing as you might be wondering what this label is used for. To improve readability the unused label should simply be removed.

This rule implements SonarSource rule S1065.

This rule is defined by the following XPath expression:

    //LabeledStatement[let $label := @Label return
          not( (.//BreakStatement | .//ContinueStatement)[@Label = $label] )
    ]

Example(s):

    class Example {
        void main() {
            lbl1: {                     // violation: Label "lbl1" is not nused
                int x = 1;
                System.out.println(x);
            }
        }
    }

Use this rule by referencing it:

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

## 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.

Since PMD 6.50.0 private fields are ignored, if the fields are annotated with any annotation or the enclosing class has any annotation. Annotations often enable a framework (such as dependency injection, mocking or e.g. Lombok) which use the fields by reflection or other means. This usage can’t be detected by static code analysis. Previously these frameworks where explicitly allowed by listing their annotations in the property "ignoredAnnotations", but that turned out to be prone of false positive for any not explicitly considered framework.

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
--- | --- | ---
ignoredFieldNames | serialVersionUID , serialPersistentFields | Field Names that are ignored from the unused check
reportForAnnotations |  | Fully qualified names of the annotation types that should be reported anyway. If an unused field has any of these annotations, then it is reported. If it has any other annotation, then it is still considered to used and is not reported.

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="ignoredFieldNames" value="serialVersionUID,serialPersistentFields" />
            <property name="reportForAnnotations" value="" />
        </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
--- | --- | ---
ignoredAnnotations | java.lang.Deprecated , jakarta.annotation.PostConstruct , jakarta.annotation.PreDestroy , lombok.EqualsAndHashCode.Include | Fully qualified names of the annotation types that should be ignored by this rule

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,jakarta.annotation.PostConstruct,jakarta.annotation.PreDestroy,lombok.EqualsAndHashCode.Include" />
        </properties>
    </rule>

## 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" />

## UseEnumCollections

Since: PMD 7.3.0

Priority: Medium (3)

Wherever possible, use `EnumSet` or `EnumMap` instead of `HashSet` and `HashMap` when the keys are of an enum type. The specialized enum collections are more space- and time-efficient. This rule reports constructor expressions for hash sets or maps whose key type is an enum type.

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

Example(s):

    import java.util.EnumMap;
                import java.util.HashSet;

                enum Example {
                    A, B, C;

                    public static Set<Example> newSet() {
                        return new HashSet<>(); // Could be EnumSet.noneOf(Example.class)
                    }

                    public static <V> Map<Example, V> newMap() {
                        return new HashMap<>(); // Could be new EnumMap<>(Example.class)
                    }
                }

Use this rule by referencing it:

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

## UseStandardCharsets

Since: PMD 6.34.0

Priority: Medium (3)

Minimum Language Version: Java 1.7

Starting with Java 7, StandardCharsets provides constants for common Charset objects, such as UTF-8. Using the constants is less error prone, and can provide a small performance advantage compared to `Charset.forName(...)` since no scan across the internal `Charset` caches is needed.

This rule is defined by the following XPath expression:

    //MethodCall[@MethodName = 'forName'][pmd-java:typeIs('java.nio.charset.Charset')]
        [
            ArgumentList/StringLiteral
                [@Image = ('"US-ASCII"', '"ISO-8859-1"', '"UTF-8"', '"UTF-16BE"', '"UTF-16LE"', '"UTF-16"')]
        ]

Example(s):

    public class UseStandardCharsets {
        public void run() {

            // looking up the charset dynamically
            try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("UTF-8"))) {
                osw.write("test");
            }

            // best to use StandardCharsets
            try (OutputStreamWriter osw = new OutputStreamWriter(out, StandardCharsets.UTF_8)) {
                osw.write("test");
            }
        }
    }

Use this rule by referencing it:

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

## 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 Java class: net.sourceforge.pmd.lang.java.rule.bestpractices.UseTryWithResourcesRule

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
--- | --- | ---
closeMethods | close , closeQuietly | Method names in finally block, which trigger this rule

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.

Byte arrays in any method and String arrays in `public static void main(String[])` methods are ignored.

This rule is defined by the following XPath expression:

    //FormalParameters[not(parent::MethodDeclaration[@Overridden=true() or @MainMethod=true()])]
      /FormalParameter[position()=last()]
       [@Varargs=false()]
       [ArrayType[not(PrimitiveType[@Kind = "byte"] or ClassType[pmd-java:typeIs('java.lang.Byte')])]
        or VariableId[ArrayDimensions] and (PrimitiveType[not(@Kind="byte")] or ClassType[not(pmd-java:typeIs('java.lang.Byte'))])]

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:

    (: while loops with single boolean literal 'false', maybe parenthesized :)
    //WhileStatement/BooleanLiteral[@True = false()]
    |
    (: do-while loops with single boolean literal ('false' or 'true'), maybe parenthesized :)
    //DoStatement/BooleanLiteral
    |
    (: while loops with conditional or'ed boolean literals, maybe parenthesized :)
    //WhileStatement[(InfixExpression[@Operator = ('|', '||')])
        (: no var access :)
        [count(VariableAccess) = 0]
        (: at least one false literal :)
        [count(BooleanLiteral[@True = false()]) >= 1]]
    |
    (: while loops with conditional and'ed boolean literals, maybe parenthesized :)
    //WhileStatement[(InfixExpression[@Operator = ('&', '&&')])
        (: at least one false literal :)
        [count(BooleanLiteral[@True = false()]) >= 1]]
    |
    (: do-while loops with conditional or'ed boolean literals, maybe parenthesized :)
    //DoStatement[(InfixExpression[@Operator = ('|', '||')])
        (: at least one true literal :)
        [count(BooleanLiteral[@True = true()]) >= 1
          (: or only boolean literal and no no var access :)
          or count(BooleanLiteral) >= 1
          and count(VariableAccess) = 0
        ]]
    |
    (: do-while loops with conditional and'ed boolean literals, maybe parenthesized :)
    //DoStatement[(InfixExpression[@Operator = ('&', '&&')])
        (: at least one false literal :)
        [count(BooleanLiteral[@True = false()]) >= 1
          (: or only boolean literal and no no var access :)
          or count(BooleanLiteral) >= 1
          and count(VariableAccess) = 0
        ]]

Example(s):

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

Use this rule by referencing it:

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