Best Practices

AbstractClassWithoutAbstractMethod

Since: PMD 3.0

Priority: Medium (3)

The abstract class does not contain any abstract methods. An abstract class suggestsan incomplete implementation, which is to be completed by subclasses implementing theabstract methods. If the class is intended to be used as a base class only (not to be instantiateddirectly) 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):

publicabstractclassFoo{voidintmethod1(){...}voidintmethod2(){...}// consider using abstract methods or removing// the abstract modifier and adding protected constructors}

Use this rule by referencing it:

<ruleref="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 thegeneration of an accessor. A factory method, or non-privatization of the constructor can eliminate thissituation. The generated class file is actually an interface. It gives the accessing class the abilityto 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. Thismeans 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):

publicclassOuter{voidmethod(){Inneric=newInner();//Causes generation of accessor class}publicclassInner{privateInner(){}}}

Use this rule by referencing it:

<ruleref="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 methodswith package-private visibility. This adds overhead, and to the dex method count on Android. This situation canbe 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. Thismeans 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):

publicclassOuterClass{privateintcounter;/* package */intid;publicclassInnerClass{InnerClass(){OuterClass.this.counter++;// wrong accessor method will be generated}publicintgetOuterClassId(){returnOuterClass.this.id;// id is package-private, no accessor method needed}}}

Use this rule by referencing it:

<ruleref="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):

publicclassFoo{privateString[]x;publicvoidfoo(String[]param){// Don't do this, make a copy of the array at leastthis.x=param;}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/ArrayIsStoredDirectly"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/ArrayIsStoredDirectly"><properties><propertyname="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 resultsif 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):

importjava.security.MessageDigest;publicclassAvoidMessageDigestFieldExample{privatefinalMessageDigestsharedMd;publicAvoidMessageDigestFieldExample()throwsException{sharedMd=MessageDigest.getInstance("SHA-256");}publicbyte[]calculateHashShared(byte[]data){// sharing a MessageDigest like this without synchronizing access// might lead to wrong resultssharedMd.reset();sharedMd.update(data);returnsharedMd.digest();}// betterpublicbyte[]calculateHash(byte[]data)throwsException{MessageDigestmd=MessageDigest.getInstance("SHA-256");md.update(data);returnmd.digest();}}

Use this rule by referencing it:

<ruleref="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):

classFoo{voidbar(){try{// do something}catch(Exceptione){e.printStackTrace();}}}

Use this rule by referencing it:

<ruleref="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 statementis 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):

publicclassFoo{publicvoidfoo(){try{// do something}catch(Exceptione){e=newNullPointerException();// not recommended}try{// do something}catch(MyException|ServerExceptione){e=newRuntimeException();// won't compile}}}

Use this rule by referencing it:

<ruleref="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 theforeachReassign 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 theforReassign 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):

publicclassFoo{privatevoidfoo(){for(Strings:listOfStrings()){s=s.trim();// OK, when foreachReassign is "firstOnly" or "allow"doSomethingWith(s);s=s.toUpper();// OK, when foreachReassign is "allow"doSomethingElseWith(s);}for(inti=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:

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

<ruleref="category/java/bestpractices.xml/AvoidReassigningLoopVariables"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/AvoidReassigningLoopVariables"><properties><propertyname="foreachReassign"value="deny"/><propertyname="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 canmake the code more difficult to understand. The code is often read with the assumption that parameter valuesdon’t change and an assignment violates therefore the principle of least astonishment. This is especially aproblem if the parameter is documented e.g. in the method’s javadoc and the new content differs from the originaldocumented content.

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

Note that this rule considers both methods and constructors. If there are multiple assignments for a formalparameter, 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):

publicclassHello{privatevoidgreet(Stringname){name=name.trim();System.out.println("Hello "+name);// preferredStringtrimmedName=name.trim();System.out.println("Hello "+trimmedName);}}

Use this rule by referencing it:

<ruleref="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 leaksif held within objects with long lifetimes.

This rule is defined by the following XPath expression:

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

Example(s):

publicclassFoo{privateStringBufferbuffer;// potential memory leak as an instance variable;}

Use this rule by referencing it:

<ruleref="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):

publicclassFoo{privateStringip="127.0.0.1";// not recommended}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/AvoidUsingHardCodedIP"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/AvoidUsingHardCodedIP"><properties><propertyname="checkAddressTypes"value="IPv4,IPv6,IPv4 mapped IPv6"/></properties></rule>

CheckResultSet

Since: PMD 4.1

Priority: Medium (3)

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

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

Example(s):

Statementstat=conn.createStatement();ResultSetrst=stat.executeQuery("SELECT name FROM person");rst.next();// what if it returns false? bad formStringfirstName=rst.getString(1);Statementstat=conn.createStatement();ResultSetrst=stat.executeQuery("SELECT name FROM person");if(rst.next()){// result is properly examined and usedStringfirstName=rst.getString(1);}else{// handle missing data}

Use this rule by referencing it:

<ruleref="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()ornot(ClassBody/MethodDeclaration)]/ClassBody/FieldDeclaration

Example(s):

publicinterfaceConstantInterface{publicstaticfinalintCONST1=1;// violation, no fields allowed in interface!staticfinalintCONST2=1;// violation, no fields allowed in interface!finalintCONST3=1;// violation, no fields allowed in interface!intCONST4=1;// violation, no fields allowed in interface!}// with ignoreIfHasMethods = falsepublicinterfaceAnotherConstantInterface{publicstaticfinalintCONST1=1;// violation, no fields allowed in interface!intanyMethod();}// with ignoreIfHasMethods = truepublicinterfaceYetAnotherConstantInterface{publicstaticfinalintCONST1=1;// no violationintanyMethod();}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/ConstantsInInterface"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/ConstantsInInterface"><properties><propertyname="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()andnot(..is../../*[last()])]

Example(s):

publicclassFoo{voidbar(inta){switch(a){case1:// do somethingbreak;default:// the default case should be last, by conventionbreak;case2:break;}}}

Use this rule by referencing it:

<ruleref="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()andnot(..is../../*[last()])]

Example(s):

publicclassFoo{voidbar(inta){switch(a){case1:// do somethingbreak;default:// the default case should be last, by conventionbreak;case2:break;}}}

Use this rule by referencing it:

<ruleref="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 implicitlygenerates a new .class file, and the object holds a strong reference to the enclosing object. For thosereasons, 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-braceinitialization. There is currently no way to find out whether a method called in the initializer is notaccessible 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 initializationreturnnewArrayList<String>(){{add("a");add("b");add("c");}};// the better way is to not create an anonymous class:List<String>a=newArrayList<>();a.add("a");a.add("b");a.add("c");returna;

Use this rule by referencing it:

<ruleref="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 asequals()
• 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 ruleS4551.

Note, that only primitive types and enums should be compared using==. To compare otherobjects,equals() is the correct way. SeeCompareObjectsWithEqualsandUseEqualsToCompareStrings.

This rule is defined by the following XPath expression:

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

Example(s):

enumColor{RED,GREEN,BLUE}classColorTester{booleanisRed(Colorcolor){returncolor.equals(Color.RED);// violation}booleanisGreen(Colorcolor){returncolor==Color.GREEN;// preferred}}

Use this rule by referencing it:

<ruleref="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 compilercheck 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 anew subclass to the sealed class hierarchy is added. We will discover this problem at compile timerather 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):

classFoo{enumMyEnum{A,B};voiddoSomething(MyEnume){switch(e){caseA->System.out.println("a");caseB->System.out.println("b");default->System.out.println("unnecessary default");};}}

Use this rule by referencing it:

<ruleref="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 overlists, arrays and iterators. A loop is safe to replace if it only uses the index variable toaccess an element of the list or array, only has one update statement, and loops througheveryelement 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):

publicclassMyClass{voidloop(List<String>l){for(inti=0;i<l.size();i++){// pre Java 1.5System.out.println(l.get(i));}for(Strings:l){// post Java 1.5System.out.println(s);}}}

Use this rule by referencing it:

<ruleref="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 valuesthe 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 loopfor(inti=0,j=0;i<10;i++,j+=2){foo();

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/ForLoopVariableCount"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/ForLoopVariableCount"><properties><propertyname="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, orotherwise 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 loggingwith 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 itif(log.isDebugEnabled()){log.debug("log something"+param1+" and "+param2+"concat strings");}// Avoid the guarding if statement with substituting parameterslog.debug("log something {} and {}",param1,param2);// Avoid the guarding if statement with formatterslog.debug("log something %s and %s",param1,param2);// This is still an issue, method invocations may be expensive / have side-effectslog.debug("log something expensive: {}",calculateExpensiveLoggingText());// Avoid the guarding if statement with lazy logging and lambdaslog.debug("log something expensive: {}",()->calculateExpensiveLoggingText());// … alternatively use method referenceslog.debug("log something expensive: {}",this::calculateExpensiveLoggingText);

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/GuardLogStatement"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/GuardLogStatement"><properties><propertyname="logLevels"value="trace,debug,info,warn,error,log,finest,finer,fine,info,warning,severe"/><propertyname="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 withthe annotation@FunctionalInterface. If an interface is accidentally a functionalinterface, 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.publicinterfaceMyInterface{voiddoSomething();}// This is clearly intended as a functional interface.@FunctionalInterfacepublicinterfaceMyInterface{voiddoSomething();}// This is clearly NOT intended as a functional interface.@SuppressWarnings("PMD.ImplicitFunctionalInterface")publicinterfaceMyInterface{voiddoSomething();}

Use this rule by referencing it:

<ruleref="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 indicatedthrough the @RunWith(Suite.class) annotation.

This rule is defined by the following XPath expression:

//MethodDeclaration[@Name='suite'andClassType[pmd-java:typeIs('junit.framework.Test')]][not(.//ReturnStatement/*[pmd-java:typeIs('junit.framework.JUnit4TestAdapter')])]

Example(s):

publicclassBadExampleextendsTestCase{publicstaticTestsuite(){returnnewSuite();}}@RunWith(Suite.class)@SuiteClasses({TestOne.class,TestTwo.class})publicclassGoodTest{}

Use this rule by referencing it:

<ruleref="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 calledtearDown() 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 calledtearDown().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 afterall tests in the class, respectively.
• TestNG provides the annotations@AfterMethod and@AfterClass to execute methods after each test or aftertests 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')orpmd-java:typeIs('org.junit.jupiter.api.AfterEach')orpmd-java:typeIs('org.junit.jupiter.api.AfterAll')orpmd-java:typeIs('org.testng.annotations.AfterClass')orpmd-java:typeIs('org.testng.annotations.AfterMethod')])](: Make sure this is a JUnit 4/5 or TestNG class :)[../MethodDeclaration[pmd-java:hasAnnotation('org.junit.Test')orpmd-java:hasAnnotation('org.junit.jupiter.api.Test')orpmd-java:hasAnnotation('org.testng.annotations.Test')]]

Example(s):

publicclassMyTest{publicvoidtearDown(){bad();}}publicclassMyTest2{@AfterpublicvoidtearDown(){good();}}

Use this rule by referencing it:

<ruleref="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 calledsetUp() 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 calledsetUp().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 alltests in the class, respectively.
• TestNG provides the annotations@BeforeMethod and@BeforeClass to execute methods before each test or beforetests 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')orpmd-java:typeIs('org.junit.jupiter.api.BeforeEach')orpmd-java:typeIs('org.junit.jupiter.api.BeforeAll')orpmd-java:typeIs('org.testng.annotations.BeforeMethod')orpmd-java:typeIs('org.testng.annotations.BeforeClass')])](: Make sure this is a JUnit 4/5 or TestNG class :)[../MethodDeclaration[pmd-java:hasAnnotation('org.junit.Test')orpmd-java:hasAnnotation('org.junit.jupiter.api.Test')orpmd-java:hasAnnotation('org.testng.annotations.Test')]]

Example(s):

publicclassMyTest{publicvoidsetUp(){bad();}}publicclassMyTest2{@BeforepublicvoidsetUp(){good();}}

Use this rule by referencing it:

<ruleref="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)orpmd-java:typeIs('junit.framework.TestCase')](: a junit 3 method :)/ClassBody/MethodDeclaration[@Visibility="public"andstarts-with(@Name,'test')andnot(ModifierList/Annotation[pmd-java:typeIs('org.junit.Test')orpmd-java:typeIs('org.junit.jupiter.api.Test')orpmd-java:typeIs('org.junit.jupiter.api.RepeatedTest')orpmd-java:typeIs('org.junit.jupiter.api.TestFactory')orpmd-java:typeIs('org.junit.jupiter.api.TestTemplate')orpmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest')orpmd-java:typeIs('org.testng.annotations.Test')])]

Example(s):

publicclassMyTest{publicvoidtestBad(){doSomething();}@TestpublicvoidtestGood(){doSomething();}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/JUnit4TestShouldUseTestAnnotation"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/JUnit4TestShouldUseTestAnnotation"><properties><propertyname="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 suchis 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()andClassBody/MethodDeclaration[ModifierList/Annotation[pmd-java:typeIs('org.junit.jupiter.api.Test')orpmd-java:typeIs('org.junit.jupiter.api.RepeatedTest')orpmd-java:typeIs('org.junit.jupiter.api.TestFactory')orpmd-java:typeIs('org.junit.jupiter.api.TestTemplate')orpmd-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')orpmd-java:typeIs('org.junit.jupiter.api.RepeatedTest')orpmd-java:typeIs('org.junit.jupiter.api.TestFactory')orpmd-java:typeIs('org.junit.jupiter.api.TestTemplate')orpmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest')]])

Example(s):

classMyTest{// not public, that's fine@TestpublicvoidtestBad(){}// should not have a public modifier@TestprotectedvoidtestAlsoBad(){}// should not have a protected modifier@TestprivatevoidtestNoRun(){}// should not have a private modifier@TestvoidtestGood(){}// package private as expected}

Use this rule by referencing it:

<ruleref="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 ofassertEquals(), 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):

publicclassFoo{@TestpublicvoidtestSomething(){assertEquals("foo","bar");// Use the form:// assertEquals("Foo does not equals bar", "foo", "bar");// instead}}

Use this rule by referencing it:

<ruleref="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 whichit 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):

publicclassMyTestCase{// Ok@TestpublicvoidtestMyCaseWithOneAssert(){booleanmyVar=false;assertFalse("should be false",myVar);}// Bad, too many asserts (assuming max=1)@TestpublicvoidtestMyCaseWithMoreAsserts(){booleanmyVar=false;assertFalse("myVar should be false",myVar);assertEquals("should equals false",false,myVar);}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/JUnitTestContainsTooManyAsserts"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/JUnitTestContainsTooManyAsserts"><properties><propertyname="maximumAsserts"value="1"/><propertyname="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 assertwith 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):

publicclassFoo{@TestpublicvoidtestSomething(){Barb=findBar();// This is better than having a NullPointerException// assertNotNull("bar not found", b);b.work();}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/JUnitTestsShouldIncludeAssert"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/JUnitTestsShouldIncludeAssert"><properties><propertyname="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):

publicclassMyTest{@TestpublicvoidtestBad(){try{doSomething();fail("should have thrown an exception");}catch(Exceptione){}}@Test(expected=Exception.class)publicvoidtestGood(){doSomething();}}

Use this rule by referencing it:

<ruleref="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 canusebreak /continue with labels. This can be changed with the propertyallowLoops to flag anylabels.

Labels make control flow difficult to understand and should be avoided. They can be confusedwith 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 multipleloops into a function - that way, you can replace the break with a return.

This rule implements SonarSource ruleS1119.

To detect unused labels, use the ruleUnusedLabel.

This rule is defined by the following XPath expression:

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

Example(s):

classScratch{publicstaticvoidmain(String[]args){intx=1;lbl1:while(true){// violation: labeled statement on loop (when property allowLoops=false)lbl2:if(x==3){// violation: labeled statementx++;breaklbl2;}lbl3:if(x==4){breaklbl1;}System.out.println(x);x++;}}}

// Badouter:for(inti=0;i<10;i++){for(intj=0;j<10;j++){breakouter;// violation - labeled break (when allowLoops=false)continueouter;// violation - labeled continue (when allowLoops=false)}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/LabeledStatement"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/LabeledStatement"><properties><propertyname="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 NullPointerExceptionscan be avoided, they will just return false. Note that switching literal positions for compareTo andcompareToIgnoreCase may change the result, see examples.

Note that compile-time constant strings are treated like literals. This is because they are inlined intothe 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):

classFoo{booleanbar(Stringx){returnx.equals("2");// should be "2".equals(x)}booleanbar(Stringx){returnx.equalsIgnoreCase("2");// should be "2".equalsIgnoreCase(x)}booleanbar(Stringx){return(x.compareTo("bar")>0);// should be: "bar".compareTo(x) < 0}booleanbar(Stringx){return(x.compareToIgnoreCase("bar")>0);// should be: "bar".compareToIgnoreCase(x) < 0}booleanbar(Stringx){returnx.contentEquals("bar");// should be "bar".contentEquals(x)}staticfinalStringCONSTANT="const";{CONSTANT.equals("literal");// not reported, this is effectively the same as writing "const".equals("foo")}}

Use this rule by referencing it:

<ruleref="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 alternateimplementations in the future as requirements change. Whenever available, declare variablesand parameters using a more general type (e.g,Set).

This rule reports uses of concrete collection types. User-defined types that should be treatedthe same as interfaces can be configured with the propertyallowedTypes.

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

Example(s):

importjava.util.ArrayList;importjava.util.HashSet;publicclassBar{// sub-optimal approachprivateArrayList<SomeType>list=newArrayList<>();publicHashSet<SomeType>getFoo(){returnnewHashSet<SomeType>();}// preferred approachprivateList<SomeType>list=newArrayList<>();publicSet<SomeType>getFoo(){returnnewHashSet<SomeType>();}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/LooseCoupling"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/LooseCoupling"><properties><propertyname="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 beremoved 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):

publicclassSecureSystem{UserData[]ud;publicUserData[]getUserData(){// Don't return directly the internal array, return a copyreturnud;}}

Use this rule by referencing it:

<ruleref="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 thatthe 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):

publicclassFooimplementsRunnable{// This method is overridden, and should have an @Override annotationpublicvoidrun(){}}

Use this rule by referencing it:

<ruleref="category/java/bestpractices.xml/MissingOverride"/>

NonExhaustiveSwitch

Since: PMD 1.0

Priority: Medium (3)

Switch statements should be exhaustive, to make their control floweasier to follow. This can be achieved by adding adefault case, or,if the switch is on an enum type, by ensuring there is one switch branchfor each enum constant.

This rule doesn’t consider Switch Statements, that use Pattern Matching, since for these thecompiler 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):

classFoo{{intx=2;switch(x){case1:intj=6;case2:intj=8;// missing default: here}}}

Use this rule by referencing it:

<ruleref="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][$strictModeorcount(distinct-values(VariableDeclarator/@BeginLine))!=count(VariableDeclarator)]|//FieldDeclaration[count(VariableDeclarator)>1][$strictModeorcount(distinct-values(VariableDeclarator/@BeginLine))!=count(VariableDeclarator)]

Example(s):

Stringname;// separate declarationsStringlastname;Stringname,lastname;// combined declaration, a violationStringname,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:

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

<ruleref="category/java/bestpractices.xml/OneDeclarationPerLine"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/OneDeclarationPerLine"><properties><propertyname="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 theexception parameter declared by that catch block. The stack trace of the originalexception could be lost, which makes the thrown exception less informative.

To preserve the stack trace, the original exception may be used as the cause ofthe new exception, usingThrowable#initCause, or passed as a constructor argumentto the new exception. It may also be preserved usingThrowable#addSuppressed.The rule actually assumes that any method or constructor that takes the originalexception as argument preserves the original stack trace.

The rule allowsInvocationTargetException andPrivilegedActionException to bereplaced by their cause exception. The discarded part of the stack trace is in thosecases only JDK-internal code, which is not very useful. The rule also ignores exceptionswhose name starts withignored.

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

Example(s):

publicclassFoo{voidgood(){try{Integer.parseInt("a");}catch(Exceptione){thrownewException(e);// Ok, this initializes the cause of the new exception}try{Integer.parseInt("a");}catch(Exceptione){throw(IllegalStateException)newIllegalStateException().initCause(e);// second possibility to create exception chain.}}voidwrong(){try{Integer.parseInt("a");}catch(Exceptione){// Violation: this only preserves the message and not the stack tracethrownewException(e.getMessage());}}}

Use this rule by referencing it:

<ruleref="category/java/bestpractices.xml/PreserveStackTrace"/>

PrimitiveWrapperInstantiation

Since: PMD 6.37.0

Priority: Medium (3)

Reports usages of primitive wrapper constructors. They are deprecatedsince Java 9 and should not be used. Even before Java 9, they canbe replaced with usage of the corresponding staticvalueOf 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 creatinga new instance each time.

Note that forBoolean, the named constantsBoolean.TRUE andBoolean.FALSEare preferred instead ofBoolean.valueOf.

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

Example(s):

publicclassFoo{privateIntegerZERO=newInteger(0);// violationprivateIntegerZERO1=Integer.valueOf(0);// betterprivateIntegerZERO1=0;// even better}

Use this rule by referencing it:

<ruleref="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 ensurestable encoding behavior between different JVMs, programs, and servers. Using the platform’sdefault charset makes the code less portable and might lead to unexpected behavior when runningon different systems.

Additional, since Java 18, the default charset for these APIs is consistently UTF-8(seeJEP 400). While this reduces unexpected behavioron 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):

publicclassFoo{voidbad()throwsIOException{newInputStreamReader(inputStream);// violationnewOutputStreamWriter(outputStream);// violationURLEncoder.encode("test string");// violation (deprecated)newPrintStream(outputStream);// violationnewPrintWriter("output.txt");// violationnewScanner(inputStream);// violationnewFormatter();// violation"test".getBytes();// violationnewByteArrayOutputStream().toString();// violationnewFileReader("input.txt");// violationnewFileWriter("output.txt");// violation}voidgood()throwsIOException{newInputStreamReader(inputStream,StandardCharsets.UTF_8);// oknewOutputStreamWriter(outputStream,StandardCharsets.UTF_8);// okURLEncoder.encode("test string",StandardCharsets.UTF_8);// oknewPrintStream(outputStream,true,StandardCharsets.UTF_8);// oknewPrintWriter("output.txt",StandardCharsets.UTF_8);// oknewScanner(inputStream,StandardCharsets.UTF_8);// oknewFormatter(Locale.US);// ok"test".getBytes(StandardCharsets.UTF_8);// oknewByteArrayOutputStream().toString(StandardCharsets.UTF_8);// oknewFileReader("input.txt",StandardCharsets.UTF_8);// oknewFileWriter("output.txt",StandardCharsets.UTF_8);// ok}}

Use this rule by referencing it:

<ruleref="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):

publicclassFooimplementsEnumeration{privateintx=42;publicbooleanhasMoreElements(){returntrue;}publicObjectnextElement(){returnString.valueOf(i++);}}

Use this rule by referencing it:

<ruleref="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):

publicclassFoo{voidbar(){Hashtableh=newHashtable();}}

Use this rule by referencing it:

<ruleref="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):

importjava.util.Vector;publicclassFoo{voidbar(){Vectorv=newVector();}}

Use this rule by referencing it:

<ruleref="category/java/bestpractices.xml/ReplaceVectorWithList"/>

SimplifiableTestAssertion

Since: PMD 6.37.0

Priority: Medium (3)

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

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

Example(s):

importorg.junit.Test;importstaticorg.junit.Assert.*;classSomeTestClass{Objecta,b;@TestvoidtestMethod(){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:

<ruleref="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 floweasier to follow. This can be achieved by adding adefault case, or,if the switch is on an enum type, by ensuring there is one switch branchfor each enum constant.

This rule doesn’t consider Switch Statements, that use Pattern Matching, since for these thecompiler 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):

classFoo{{intx=2;switch(x){case1:intj=6;case2:intj=8;// missing default: here}}}

Use this rule by referencing it:

<ruleref="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 inthe codebase even in production code. By using a logger one can enable/disable this behaviour atwill (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):

classFoo{Loggerlog=Logger.getLogger(Foo.class.getName());publicvoidtestA(){System.out.println("Entering test");// Better use thislog.fine("Entering test");}}

Use this rule by referencing it:

<ruleref="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 ofassertEquals(), 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):

publicclassFoo{@TestpublicvoidtestSomething(){assertEquals("foo","bar");// Use the form:// assertEquals("Foo does not equals bar", "foo", "bar");// instead}}

Use this rule by referencing it:

<ruleref="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 whichit 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):

publicclassMyTestCase{// Ok@TestpublicvoidtestMyCaseWithOneAssert(){booleanmyVar=false;assertFalse("should be false",myVar);}// Bad, too many asserts (assuming max=1)@TestpublicvoidtestMyCaseWithMoreAsserts(){booleanmyVar=false;assertFalse("myVar should be false",myVar);assertEquals("should equals false",false,myVar);}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/UnitTestContainsTooManyAsserts"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UnitTestContainsTooManyAsserts"><properties><propertyname="maximumAsserts"value="1"/><propertyname="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 assertwith 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):

publicclassFoo{@TestpublicvoidtestSomething(){Barb=findBar();// This is better than having a NullPointerException// assertNotNull("bar not found", b);b.work();}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/UnitTestShouldIncludeAssert"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UnitTestShouldIncludeAssert"><properties><propertyname="extraAssertMethodNames"value=""/></properties></rule>

UnitTestShouldUseAfterAnnotation

Since: PMD 4.0

Priority: Medium (3)

This rule detects methods calledtearDown() 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 calledtearDown().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 afterall tests in the class, respectively.
• TestNG provides the annotations@AfterMethod and@AfterClass to execute methods after each test or aftertests 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')orpmd-java:typeIs('org.junit.jupiter.api.AfterEach')orpmd-java:typeIs('org.junit.jupiter.api.AfterAll')orpmd-java:typeIs('org.testng.annotations.AfterClass')orpmd-java:typeIs('org.testng.annotations.AfterMethod')])](: Make sure this is a JUnit 4/5 or TestNG class :)[../MethodDeclaration[pmd-java:hasAnnotation('org.junit.Test')orpmd-java:hasAnnotation('org.junit.jupiter.api.Test')orpmd-java:hasAnnotation('org.testng.annotations.Test')]]

Example(s):

publicclassMyTest{publicvoidtearDown(){bad();}}publicclassMyTest2{@AfterpublicvoidtearDown(){good();}}

Use this rule by referencing it:

<ruleref="category/java/bestpractices.xml/UnitTestShouldUseAfterAnnotation"/>

UnitTestShouldUseBeforeAnnotation

Since: PMD 4.0

Priority: Medium (3)

This rule detects methods calledsetUp() 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 calledsetUp().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 alltests in the class, respectively.
• TestNG provides the annotations@BeforeMethod and@BeforeClass to execute methods before each test or beforetests 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')orpmd-java:typeIs('org.junit.jupiter.api.BeforeEach')orpmd-java:typeIs('org.junit.jupiter.api.BeforeAll')orpmd-java:typeIs('org.testng.annotations.BeforeMethod')orpmd-java:typeIs('org.testng.annotations.BeforeClass')])](: Make sure this is a JUnit 4/5 or TestNG class :)[../MethodDeclaration[pmd-java:hasAnnotation('org.junit.Test')orpmd-java:hasAnnotation('org.junit.jupiter.api.Test')orpmd-java:hasAnnotation('org.testng.annotations.Test')]]

Example(s):

publicclassMyTest{publicvoidsetUp(){bad();}}publicclassMyTest2{@BeforepublicvoidsetUp(){good();}}

Use this rule by referencing it:

<ruleref="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)orpmd-java:typeIs('junit.framework.TestCase')](: a junit 3 method :)/ClassBody/MethodDeclaration[@Visibility="public"andstarts-with(@Name,'test')andnot(ModifierList/Annotation[pmd-java:typeIs('org.junit.Test')orpmd-java:typeIs('org.junit.jupiter.api.Test')orpmd-java:typeIs('org.junit.jupiter.api.RepeatedTest')orpmd-java:typeIs('org.junit.jupiter.api.TestFactory')orpmd-java:typeIs('org.junit.jupiter.api.TestTemplate')orpmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest')orpmd-java:typeIs('org.testng.annotations.Test')])]

Example(s):

publicclassMyTest{publicvoidtestBad(){doSomething();}@TestpublicvoidtestGood(){doSomething();}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/UnitTestShouldUseTestAnnotation"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UnitTestShouldUseTestAnnotation"><properties><propertyname="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(newString[]{"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):

importjava.util.Arrays;classC{static{Arrays.asList(newString[]{"foo","bar",});// should beArrays.asList("foo","bar");}}

Use this rule by referencing it:

<ruleref="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 violationduring the current run. That means you cannot run this rule separately from other rules, it mustalways be run with all the rules that could produce a warning. This is most likely not a problem, asyou 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 awarning there that must be suppressed. In the future we might be able to check for other common oneslike@SuppressWarnings("unchecked") or"fallthrough".

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

Example(s):

publicclassSomething{// Unless some rule triggered on the following line, this rule will report the comment:privatevoidfoo(){}// NOPMD}

Use this rule by referencing it:

<ruleref="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 ofthe variable.

The rule tracks assignments to fields ofthis, 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 subsumesUnusedLocalVariable, andUnusedFormalParameter.Those violations are filteredout by default, in case you already have enabled those rules, but may be enabled with the propertyreportUnusedVariables. Variables whose name starts withignored orunused are filtered out, asis 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 thethrow statement, in particular,things likeassert statements, or NullPointerExceptions on dereference are ignored.
• The rule cannot resolve assignments across constructors, when they’re called with the specialthis(...) 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):

classA{// this field initializer is redundant,// it is always overwritten in the constructorintf=1;A(intf){this.f=f;}}

classB{intmethod(inti,intj){// this initializer is redundant,// it is overwritten in all branches of the `if`intk=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 kif(i<j)k=i;elsek=j;returnj;}}

classC{intmethod(){inti=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}}

classC{// variables that are truly unused (at most assigned to, but never accessed)// are only reported if property `reportUnusedVariables` is truevoidmethod(intparam){}// for example this method parameter// even then, you can suppress the violation with an annotation:voidmethod(@SuppressWarning("unused")intparam){}// 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(Somethingignored=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(Exceptione){// 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:

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

<ruleref="category/java/bestpractices.xml/UnusedAssignment"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UnusedAssignment"><properties><propertyname="checkUnusedPrefixIncrement"value="false"/><propertyname="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 withignored orunused 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 thecheckAll property totrue. 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):

publicclassFoo{privatevoidbar(Stringhowdy){// howdy is not used}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/UnusedFormalParameter"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UnusedFormalParameter"><properties><propertyname="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 ruleS1065.

This rule is defined by the following XPath expression:

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

Example(s):

classExample{voidmain(){lbl1:{// violation: Label "lbl1" is not nusedintx=1;System.out.println(x);}}}

Use this rule by referencing it:

<ruleref="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 withignored orunused are filtered out.

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

Example(s):

publicclassFoo{publicvoiddoSomething(){inti=5;// Unused}}

Use this rule by referencing it:

<ruleref="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 theenclosing class has any annotation. Annotations often enable a framework (such as dependency injection, mockingor 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):

publicclassSomething{privatestaticintFOO=2;// Unusedprivateinti=5;// Unusedprivateintj=6;publicintaddOne(){returnj++;}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/UnusedPrivateField"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UnusedPrivateField"><properties><propertyname="ignoredFieldNames"value="serialVersionUID,serialPersistentFields"/><propertyname="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):

publicclassSomething{privatevoidfoo(){}// unused}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/UnusedPrivateMethod"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UnusedPrivateMethod"><properties><propertyname="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):

publicclassFoo{voidgood(){Listfoo=getList();if(foo.isEmpty()){// blah}}voidbad(){Listfoo=getList();if(foo.size()==0){// blah}}}

Use this rule by referencing it:

<ruleref="category/java/bestpractices.xml/UseCollectionIsEmpty"/>

UseEnumCollections

Since: PMD 7.3.0

Priority: Medium (3)

Wherever possible, useEnumSet orEnumMap instead ofHashSet andHashMap when the keysare 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 keytype is an enum type.

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

Example(s):

importjava.util.EnumMap;importjava.util.HashSet;enumExample{A,B,C;publicstaticSet<Example>newSet(){returnnewHashSet<>();// Could be EnumSet.noneOf(Example.class)}publicstatic<V>Map<Example,V>newMap(){returnnewHashMap<>();// Could be new EnumMap<>(Example.class)}}

Use this rule by referencing it:

<ruleref="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 toCharset.forName(...)since no scan across the internalCharset 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):

publicclassUseStandardCharsets{publicvoidrun(){// looking up the charset dynamicallytry(OutputStreamWriterosw=newOutputStreamWriter(out,Charset.forName("UTF-8"))){osw.write("test");}// best to use StandardCharsetstry(OutputStreamWriterosw=newOutputStreamWriter(out,StandardCharsets.UTF_8)){osw.write("test");}}}

Use this rule by referencing it:

<ruleref="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 endof the statement. It avoids the need of explicitly closing the resources in a finally block. Additionally exceptionsare better handled: If an exception occurred both in thetry block andfinally block, then the exception fromthe try block was suppressed. With thetry-with-resources statement, the exception thrown from the try-block ispreserved.

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

Example(s):

publicclassTryWithResources{publicvoidrun(){InputStreamin=null;try{in=openInputStream();inti=in.read();}catch(IOExceptione){e.printStackTrace();}finally{try{if(in!=null)in.close();}catch(IOExceptionignored){// ignored}}// better use try-with-resourcestry(InputStreamin2=openInputStream()){inti=in2.read();}}}

This rule has the following properties:

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

<ruleref="category/java/bestpractices.xml/UseTryWithResources"/>

Use this rule and customize it:

<ruleref="category/java/bestpractices.xml/UseTryWithResources"><properties><propertyname="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 syntacticsugar provides flexibility for users of these methods and constructors, allowing them to avoidhaving to deal with the creation of an array.

Byte arrays in any method and String arrays inpublic 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"]orClassType[pmd-java:typeIs('java.lang.Byte')])]orVariableId[ArrayDimensions]and(PrimitiveType[not(@Kind="byte")]orClassType[not(pmd-java:typeIs('java.lang.Byte'))])]

Example(s):

publicclassFoo{publicvoidfoo(Strings,Object[]args){// Do something here...}publicvoidbar(Strings,Object...args){// Ahh, varargs tastes much better...}}

Use this rule by referencing it:

<ruleref="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 isapparent that it loops forever, whereaswhile (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 :)orcount(BooleanLiteral)>=1andcount(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 :)orcount(BooleanLiteral)>=1andcount(VariableAccess)=0]]

Example(s):

publicclassExample{{while(true){}// allowedwhile(false){}// disalloweddo{}while(true);// disalloweddo{}while(false);// disalloweddo{}while(false|false);// disalloweddo{}while(false||false);// disallowed}}

Use this rule by referencing it: