Since: PMD 1.03
Priority: Medium (3)
Avoid assignments in operands; this can make code more complicated and harder to read.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.AssignmentInOperandRule
Example(s):
publicvoidbar(){intx=2;if((x=getX())==3){System.out.println("3!");}}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| allowIf | false | Allow assignment within the conditional expression of an if statement |
| allowFor | false | Allow assignment within the conditional expression of a for statement |
| allowWhile | false | Allow assignment within the conditional expression of a while statement |
| allowIncrementDecrement | false | Allow increment or decrement operators within the conditional expression of an if, for, or while statement |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/AssignmentInOperand"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/AssignmentInOperand"><properties><propertyname="allowIf"value="false"/><propertyname="allowFor"value="false"/><propertyname="allowWhile"value="false"/><propertyname="allowIncrementDecrement"value="false"/></properties></rule>Since: PMD 2.2
Priority: Medium (3)
Identifies a possible unsafe usage of a static field.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.AssignmentToNonFinalStaticRule
Example(s):
publicclassStaticField{staticintx;publicFinalFields(inty){x=y;// unsafe}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AssignmentToNonFinalStatic"/>Since: PMD 4.1
Priority: Medium (3)
Methods such asgetDeclaredConstructors(),getDeclaredMethods(), andgetDeclaredFields() alsoreturn private constructors, methods and fields. These can be made accessible by callingsetAccessible(true).This gives access to normally protected data which violates the principle of encapsulation.
This rule detects calls tosetAccessible and finds possible accessibility alterations.If the call tosetAccessible is wrapped within aPrivilegedAction, then the access alterationis assumed to be deliberate and is not reported.
Note that with Java 17 the Security Manager, which is used forPrivilegedAction execution,is deprecated:JEP 411: Deprecate the Security Manager for Removal.For future-proof code, deliberate access alteration should be suppressed using the usualsuppression methods (e.g. by using@SuppressWarnings annotation).
This rule is defined by the following XPath expression:
//MethodCall[pmd-java:matchesSig("java.lang.reflect.AccessibleObject#setAccessible(boolean)")orpmd-java:matchesSig("_#setAccessible(java.lang.reflect.AccessibleObject[],boolean)")][not(ArgumentList/BooleanLiteral[@True=false()])](: exclude anonymous privileged action classes :)[not(ancestor::ConstructorCall[1][pmd-java:typeIs('java.security.PrivilegedAction')]/AnonymousClassDeclaration)](: exclude inner privileged action classes :)[not(ancestor::ClassDeclaration[1][pmd-java:typeIs('java.security.PrivilegedAction')])](: exclude privileged action lambdas :)[not(ancestor::LambdaExpression[pmd-java:typeIs('java.security.PrivilegedAction')])]Example(s):
importjava.lang.reflect.Constructor;importjava.lang.reflect.Field;importjava.lang.reflect.Method;importjava.security.AccessController;importjava.security.PrivilegedAction;publicclassViolation{privatevoidinvalidSetAccessCalls()throwsNoSuchMethodException,SecurityException{Constructor<?>constructor=this.getClass().getDeclaredConstructor(String.class);// call to forbidden setAccessibleconstructor.setAccessible(true);MethodprivateMethod=this.getClass().getDeclaredMethod("aPrivateMethod");// call to forbidden setAccessibleprivateMethod.setAccessible(true);// deliberate accessibility alterationStringprivateField=AccessController.doPrivileged(newPrivilegedAction<String>(){@OverridepublicStringrun(){try{Fieldfield=Violation.class.getDeclaredField("aPrivateField");field.setAccessible(true);return(String)field.get(null);}catch(ReflectiveOperationException|SecurityExceptione){thrownewRuntimeException(e);}}});}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidAccessibilityAlteration"/>Since: PMD 3.4
Priority: Medium High (2)
Maximum Language Version: Java 1.3
Use of the termassert will conflict with newer versions of Java since it is a reserved word.
Since Java 1.4, the tokenassert became a reserved word and using it as an identifier willresult in a compilation failure for Java 1.4 and later. This rule is therefore only usefulfor old Java code before Java 1.4. It can be used to identify problematic code prior to a Java update.
This rule is defined by the following XPath expression:
//VariableId[@Name='assert']Example(s):
publicclassA{publicclassFoo{Stringassert="foo";}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidAssertAsIdentifier"/>Since: PMD 5.0
Priority: Medium High (2)
Using a branching statement as the last part of a loop may be a bug, and/or is confusing.Ensure that the usage is not a bug, or consider using another approach.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.AvoidBranchingStatementAsLastInLoopRule
Example(s):
// unusual use of branching statement in a loopfor(inti=0;i<10;i++){if(i*i<=25){continue;}break;}// this makes more sense...for(inti=0;i<10;i++){if(i*i>25){break;}}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| checkBreakLoopTypes | for , do , while | List of loop types in which break statements will be checked |
| checkContinueLoopTypes | for , do , while | List of loop types in which continue statements will be checked |
| checkReturnLoopTypes | for , do , while | List of loop types in which return statements will be checked |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop"><properties><propertyname="checkBreakLoopTypes"value="for,do,while"/><propertyname="checkContinueLoopTypes"value="for,do,while"/><propertyname="checkReturnLoopTypes"value="for,do,while"/></properties></rule>Since: PMD 3.0
Priority: Medium (3)
The method Object.finalize() is called by the garbage collector on an object when garbage collection determinesthat there are no more references to the object. It should not be invoked by application logic.
Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
This rule is defined by the following XPath expression:
//MethodCall[pmd-java:matchesSig("java.lang.Object#finalize()")](: it's ok inside finalize :)[not(SuperExpressionandancestor::*[self::MethodDeclarationorself::Initializer][1][@Name='finalize'][@Arity=0][VoidType])]Example(s):
voidfoo(){Barb=newBar();b.finalize();}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidCallingFinalize"/>Since: PMD 1.8
Priority: Medium (3)
Code should never throw NullPointerExceptions under normal circumstances. A catch block may hide theoriginal error, causing other, more subtle problems later on.
This rule is defined by the following XPath expression:
//CatchClause/CatchParameter/ClassType[pmd-java:typeIsExactly('java.lang.NullPointerException')]Example(s):
publicclassFoo{voidbar(){try{// do something}catch(NullPointerExceptionnpe){}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidCatchingNPE"/>Since: PMD 1.2
Priority: Medium (3)
Catching Throwable errors is not recommended since its scope is very broad. It includes runtime issues such asOutOfMemoryError that should be exposed and managed separately.
This rule is defined by the following XPath expression:
//CatchParameter[ClassType[pmd-java:typeIsExactly('java.lang.Throwable')]]/VariableIdExample(s):
publicvoidbar(){try{// do something}catch(Throwableth){// should not catch Throwableth.printStackTrace();}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidCatchingThrowable"/>Since: PMD 3.4
Priority: Medium (3)
One might assume that the result of "new BigDecimal(0.1)" is exactly equal to 0.1, but it is actuallyequal to .1000000000000000055511151231257827021181583404541015625.This is because 0.1 cannot be represented exactly as a double (or as a binary fraction of any finitelength). Thus, the long value that is being passed in to the constructor is not exactly equal to 0.1,appearances notwithstanding.
The (String) constructor, on the other hand, is perfectly predictable: ‘new BigDecimal("0.1")’ isexactly equal to 0.1, as one would expect. Therefore, it is generally recommended that the(String) constructor be used in preference to this one.
This rule is defined by the following XPath expression:
//ConstructorCall[pmd-java:matchesSig('java.math.BigDecimal#new(double)')]Example(s):
BigDecimalbd=newBigDecimal(1.123);// loss of precision, this would trigger the ruleBigDecimalbd=newBigDecimal("1.123");// preferred approachBigDecimalbd=newBigDecimal(12);// preferred approach, ok for integer valuesUse this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidDecimalLiteralsInBigDecimalConstructor"/>Since: PMD 1.0
Priority: Medium (3)
Code containing duplicate String literals can usually be improved by declaring the String as a constant field.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.AvoidDuplicateLiteralsRule
Example(s):
privatevoidbar(){buz("Howdy");buz("Howdy");buz("Howdy");buz("Howdy");}privatevoidbuz(Stringx){}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| maxDuplicateLiterals | 4 | Max duplicate literals |
| minimumLength | 3 | Minimum string length to check |
| skipAnnotations | false | Skip literals within annotations |
| exceptionList | List of literals to ignore. A literal is ignored if its image can be found in this list. Components of this list should not be surrounded by double quotes. |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/AvoidDuplicateLiterals"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/AvoidDuplicateLiterals"><properties><propertyname="maxDuplicateLiterals"value="4"/><propertyname="minimumLength"value="3"/><propertyname="skipAnnotations"value="false"/><propertyname="exceptionList"value=""/></properties></rule>Since: PMD 3.4
Priority: Medium High (2)
Maximum Language Version: Java 1.4
Use of the termenum will conflict with newer versions of Java since it is a reserved word.
Since Java 1.5, the tokenenum became a reserved word and using it as an identifier willresult in a compilation failure for Java 1.5 and later. This rule is therefore only usefulfor old Java code before Java 1.5. It can be used to identify problematic code prior to a Java update.
This rule is defined by the following XPath expression:
//VariableId[@Name='enum']Example(s):
publicclassA{publicclassFoo{Stringenum="foo";}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidEnumAsIdentifier"/>Since: PMD 3.0
Priority: Medium (3)
It can be confusing to have a field name with the same name as a method. While this is permitted,having information (field) and actions (method) is not clear naming. Developers versed inSmalltalk often prefer this approach as the methods denote accessor methods.
This rule is defined by the following XPath expression:
//FieldDeclaration/VariableDeclarator/VariableId[some$methodin../../..[self::ClassBodyorself::EnumBody]/MethodDeclarationsatisfieslower-case(@Name)=lower-case($method/@Name)]Example(s):
publicclassFoo{Objectbar;// bar is data or an action or both?voidbar(){}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidFieldNameMatchingMethodName"/>Since: PMD 3.0
Priority: Medium (3)
It is somewhat confusing to have a field name matching the declaring type name.This probably means that type and/or field names should be chosen more carefully.
This rule is defined by the following XPath expression:
//FieldDeclaration/VariableDeclarator/VariableId[lower-case(@Name)=lower-case(ancestor::ClassDeclaration[1]/@SimpleName)]Example(s):
publicclassFooextendsBar{intfoo;// There is probably a better name that can be used}publicinterfaceOperation{intOPERATION=1;// There is probably a better name that can be used}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidFieldNameMatchingTypeName"/>Since: PMD 3.0
Priority: Medium (3)
Each caught exception type should be handled in its own catch clause.
This rule is defined by the following XPath expression:
//CatchParameter/following-sibling::Block//InfixExpression[@Operator='instanceof']/VariableAccess[@Name=./ancestor::Block/preceding-sibling::CatchParameter/@Name]Example(s):
try{// Avoid this// do something}catch(Exceptionee){if(eeinstanceofIOException){cleanup();}}try{// Prefer this:// do something}catch(IOExceptionee){cleanup();}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidInstanceofChecksInCatchClause"/>Since: PMD 4.2.6
Priority: Medium (3)
Avoid using hard-coded literals in conditional statements. By declaring them as static variablesor private members with descriptive names maintainability is enhanced. By default, the literals "-1" and "0" are ignored.More exceptions can be defined with the property "ignoreMagicNumbers".
The rule doesn’t consider deeper expressions by default, but this can be enabled via the propertyignoreExpressions.With this property set to false, if-conditions likei == 1 + 5 are reported as well. Note that in that case,the property ignoreMagicNumbers is not taken into account, if there are multiple literals involved in such an expression.
This rule is defined by the following XPath expression:
(: simple case - no deep expressions - this is always executed :)//IfStatement/*[1]/*[pmd-java:nodeIs('Literal')][not(pmd-java:nodeIs('NullLiteral'))][not(pmd-java:nodeIs('BooleanLiteral'))][empty(index-of(tokenize($ignoreMagicNumbers,'\s*,\s*'),@Image))]|(: consider also deeper expressions :)//IfStatement[$ignoreExpressions=false()]/*[1]//*[not(self::UnaryExpression[@Operator='-'])]/*[pmd-java:nodeIs('Literal')][not(pmd-java:nodeIs('NullLiteral'))][not(pmd-java:nodeIs('BooleanLiteral'))][empty(index-of(tokenize($ignoreMagicNumbers,'\s*,\s*'),@Image))]|(: consider negative literals :)//IfStatement[$ignoreExpressions=false()]/*[1]//UnaryExpression[@Operator='-']/*[pmd-java:nodeIs('Literal')][not(pmd-java:nodeIs('NullLiteral'))][not(pmd-java:nodeIs('BooleanLiteral'))][empty(index-of(tokenize($ignoreMagicNumbers,'\s*,\s*'),concat('-',@Image)))]|(: consider multiple literals in expressions :)//IfStatement[$ignoreExpressions=false()]/*[1][count(*[pmd-java:nodeIs('Literal')][not(pmd-java:nodeIs('NullLiteral'))][not(pmd-java:nodeIs('BooleanLiteral'))])>1]Example(s):
privatestaticfinalintMAX_NUMBER_OF_REQUESTS=10;publicvoidcheckRequests(){if(i==10){// magic number, buried in a methoddoSomething();}if(i==MAX_NUMBER_OF_REQUESTS){// preferred approachdoSomething();}if(aString.indexOf('.')!=-1){}// magic number -1, by default ignoredif(aString.indexOf('.')>=0){}// alternative approachif(aDouble>0.0){}// magic number 0.0if(aDouble>=Double.MIN_VALUE){}// preferred approach// with rule property "ignoreExpressions" set to "false"if(i==pos+5){}// violation: magic number 5 within an (additive) expressionif(i==pos+SUFFIX_LENGTH){}// preferred approachif(i==5&&"none".equals(aString)){}// 2 violations: magic number 5 and literal "none"}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| ignoreMagicNumbers | -1,0 | Comma-separated list of magic numbers, that should be ignored |
| ignoreExpressions | true | If true, only literals in simple if conditions are considered. Otherwise literals in expressions are checked, too. |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/AvoidLiteralsInIfCondition"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/AvoidLiteralsInIfCondition"><properties><propertyname="ignoreMagicNumbers"value="-1,0"/><propertyname="ignoreExpressions"value="true"/></properties></rule>Since: PMD 4.2.6
Priority: Medium High (2)
Statements in a catch block that invoke accessors on the exception without using the informationonly add to code size. Either remove the invocation, or use the return result.
This rule is defined by the following XPath expression:
//CatchClause/Block/ExpressionStatement/MethodCall[pmd-java:matchesSig("java.lang.Throwable#getMessage()")orpmd-java:matchesSig("java.lang.Throwable#getLocalizedMessage()")orpmd-java:matchesSig("java.lang.Throwable#getCause()")orpmd-java:matchesSig("java.lang.Throwable#getStackTrace()")orpmd-java:matchesSig("java.lang.Object#toString()")]Example(s):
publicvoidbar(){try{// do something}catch(SomeExceptionse){se.getMessage();}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidLosingExceptionInformation"/>Since: PMD 4.2
Priority: Medium High (2)
The use of multiple unary operators may be problematic, and/or confusing.Ensure that the intended usage is not a bug, or consider simplifying the expression.
This rule is defined by the following XPath expression:
(: Only report on the toplevel one :)//UnaryExpression[UnaryExpressionandnot(parent::UnaryExpression)]Example(s):
// These are typo bugs, or at best needlessly complex and confusing:inti=--1;intj=+-+1;intz=~~2;booleanb=!!true;booleanc=!!!true;// These are better:inti=1;intj=-1;intz=2;booleanb=true;booleanc=false;// And these just make your brain hurt:inti=~-2;intj=-~7;Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/AvoidMultipleUnaryOperators"/>Since: PMD 3.9
Priority: Medium (3)
Integer literals should not start with zero since this denotes that the rest of literal will beinterpreted as an octal value.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.AvoidUsingOctalValuesRule
Example(s):
inti=012;// set i with 10 not 12intj=010;// set j with 8 not 10k=i*j;// set k with 80 not 120This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| strict | false | Detect violations between 00 and 07 |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/AvoidUsingOctalValues"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/AvoidUsingOctalValues"><properties><propertyname="strict"value="false"/></properties></rule>Since: PMD 3.8
Priority: Medium High (2)
The null check is broken since it will throw a NullPointerException itself.It is likely that you used || instead of && or vice versa.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.BrokenNullCheckRule
Example(s):
publicStringbar(Stringstring){// should be &&if(string!=null||!string.equals(""))returnstring;// should be ||if(string==null&&string.equals(""))returnstring;}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/BrokenNullCheck"/>Since: PMD 4.2.5
Priority: Medium (3)
Super should be called at the start of the method
This rule is defined by the following XPath expression:
//ClassDeclaration[pmd-java:typeIs('android.app.Activity')orpmd-java:typeIs('android.app.Application')orpmd-java:typeIs('android.app.Service')]//MethodDeclaration[@Name=('onCreate','onConfigurationChanged','onPostCreate','onPostResume','onRestart','onRestoreInstanceState','onResume','onStart')][not(Block/*[1]/MethodCall[SuperExpression][@MethodName=ancestor::MethodDeclaration/@Name])]Example(s):
importandroid.app.Activity;importandroid.os.Bundle;publicclassDummyActivityextendsActivity{publicvoidonCreate(Bundlebundle){// missing call to super.onCreate(bundle)foo();}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/CallSuperFirst"/>Since: PMD 4.2.5
Priority: Medium (3)
Super should be called at the end of the method
This rule is defined by the following XPath expression:
//ClassDeclaration[pmd-java:typeIs('android.app.Activity')orpmd-java:typeIs('android.app.Application')orpmd-java:typeIs('android.app.Service')]//MethodDeclaration[@Name=('finish','onDestroy','onPause','onSaveInstanceState','onStop','onTerminate')][not(Block/*[last()]/MethodCall[SuperExpression][@MethodName=ancestor::MethodDeclaration/@Name])]Example(s):
importandroid.app.Activity;publicclassDummyActivityextendsActivity{publicvoidonPause(){foo();// missing call to super.onPause()}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/CallSuperLast"/>Since: PMD 5.0
Priority: Medium (3)
The skip() method may skip a smaller number of bytes than requested. Check the returned value to find out if it was the case or not.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.CheckSkipResultRule
Example(s):
publicclassFoo{privateFileInputStream_s=newFileInputStream("file");publicvoidskip(intn)throwsIOException{_s.skip(n);// You are not sure that exactly n bytes are skipped}publicvoidskipExactly(intn)throwsIOException{while(n!=0){longskipped=_s.skip(n);if(skipped==0)thrownewEOFException();n-=skipped;}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/CheckSkipResult"/>Since: PMD 3.4
Priority: Medium (3)
When deriving an array of a specific class from your Collection, one should provide an array ofthe same class as the parameter of thetoArray() method. Doing otherwise will resultin aClassCastException.
This rule is defined by the following XPath expression:
//CastExpression[ArrayType/ClassType[not(pmd-java:typeIsExactly('java.lang.Object'))]]/MethodCall[pmd-java:matchesSig("java.util.Collection#toArray()")]Example(s):
Collectionc=newArrayList();Integerobj=newInteger(1);c.add(obj);// this would trigger the rule (and throw a ClassCastException if executed)Integer[]a=(Integer[])c.toArray();// this is fine and will not trigger the ruleInteger[]b=(Integer[])c.toArray(newInteger[0]);Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ClassCastExceptionWithToArray"/>Since: PMD 5.4.0
Priority: Medium (3)
The java manual says "By convention, classes that implement this interface should overrideObject.clone (which is protected) with a public method."
This rule is defined by the following XPath expression:
//MethodDeclaration[not(pmd-java:modifiers()="public")][@Name='clone'][@Arity=0]Example(s):
publicclassFooimplementsCloneable{@OverrideprotectedObjectclone()throwsCloneNotSupportedException{// Violation, must be public}}publicclassFooimplementsCloneable{@OverrideprotectedFooclone(){// Violation, must be public}}publicclassFooimplementsCloneable{@OverridepublicObjectclone()// Ok}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/CloneMethodMustBePublic"/>Since: PMD 1.9
Priority: Medium (3)
The method clone() should only be implemented if the class implements the Cloneable interface with the exception ofa final method that only throws CloneNotSupportedException.
The rule can also detect, if the class implements or extends a Cloneable class.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.CloneMethodMustImplementCloneableRule
Example(s):
publicclassMyClass{publicObjectclone()throwsCloneNotSupportedException{returnfoo;}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/CloneMethodMustImplementCloneable"/>Since: PMD 5.4.0
Priority: Medium (3)
Minimum Language Version: Java 1.5
If a class implementsCloneable the return type of the methodclone() must be the class name. That way, the callerof the clone method doesn’t need to cast the returned clone to the correct type.
Note: Such a covariant return type is only possible with Java 1.5 or higher.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Name='clone'][@Arity=0][ClassType[1]/@SimpleName!=ancestor::ClassDeclaration[1]/@SimpleName]Example(s):
publicclassFooimplementsCloneable{@OverrideprotectedObjectclone(){// Violation, Object must be Foo}}publicclassFooimplementsCloneable{@OverridepublicFooclone(){//Ok}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/CloneMethodReturnTypeMustMatchClassName"/>Since: PMD 1.2.2
Priority: Medium (3)
Ensure that resources (likejava.sql.Connection,java.sql.Statement, andjava.sql.ResultSet objectsand any subtype ofjava.lang.AutoCloseable) are always closed after use.Failing to do so might result in resource leaks.
Note: It suffices to configure the super type, e.g.java.lang.AutoCloseable, so that this rule automatically triggerson any subtype (e.g.java.io.FileInputStream). Additionally specifyingjava.sql.Connection helps in detectingthe types, if the type resolution / auxclasspath is not correctly setup.
Note: Since PMD 6.16.0 the default value for the propertytypes containsjava.lang.AutoCloseable and detectsnow cases where the standardjava.io.*Stream classes are involved. In order to restore the old behaviour,just remove "AutoCloseable" from the types.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.CloseResourceRule
Example(s):
publicclassBar{publicvoidwithSQL(){Connectionc=pool.getConnection();try{// do stuff}catch(SQLExceptionex){// handle exception}finally{// oops, should close the connection using 'close'!// c.close();}}publicvoidwithFile(){InputStreamfile=newFileInputStream(newFile("/tmp/foo"));try{intc=file.in();}catch(IOExceptione){// handle exception}finally{// TODO: close file}}}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| closeTargets | Methods which may close this resource | |
| types | java.lang.AutoCloseable , java.sql.Connection , java.sql.Statement , java.sql.ResultSet | Affected types |
| closeAsDefaultTarget | true | Consider ‘close’ as a target by default |
| allowedResourceTypes | java.io.ByteArrayOutputStream , java.io.ByteArrayInputStream , java.io.StringWriter , java.io.CharArrayWriter , java.util.stream.Stream , java.util.stream.IntStream , java.util.stream.LongStream , java.util.stream.DoubleStream | Exact class names that do not need to be closed |
| closeNotInFinally | false | Detect if ‘close’ (or other closeTargets) is called outside of a finally-block |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/CloseResource"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/CloseResource"><properties><propertyname="closeTargets"value=""/><propertyname="types"value="java.lang.AutoCloseable,java.sql.Connection,java.sql.Statement,java.sql.ResultSet"/><propertyname="closeAsDefaultTarget"value="true"/><propertyname="allowedResourceTypes"value="java.io.ByteArrayOutputStream,java.io.ByteArrayInputStream,java.io.StringWriter,java.io.CharArrayWriter,java.util.stream.Stream,java.util.stream.IntStream,java.util.stream.LongStream,java.util.stream.DoubleStream"/><propertyname="closeNotInFinally"value="false"/></properties></rule>Since: PMD 3.2
Priority: Medium (3)
Useequals() to compare object references; avoid comparing them with==.
Since comparing objects with named constants is useful in some cases (eg, whendefining constants for sentinel values), the rule ignores comparisons againstfields with all-caps name (egthis == SENTINEL), which is a common namingconvention for constant fields.
You may allow some types to be compared by reference by listing the exceptionsin thetypesThatCompareByReference property.
This rule is defined by the following XPath expression:
//InfixExpression[@Operator=("==","!=")][count(*[not(self::NullLiteral)][pmd-java:typeIs('java.lang.Object')][not(some$tin$typesThatCompareByReferencesatisfiespmd-java:typeIs($t))])=2][not(ancestor::MethodDeclaration[1][@Name="equals"])](: Is not a field access with an all-caps identifier :)[not(FieldAccess[upper-case(@Name)=@Name]orVariableAccess[upper-case(@Name)=@Name])]Example(s):
classFoo{booleanbar(Stringa,Stringb){returna==b;}}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| typesThatCompareByReference | java.lang.Enum , java.lang.Class | List of canonical type names for which reference comparison is allowed. |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/CompareObjectsWithEquals"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/CompareObjectsWithEquals"><properties><propertyname="typesThatCompareByReference"value="java.lang.Enum,java.lang.Class"/></properties></rule>Since: PMD 6.36.0
Priority: Medium (3)
Reports comparisons with double and floatNaN (Not-a-Number) values.These arespecifiedto have unintuitive behavior: NaN is considered unequal to itself.This means a check likesomeDouble == Double.NaN will always returnfalse, even ifsomeDouble is really the NaN value. To test whether avalue is the NaN value, one should instead useDouble.isNaN(someDouble)(orFloat.isNaN). The!= operator should be treated similarly.Finally, comparisons likesomeDouble <= Double.NaN are nonsensicaland will always evaluate to false.
This rule has been renamed from "BadComparison" in PMD 6.36.0.
This rule is defined by the following XPath expression:
//InfixExpression[@Operator=("==","!=","<=",">=","<",">")]/FieldAccess[@Name='NaN'and(pmd-java:typeIs('double')orpmd-java:typeIs('float'))]Example(s):
booleanx=(y==Double.NaN);Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ComparisonWithNaN"/>Since: PMD 7.1.0
Priority: Medium (3)
Reports a confusing argument passed to a varargs method.
This can occur when an array is passed as a single varargs argument, when the array type is not exactly thetype of array that the varargs method expects. If, that array is a subtype of the component type of the expectedarray type, then it might not be clear what value the called varargs method will receive.For instance if you have:
voidvarargs(Object...parm);and call it like so:
varargs(newString[]{"a"});it is not clear whether you intended the method to receive the valuenew Object[]{ new String[] {"a"} } orjustnew String[] {"a"} (the latter happens). This confusion occurs becauseString[] is both a subtypeofObject[] and ofObject. To clarify your intent in this case, use a cast or pass individual elements like so:
// varargs call// parm will be `new Object[] { "a" }`varargs("a");// non-varargs call// parm will be `new String[] { "a" }`varargs((Object[])newString[]{"a"});// varargs call// parm will be `new Object[] { new String[] { "a" } }`varargs((Object)newString[]{"a"});Another confusing case is when you passnull as the varargs argument. Here it is not clear whether you intendedto pass an array with a single null element, or a null array (the latter happens). This can similarly be clarifiedwith a cast.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.ConfusingArgumentToVarargsMethodRule
Example(s):
importjava.util.Arrays;abstractclassC{abstractvoidvarargs(Object...args);static{varargs(newString[]{"a"});varargs(null);}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ConfusingArgumentToVarargsMethod"/>Since: PMD 1.04
Priority: High (1)
Reports calls to overridable methods onthis during object initialization. Theseare invoked on an incompletely constructed object and can be difficult to debug if overridden.This is because the subclass usually assumes that the superclass is completely initializedin all methods. If that is not the case, bugs can appear in the constructor, for instance,some fields that are still null may cause a NullPointerException or be stored somewhereelse to blow up later.
To avoid this problem, only use methods that are static, private, or final in constructors.Note that those methods also must not call overridable methods transitively to be safe.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.ConstructorCallsOverridableMethodRule
Example(s):
publicclassSeniorClass{publicSeniorClass(){toString();//may throw NullPointerException if overridden}publicStringtoString(){return"IAmSeniorClass";}}publicclassJuniorClassextendsSeniorClass{privateStringname;publicJuniorClass(){super();//Automatic call leads to NullPointerExceptionname="JuniorClass";}publicStringtoString(){returnname.toUpperCase();}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ConstructorCallsOverridableMethod"/>Since: PMD 6.13.0
Priority: Medium (3)
The method appears to be a test case since it has public or default visibility,non-static access, no arguments, no return value, has no annotations, but is amember of a class that has one or more JUnit test cases. If it is a utilitymethod, it should likely have private visibility. If it is an ignored test, itshould be annotated with @Test and @Ignore.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.DetachedTestCaseRule
Example(s):
publicclassMyTest{@TestpublicvoidsomeTest(){}// violation: Not annotatedpublicvoidsomeOtherTest(){}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DetachedTestCase"/>Since: PMD 4.2
Priority: Medium High (2)
Calls toSystem.gc(),Runtime.getRuntime().gc(), andSystem.runFinalization() are not advised.Code should have the same behavior whether the garbage collection is disabled using the option-Xdisableexplicitgc or not.
Moreover, "modern" JVMs do a very good job handling garbage collections. If memory usage issues unrelated to memoryleaks develop within an application, it should be dealt with JVM options rather than within the code itself.
This rule is defined by the following XPath expression:
//MethodCall[pmd-java:matchesSig("java.lang.System#gc()")orpmd-java:matchesSig("java.lang.Runtime#gc()")orpmd-java:matchesSig("java.lang.System#runFinalization()")orpmd-java:matchesSig("java.lang.Runtime#runFinalization()")]Example(s):
publicclassGCCall{publicGCCall(){// Explicit gc call !System.gc();}publicvoiddoSomething(){// Explicit gc call !Runtime.getRuntime().gc();}publicexplicitGCcall(){// Explicit gc call !System.gc();}publicvoiddoSomething(){// Explicit gc call !Runtime.getRuntime().gc();}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DoNotCallGarbageCollectionExplicitly"/>Since: PMD 6.0.0
Priority: Medium (3)
Extend Exception or RuntimeException instead of Throwable.
This rule is defined by the following XPath expression:
//ClassDeclaration/ExtendsList/ClassType[pmd-java:typeIsExactly('java.lang.Throwable')]Example(s):
publicclassFooextendsThrowable{}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DoNotExtendJavaLangThrowable"/>Since: PMD 4.2.6
Priority: Medium (3)
Use Environment.getExternalStorageDirectory() instead of "/sdcard"
This rule is defined by the following XPath expression:
//StringLiteral[starts-with(@Image,'"/sdcard')]Example(s):
publicclassMyActivityextendsActivity{protectedvoidfoo(){StringstorageLocation="/sdcard/mypackage";// hard-coded, poor approachstorageLocation=Environment.getExternalStorageDirectory()+"/mypackage";// preferred approach}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DoNotHardCodeSDCard"/>Since: PMD 4.1
Priority: Medium (3)
Web applications should not callSystem.exit(), since only the web container or theapplication server should stop the JVM. Otherwise a web application would terminate all other applicationsrunning on the same application server.
This rule also checks for the equivalent callsRuntime.getRuntime().exit() andRuntime.getRuntime().halt().
This rule has been renamed from "DoNotCallSystemExit" in PMD 6.29.0.
This rule is defined by the following XPath expression:
//(MethodDeclaration[@MainMethod=false()]|Initializer)//MethodCall[pmd-java:matchesSig("java.lang.System#exit(int)")orpmd-java:matchesSig("java.lang.Runtime#exit(int)")orpmd-java:matchesSig("java.lang.Runtime#halt(int)")]Example(s):
publicvoidbar(){System.exit(0);// never call this when running in an application server!}publicvoidfoo(){Runtime.getRuntime().exit(0);// never stop the JVM manually, the container will do this.}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DoNotTerminateVM"/>Since: PMD 4.2
Priority: Medium Low (4)
Throwing exceptions within a ‘finally’ block is confusing since they may mask other exceptionsor code defects.Note: This is a PMD implementation of the Lint4j rule "A throw in a finally block"
This rule is defined by the following XPath expression:
//FinallyClause[descendant::ThrowStatement]Example(s):
publicclassFoo{publicvoidbar(){try{// Here do some stuff}catch(Exceptione){// Handling the issue}finally{// is this really a good idea ?thrownewException();}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DoNotThrowExceptionInFinally"/>Since: PMD 1.5
Priority: Medium Low (4)
Avoid importing anything from the ‘sun.*’ packages. These packages are not portableand are likely to change.
If you find yourself having to depend on Sun APIs, confine this dependency to assmall a scope as possible, for instance by writing a stable wrapper class aroundthe unstable API. You can then suppress this rule in the implementation of the wrapper.
This rule is defined by the following XPath expression:
//ImportDeclaration[starts-with(@ImportedName,'sun.')]Example(s):
importsun.misc.foo;publicclassFoo{}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DontImportSun"/>Since: PMD 4.3
Priority: Medium (3)
Don’t use floating point for loop indices. If you must use floating point, use doubleunless you’re certain that float provides enough precision and you have a compellingperformance need (space or time).
This rule is defined by the following XPath expression:
//ForStatement/ForInit//VariableId[pmd-java:typeIs('float')]Example(s):
publicclassCount{publicstaticvoidmain(String[]args){finalintSTART=2000000000;intcount=0;for(floatf=START;f<START+50;f++)count++;//Prints 0 because (float) START == (float) (START + 50).System.out.println(count);//The termination test misbehaves due to floating point granularity.}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/DontUseFloatTypeForLoopIndices"/>Since: PMD 0.1
Priority: Medium (3)
Empty Catch Block finds instances where an exception is caught, but nothing is done.In most circumstances, this swallows an exception which should either be acted onor reported.
This rule is defined by the following XPath expression:
//CatchClause[Block[count(*)=0and($allowCommentedBlocks=false()or@containsComment=false())]andCatchParameter/VariableId[not(matches(@Name,$allowExceptionNameRegex))]]Example(s):
publicvoiddoSomething(){try{FileInputStreamfis=newFileInputStream("/tmp/bugger");}catch(IOExceptionioe){// not good}}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| allowCommentedBlocks | false | Empty blocks containing comments will be skipped |
| allowExceptionNameRegex | ^(ignored|expected)$ | Empty blocks catching exceptions with names matching this regular expression will be skipped |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/EmptyCatchBlock"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/EmptyCatchBlock"><properties><propertyname="allowCommentedBlocks"value="false"/><propertyname="allowExceptionNameRegex"value="^(ignored|expected)$"/></properties></rule>Since: PMD 1.5
Priority: Medium (3)
Empty finalize methods serve no purpose and should be removed. Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Name='finalize'][@Arity=0]/Block[not(*)]Example(s):
publicclassFoo{protectedvoidfinalize(){}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/EmptyFinalizer"/>Since: PMD 1.9
Priority: High (1)
Tests for null should not use the equals() method. The ‘==’ operator should be used instead.
This rule is defined by the following XPath expression:
//MethodCall[@MethodName="equals"andArgumentList[count(*)=1andNullLiteral]]Example(s):
Stringx="foo";if(x.equals(null)){// bad formdoSomething();}if(x==null){// preferreddoSomething();}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/EqualsNull"/>Since: PMD 1.5
Priority: Medium (3)
If the finalize() is implemented, its last action should be to call super.finalize. Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Name="finalize"][@Arity=0]/Block/*[last()][not(MethodCall[@MethodName="finalize"]/SuperExpression)][not(FinallyClause/Block/ExpressionStatement/MethodCall[@MethodName="finalize"]/SuperExpression)]Example(s):
protectedvoidfinalize(){something();// neglected to call super.finalize()}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/FinalizeDoesNotCallSuperFinalize"/>Since: PMD 1.5
Priority: Medium (3)
If the finalize() is implemented, it should do something besides just calling super.finalize(). Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Name='finalize'][@Arity=0][Block[@Size=1]/ExpressionStatement/MethodCall[@MethodName="finalize"][SuperExpression]]Example(s):
protectedvoidfinalize(){super.finalize();}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/FinalizeOnlyCallsSuperFinalize"/>Since: PMD 1.5
Priority: Medium (3)
Methods named finalize() should not have parameters. It is confusing and most likely an attempt tooverload Object.finalize(). It will not be called by the VM.
Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Name='finalize'][@Arity>0]Example(s):
publicclassFoo{// this is confusing and probably a bugprotectedvoidfinalize(inta){}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/FinalizeOverloaded"/>Since: PMD 1.1
Priority: Medium (3)
When overriding the finalize(), the new method should be set as protected. If made public,other classes may invoke it at inappropriate times.
Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Visibility!="protected"][@Name='finalize'][@Arity=0]Example(s):
publicvoidfinalize(){// do something}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/FinalizeShouldBeProtected"/>Since: PMD 2.0
Priority: Medium (3)
Avoid idempotent operations - they have no effect.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.IdempotentOperationsRule
Example(s):
publicclassFoo{publicvoidbar(){intx=2;x=x;}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/IdempotentOperations"/>Since: PMD 3.0
Priority: Medium (3)
Switch statements without break or return statements for each case optionmay indicate problematic behaviour. Empty cases are ignored as these indicatean intentional fall-through.
You can ignore a violation by commenting// fallthrough before the case labelwhich is reached by fallthrough, or with@SuppressWarnings("fallthrough").
This rule has been renamed from "MissingBreakInSwitch" in PMD 6.37.0.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.ImplicitSwitchFallThroughRule
Example(s):
publicvoidbar(intstatus){switch(status){caseCANCELLED:doCancelled();// break; hm, should this be commented out?caseNEW:doNew();// is this really a fall-through?// what happens if you add another case after this one?caseREMOVED:doRemoved();// fallthrough - this comment just clarifies that you want a fallthroughcaseOTHER:// empty case - this is interpreted as an intentional fall-throughcaseERROR:doErrorHandling();break;}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ImplicitSwitchFallThrough"/>Since: PMD 2.0
Priority: Medium Low (4)
Avoid instantiating an object just to call getClass() on it; use the .class public member instead.
This rule is defined by the following XPath expression:
//MethodCall[@MethodName='getClass'][ConstructorCall]Example(s):
// replace thisClassc=newString().getClass();// with this:Classc=String.class;Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/InstantiationToGetClass"/>Since: PMD 5.5.0
Priority: Low (5)
Check for messages in slf4j and log4j2 (since 6.19.0) loggers with non matching number of arguments and placeholders.
Since 6.32.0 in addition to parameterized message placeholders ({}) also format specifiers of string formattedmessages are supported (%s).
This rule has been renamed from "InvalidSlf4jMessageFormat" in PMD 6.19.0.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.InvalidLogMessageFormatRule
Example(s):
LOGGER.error("forget the arg {}");LOGGER.error("forget the arg %s");LOGGER.error("too many args {}","arg1","arg2");LOGGER.error("param {}","arg1",newIllegalStateException("arg"));//The exception is shown separately, so is correct.Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/InvalidLogMessageFormat"/>Since: PMD 1.0
Priority: Medium (3)
Avoid jumbled loop incrementers - it’s usually a mistake, and is confusing even if intentional.
This rule is defined by the following XPath expression:
//ForStatement[not(ForInit)orForInit//VariableId/@Name!=ForUpdate//VariableAccess/@Name][ForUpdate//VariableAccess[@AccessType='WRITE']/@Name=ancestor::ForStatement/ForInit//VariableId/@Name]Example(s):
publicclassJumbledIncrementerRule1{publicvoidfoo(){for(inti=0;i<10;i++){// only references 'i'for(intk=0;k<20;i++){// references both 'i' and 'k'System.out.println("Hello");}}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/JumbledIncrementer"/>Since: PMD 1.0
Priority: Medium (3)
In JUnit 3, the setUp method is used to set up all data entities required in running tests.The tearDown method is used to clean up all data entities required in running tests.You should not misspell method name if you want your test to set up and clean up everything correctly.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.JUnitSpellingRule
Example(s):
importjunit.framework.*;publicclassFooextendsTestCase{publicvoidsetup(){}// oops, should be setUppublicvoidTearDown(){}// oops, should be tearDown}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/JUnitSpelling"/>Since: PMD 1.0
Priority: Medium (3)
The suite() method in a JUnit test needs to be both public and static.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.JUnitStaticSuiteRule
Example(s):
importjunit.framework.*;publicclassFooextendsTestCase{publicvoidsuite(){}// oops, should be static}importjunit.framework.*;publicclassFooextendsTestCase{privatestaticvoidsuite(){}// oops, should be public}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/JUnitStaticSuite"/>Since: PMD 1.5
Priority: Medium (3)
A method should not have the same name as its containing class.This would be confusing as it would look like a constructor.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Name=ancestor::ClassDeclaration/@SimpleName]Example(s):
publicclassMyClass{publicMyClass(){}// this is OK because it is a constructorpublicvoidMyClass(){}// this is bad because it is a method}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/MethodWithSameNameAsEnclosingClass"/>Since: PMD 3.5
Priority: Medium (3)
The null check here is misplaced. If the variable is null aNullPointerException will be thrown.Either the check is useless (the variable will never benull) or it is incorrect.
This rule is defined by the following XPath expression:
//InfixExpression[@Operator='&&']/InfixExpression[@Operator='!='](: one side is null :)[NullLiteral](: other side checks for the variable used somewhere in the first child of conditional and expression :)[VariableAccess][some$varinpreceding-sibling::*//VariableAccess[parent::MethodCallorparent::FieldAccess][not(ancestor::InfixExpression[@Operator='||'])]/@Namesatisfies$var=VariableAccess/@Name]/VariableAccess|//InfixExpression[@Operator='||']/InfixExpression[@Operator='=='](: one side is null :)[NullLiteral](: other side checks for the variable used somewhere in the first child of conditional or expression :)[VariableAccess][some$varinpreceding-sibling::*//VariableAccess[parent::MethodCallorparent::FieldAccess][not(ancestor::InfixExpression[@Operator='&&'])]/@Namesatisfies$var=VariableAccess/@Name]/VariableAccessExample(s):
publicclassFoo{voidbar(){if(a.equals(baz)&&a!=null){}// a could be null, misplaced null checkif(a!=null&&a.equals(baz)){}// correct null check}}publicclassFoo{voidbar(){if(a.equals(baz)||a==null){}// a could be null, misplaced null checkif(a==null||a.equals(baz)){}// correct null check}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/MisplacedNullCheck"/>Since: PMD 3.0
Priority: Medium (3)
Serializable classes should provide a serialVersionUID field.The serialVersionUID field is also needed for abstract base classes. Each individual class in the inheritancechain needs an own serialVersionUID field. See alsoShould an abstract class have a serialVersionUID.
This rule is defined by the following XPath expression:
//ClassDeclaration[@Interface=false()][count(ClassBody/FieldDeclaration/VariableDeclarator/VariableId[@Name='serialVersionUID'])=0][(ImplementsList|ExtendsList)/ClassType[pmd-java:typeIs('java.io.Serializable')]]Example(s):
publicclassFooimplementsjava.io.Serializable{Stringname;// Define serialization id to avoid serialization related bugs// i.e., public static final long serialVersionUID = 4328743;}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/MissingSerialVersionUID"/>Since: PMD 3.0
Priority: Medium (3)
A class that has private constructors and does not have any static methods or fields cannot be used.
When one of the private constructors is annotated with one of the annotations, then the class is not considerednon-instantiatable anymore and no violation will be reported.See the propertyannotations.
This rule is defined by the following XPath expression:
let$topLevelClass:=/*/ClassDeclarationreturnlet$isLombokUtility:=exists($topLevelClass[pmd-java:hasAnnotation('lombok.experimental.UtilityClass')])return$topLevelClass[(: non-instantiable :)$isLombokUtilityor((: no lombok produced constructors :)not(pmd-java:hasAnnotation('lombok.NoArgsConstructor')orpmd-java:hasAnnotation('lombok.RequiredArgsConstructor')orpmd-java:hasAnnotation('lombok.AllArgsConstructor')orpmd-java:hasAnnotation('lombok.Builder'))and(: or has non-default constructors … :)ClassBody/ConstructorDeclarationand(: … but only private … :)not(ClassBody/ConstructorDeclaration[@Visibility!="private"])and(: … and none annotated … :)(every$xin$annotationssatisfiesnot(ClassBody/ConstructorDeclaration/ModifierList/Annotation[pmd-java:typeIs($x)])))][(: With no visible static methods … :)not(ClassBody/MethodDeclaration[($isLombokUtilityorpmd-java:modifiers()="static")and@Visibility!="private"])and(: … nor fields … :)not(ClassBody/FieldDeclaration[($isLombokUtilityorpmd-java:modifiers()="static")and@Visibility!="private"])and(: … no nested classes, that are non-private and static … :)not(ClassBody/ClassDeclaration[pmd-java:modifiers()="static"and@Visibility!="private"](: … and a non-private method returning the outer class type … :)[(ClassBody/MethodDeclaration[@Visibility!="private"][descendant::ReturnStatement/*[1][pmd-java:typeIs(ancestor::ClassDeclaration[@Nested=false()]/@BinaryName)]])or((: … or the inner class extends the outer class :)ExtendsList/ClassType[@SimpleName=ancestor::ClassDeclaration[@Nested=false()]/@SimpleName])])]Example(s):
// This class is unusable, since it cannot be// instantiated (private constructor),// and no static method can be called.publicclassFoo{privateFoo(){}voidfoo(){}}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| annotations | org.springframework.beans.factory.annotation.Autowired , javax.inject.Inject , com.google.inject.Inject , lombok.Builder | If a constructor is annotated with one of these annotations, then the class is ignored. |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/MissingStaticMethodInNonInstantiatableClass"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/MissingStaticMethodInNonInstantiatableClass"><properties><propertyname="annotations"value="org.springframework.beans.factory.annotation.Autowired,javax.inject.Inject,com.google.inject.Inject,lombok.Builder"/></properties></rule>Since: PMD 2.0
Priority: Medium High (2)
Normally only one logger is used in each class. This rule supports slf4j, log4j, Java Util Logging andlog4j2 (since 6.19.0).
This rule is defined by the following XPath expression:
//ClassDeclaration[count(ClassBody/FieldDeclaration/ClassType[pmd-java:typeIs("org.apache.log4j.Logger")orpmd-java:typeIs("org.apache.logging.log4j.Logger")orpmd-java:typeIs("java.util.logging.Logger")orpmd-java:typeIs("org.slf4j.Logger")])>1]Example(s):
publicclassFoo{Loggerlog=Logger.getLogger(Foo.class.getName());// It is very rare to see two loggers on a class, normally// log information is multiplexed by levelsLoggerlog2=Logger.getLogger(Foo.class.getName());}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/MoreThanOneLogger"/>Since: PMD 1.5
Priority: Medium (3)
A non-case label (e.g. a named break/continue label) was present in a switch statement or switch expression.This is legal, but confusing. It is easy to mix up the case labels and the non-case labels.
Note: This rule was renamed fromNonCaseLabelInSwitchStatement with PMD 7.7.0.
This rule is defined by the following XPath expression:
//(SwitchStatement|SwitchExpression)//LabeledStatementExample(s):
publicclassFoo{voidbar(inta){switch(a){case1:// do somethingmylabel:// this is legal, but confusing!break;default:break;}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/NonCaseLabelInSwitch"/>Deprecated
This rule has been renamed. Use instead:NonCaseLabelInSwitch
Deprecated
Since: PMD 1.5
Priority: Medium (3)
A non-case label (e.g. a named break/continue label) was present in a switch statement or switch expression.This is legal, but confusing. It is easy to mix up the case labels and the non-case labels.
Note: This rule was renamed fromNonCaseLabelInSwitchStatement with PMD 7.7.0.
This rule is defined by the following XPath expression:
//(SwitchStatement|SwitchExpression)//LabeledStatementExample(s):
publicclassFoo{voidbar(inta){switch(a){case1:// do somethingmylabel:// this is legal, but confusing!break;default:break;}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/NonCaseLabelInSwitchStatement"/>Since: PMD 1.1
Priority: Medium (3)
If a class is marked asSerializable, then all fields need to be serializable as well. In order to excludea field, it can be marked as transient. Static fields are not considered.
This rule reports all fields, that are not serializable.
If a class implements the methods to perform manual serialization (writeObject,readObject) or usesa replacement object (writeReplace,readResolve) then this class is ignored.
Note: This rule has been revamped with PMD 6.52.0. It was previously called "BeanMembersShouldSerialize".The propertyprefix has been deprecated, since in a serializable class all fields have to beserializable regardless of the name.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.NonSerializableClassRule
Example(s):
classBuzzimplementsjava.io.Serializable{privatestaticfinallongserialVersionUID=1L;privatetransientintsomeFoo;// good, it's transientprivatestaticintotherFoo;// also OK, it's staticprivatejava.io.FileInputStreamstream;// bad - FileInputStream is not serializablepublicvoidsetStream(FileInputStreamstream){this.stream=stream;}publicintgetSomeFoo(){returnthis.someFoo;}}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| checkAbstractTypes | false | Enable to verify fields with abstract types like abstract classes, interfaces, generic types or java.lang.Object. Enabling this might lead to more false positives, since the concrete runtime type can actually be serializable. |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/NonSerializableClass"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/NonSerializableClass"><properties><propertyname="checkAbstractTypes"value="false"/></properties></rule>Since: PMD 1.5
Priority: Medium (3)
A non-static initializer block will be called any time a constructor is invoked (just prior toinvoking the constructor). While this is a valid language construct, it is rarely used and isconfusing.
This rule is defined by the following XPath expression:
//Initializer[@Static=false()][not(ancestor::*[3][self::ConstructorCallorself::EnumConstant])]Example(s):
publicclassMyClass{// this block gets run before any call to a constructor{System.out.println("I am about to construct myself");}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/NonStaticInitializer"/>Since: PMD 1.02
Priority: Medium (3)
Assigning a "null" to a variable (outside of its declaration) is usually bad form. Sometimes, this typeof assignment is an indication that the programmer doesn’t completely understand what is going on in the code.
NOTE: This sort of assignment may used in some cases to dereference objects and encourage garbage collection.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.NullAssignmentRule
Example(s):
publicvoidbar(){Objectx=null;// this is OKx=newObject();// big, complex piece of code herex=null;// this is not required// big, complex piece of code here}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/NullAssignment"/>Since: PMD 0.4
Priority: Medium (3)
Override both public boolean Object.equals(Object other), and public int Object.hashCode(), or override neither. Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitly delegating to your superclass.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.OverrideBothEqualsAndHashcodeRule
Example(s):
publicclassBar{// poor, missing a hashcode() methodpublicbooleanequals(Objecto){// do some comparison}}publicclassBaz{// poor, missing an equals() methodpublicinthashCode(){// return some hash value}}publicclassFoo{// perfect, both methods providedpublicbooleanequals(Objectother){// do some comparison}publicinthashCode(){// return some hash value}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/OverrideBothEqualsAndHashcode"/>Since: PMD 1.4
Priority: Medium High (2)
Object clone() should be implemented with super.clone().
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.ProperCloneImplementationRule
Example(s):
classFoo{publicObjectclone(){returnnewFoo();// This is bad}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ProperCloneImplementation"/>Since: PMD 3.3
Priority: Medium (3)
A logger should normally be defined private static final and be associated with the correct class.private final Log log; is also allowed for rare cases where loggers need to be passed around,with the restriction that the logger needs to be passed into the constructor.
This rule is defined by the following XPath expression:
//FieldDeclaration[ClassType[pmd-java:typeIs($loggerClass)]][(: check modifiers :)(not(pmd-java:modifiers()='private')ornot(pmd-java:modifiers()='final'))(: check logger name :)or(pmd-java:modifiers()='static'andVariableDeclarator/VariableId/@Name!=$staticLoggerName)or(not(pmd-java:modifiers()='static')andVariableDeclarator/VariableId/@Name!=$loggerName)(: check logger argument type matches class or enum name :)or.//ArgumentList/ClassLiteral/ClassType/@SimpleName!=ancestor::ClassDeclaration/@SimpleNameor.//ArgumentList/ClassLiteral/ClassType/@SimpleName!=ancestor::EnumDeclaration/@SimpleName(: special case - final logger initialized inside constructor :)or(VariableDeclarator/@Initializer=false()andnot(pmd-java:modifiers()='static')andnot(ancestor::ClassBody/ConstructorDeclaration//AssignmentExpression[@Operator='='][FieldAccess[1]/@Name=$loggerNameorVariableAccess[1]/@Name=$loggerName][*[2][@Name=ancestor::ConstructorDeclaration//FormalParameter/VariableId/@Name]]))]Example(s):
publicclassFoo{privatestaticfinalLogLOG=LogFactory.getLog(Foo.class);// proper wayprotectedLogLOG=LogFactory.getLog(Testclass.class);// wrong approach}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| staticLoggerName | LOG | Name of the static Logger variable |
| loggerName | log | Name of the Logger instance variable |
| loggerClass | org.apache.commons.logging.Log | Class name of the logger |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/ProperLogger"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/ProperLogger"><properties><propertyname="staticLoggerName"value="LOG"/><propertyname="loggerName"value="log"/><propertyname="loggerClass"value="org.apache.commons.logging.Log"/></properties></rule>Since: PMD 6.37.0
Priority: High (1)
For any method that returns an collection (such as an array, Collection or Map), it is better to returnan empty one rather than a null reference. This removes the need for null checking all results and avoidsinadvertent NullPointerExceptions.
See Effective Java, 3rd Edition, Item 54: Return empty collections or arrays instead of null
This rule is defined by the following XPath expression:
//ReturnStatement/NullLiteral[ancestor::MethodDeclaration[1][ArrayTypeorClassType[pmd-java:typeIs('java.util.Collection')orpmd-java:typeIs('java.util.Map')]]][not(./ancestor::LambdaExpression)]Example(s):
publicclassExample{// Not a good idea...publicint[]badBehavior(){// ...returnnull;}// Good behaviorpublicString[]bonnePratique(){//...returnnewString[0];}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ReturnEmptyCollectionRatherThanNull"/>Since: PMD 1.05
Priority: Medium (3)
Avoid returning from a finally block, this can discard exceptions.
This rule is defined by the following XPath expression:
//FinallyClause//ReturnStatementexcept//FinallyClause//(MethodDeclaration|LambdaExpression)//ReturnStatementExample(s):
publicclassBar{publicStringfoo(){try{thrownewException("My Exception");}catch(Exceptione){throwe;}finally{return"A. O. K.";// return not recommended here}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/ReturnFromFinallyBlock"/>Since: PMD 2.0
Priority: Medium (3)
Be sure to specify a Locale when creating SimpleDateFormat instances to ensure that locale-appropriateformatting is used.
This rule is defined by the following XPath expression:
//ConstructorCall[pmd-java:typeIs('java.text.SimpleDateFormat')][ArgumentList/@Size=1]Example(s):
publicclassFoo{// Should specify Locale.US (or whatever)privateSimpleDateFormatsdf=newSimpleDateFormat("pattern");}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/SimpleDateFormatNeedsLocale"/>Since: PMD 5.4
Priority: Medium High (2)
Some classes contain overloaded getInstance. The problem with overloaded getInstance methodsis that the instance created using the overloaded method is not cached and so,for each call and new objects will be created for every invocation.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.SingleMethodSingletonRule
Example(s):
publicclassSingleton{privatestaticSingletonsingleton=newSingleton();privateSingleton(){}publicstaticSingletongetInstance(){returnsingleton;}publicstaticSingletongetInstance(Objectobj){Singletonsingleton=(Singleton)obj;returnsingleton;//violation}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/SingleMethodSingleton"/>Since: PMD 5.4
Priority: Medium High (2)
A singleton class should only ever have one instance. Failure to checkwhether an instance has already been created may result in multipleinstances being created.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.SingletonClassReturningNewInstanceRule
Example(s):
classSingleton{privatestaticSingletoninstance=null;publicstaticSingletongetInstance(){synchronized(Singleton.class){returnnewSingleton();// this should be assigned to the field}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/SingletonClassReturningNewInstance"/>Since: PMD 4.1
Priority: Medium (3)
According to the J2EE specification, an EJB should not have any static fieldswith write access. However, static read-only fields are allowed. This ensures properbehavior especially when instances are distributed by the container on several JREs.
This rule is defined by the following XPath expression:
//ClassDeclaration[ImplementsList/ClassType[pmd-java:typeIs('javax.ejb.SessionBean')orpmd-java:typeIs('javax.ejb.EJBHome')orpmd-java:typeIs('javax.ejb.EJBLocalObject')orpmd-java:typeIs('javax.ejb.EJBLocalHome')orpmd-java:typeIs('javax.ejb.EJBObject')]]/ClassBody/FieldDeclaration[pmd-java:modifiers()='static'][not(pmd-java:modifiers()='final')]Example(s):
publicclassSomeEJBextendsEJBObjectimplementsEJBLocalHome{privatestaticintCountA;// poor, field can be editedprivatestaticfinalintCountB;// preferred, read-only access}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/StaticEJBFieldShouldBeFinal"/>Since: PMD 3.9
Priority: Medium Low (4)
Individual character values provided as initialization arguments will be converted into integers.This can lead to internal buffer sizes that are larger than expected. Some examples:
new StringBuffer() // 16new StringBuffer(6) // 6new StringBuffer("hello world") // 11 + 16 = 27new StringBuffer('A') // chr(A) = 65new StringBuffer("A") // 1 + 16 = 17new StringBuilder() // 16new StringBuilder(6) // 6new StringBuilder("hello world") // 11 + 16 = 27new StringBuilder('C') // chr(C) = 67new StringBuilder("A") // 1 + 16 = 17This rule is defined by the following XPath expression:
//ConstructorCall[ArgumentList/*[pmd-java:typeIsExactly('char')]][pmd-java:matchesSig('java.lang.StringBuilder#new(int)')orpmd-java:matchesSig('java.lang.StringBuffer#new(int)')]Example(s):
// misleading instantiation, these buffers// are actually sized to 99 characters longStringBuffersb1=newStringBuffer('c');StringBuildersb2=newStringBuilder('c');// in these forms, just single characters are allocatedStringBuffersb3=newStringBuffer("c");StringBuildersb4=newStringBuilder("c");Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/StringBufferInstantiationWithChar"/>Since: PMD 2.0
Priority: Medium High (2)
The method name and parameter number are suspiciously close toObject.equals, which can denote anintention to override it. However, the method does not overrideObject.equals, but overloads it instead.OverloadingObject.equals method is confusing for other programmers, error-prone and hard to maintain,especially when using inheritance, because@Override annotations used in subclasses can provide a falsesense of security. For more information onObject.equals method, see Effective Java, 3rd Edition,Item 10: Obey the general contract when overriding equals.
This rule is defined by the following XPath expression:
//MethodDeclaration[@Name='equals'][(@Arity=1andnot(FormalParameters/FormalParameter[pmd-java:typeIsExactly('java.lang.Object')])ornot(PrimitiveType[@Kind='boolean']))or(@Arity=2andPrimitiveType[@Kind='boolean']andFormalParameters/FormalParameter[pmd-java:typeIsExactly('java.lang.Object')]andnot(pmd-java:hasAnnotation('java.lang.Override')))]|//MethodDeclaration[@Name='equal'][@Arity=1andFormalParameters/FormalParameter[pmd-java:typeIsExactly('java.lang.Object')]]Example(s):
publicclassFoo{publicintequals(Objecto){// oops, this probably was supposed to be boolean equals}publicbooleanequals(Strings){// oops, this probably was supposed to be equals(Object)}publicbooleanequals(Objecto1,Objecto2){// oops, this probably was supposed to be equals(Object)}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/SuspiciousEqualsMethodName"/>Since: PMD 1.5
Priority: Medium (3)
The method name and return type are suspiciously close to hashCode(), which may denote an intentionto override the hashCode() method.
This rule is defined by the following XPath expression:
//MethodDeclaration[lower-case(@Name)='hashcode'and@Name!='hashCode'and@Arity=0andPrimitiveType[@Kind='int']]Example(s):
publicclassFoo{publicinthashcode(){// oops, this probably was supposed to be 'hashCode'}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/SuspiciousHashcodeMethodName"/>Since: PMD 1.5
Priority: Medium (3)
A suspicious octal escape sequence was found inside a String literal.The Java language specification (section 3.10.6) says an octalescape sequence inside a literal String shall consist of a backslashfollowed by:
OctalDigit | OctalDigit OctalDigit | ZeroToThree OctalDigit OctalDigitAny octal escape sequence followed by non-octal digits can be confusing,e.g. "\038" is interpreted as the octal escape sequence "\03" followed bythe literal character "8".
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.SuspiciousOctalEscapeRule
Example(s):
publicvoidfoo(){// interpreted as octal 12, followed by character '8'System.out.println("suspicious: \128");}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/SuspiciousOctalEscape"/>Since: PMD 3.0
Priority: Medium (3)
Test classes typically end with the suffix "Test", "Tests" or "TestCase". Having a non-test class with that nameis not a good practice, since most people will assume it is a test case. Test classes have test methodsnamed "testXXX" (JUnit3) or use annotations (e.g.@Test).
The suffix can be configured using the propertytestClassPattern. To disable the detection of possible test classesby name, set this property to an empty string.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.TestClassWithoutTestCasesRule
Example(s):
//Consider changing the name of the class if it is not a test//Consider adding test methods if it is a testpublicclassCarTest{publicstaticvoidmain(String[]args){// do something}// code}This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| testClassPattern | ^(?:.*\.)?Test[^\.]*$|^(?:.*\.)?.*Tests?$|^(?:.*\.)?.*TestCase$ | Test class name pattern to identify test classes by their fully qualified name. An empty pattern disables test class detection by name. Since PMD 6.51.0. |
Use this rule with the default properties by just referencing it:
<ruleref="category/java/errorprone.xml/TestClassWithoutTestCases"/>Use this rule and customize it:
<ruleref="category/java/errorprone.xml/TestClassWithoutTestCases"><properties><propertyname="testClassPattern"value="^(?:.*\.)?Test[^\.]*$|^(?:.*\.)?.*Tests?$|^(?:.*\.)?.*TestCase$"/></properties></rule>Since: PMD 1.5
Priority: Medium (3)
Do not use "if" statements whose conditionals are always true or always false.
This rule is defined by the following XPath expression:
//IfStatement[BooleanLiteral[1]]Example(s):
publicclassFoo{publicvoidclose(){if(true){// fixed conditional, not recommended// ...}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UnconditionalIfStatement"/>Since: PMD 3.0
Priority: Medium (3)
A JUnit test assertion with a boolean literal is unnecessary since it always will evaluate to the same thing.Consider using flow control (in case ofassertTrue(false) or similar) or simply removingstatements likeassertTrue(true) andassertFalse(false). If you just want a test to halt after findingan error, use thefail() method and provide an indication message of why it did.
This rule is defined by the following XPath expression:
//ClassDeclaration[pmd-java:typeIs('junit.framework.TestCase')or.//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')]]//MethodCall[@MethodName=('assertTrue','assertFalse')][ArgumentList[BooleanLiteralorUnaryExpression[@Operator='!'][BooleanLiteral]]]Example(s):
publicclassSimpleTestextendsTestCase{publicvoidtestX(){assertTrue(true);// serves no real purpose - remove it}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UnnecessaryBooleanAssertion"/>Since: PMD 3.3
Priority: Medium (3)
Using equalsIgnoreCase() is faster than using toUpperCase/toLowerCase().equals()
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.UnnecessaryCaseChangeRule
Example(s):
booleananswer1=buz.toUpperCase().equals("BAZ");// should be buz.equalsIgnoreCase("BAZ")booleananswer2=buz.toUpperCase().equalsIgnoreCase("BAZ");// another unnecessary toUpperCase()Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UnnecessaryCaseChange"/>Since: PMD 0.1
Priority: Medium (3)
Avoid the use temporary objects when converting primitives to Strings. Use the static conversion methodson the wrapper classes instead.
This rule is defined by the following XPath expression:
//MethodCall[@MethodName='toString'][ConstructorCall[position()=1][pmd-java:typeIs('java.lang.Integer')orpmd-java:typeIs('java.lang.Long')orpmd-java:typeIs('java.lang.Float')orpmd-java:typeIs('java.lang.Byte')orpmd-java:typeIs('java.lang.Double')orpmd-java:typeIs('java.lang.Short')]]Example(s):
publicStringconvert(intx){Stringfoo=newInteger(x).toString();// this wastes an objectreturnInteger.toString(x);// preferred approach}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UnnecessaryConversionTemporary"/>Since: PMD 3.5
Priority: Medium (3)
After checking an object reference for null, you should invoke equals() on that object rather than passingit to another object’s equals() method.
This rule is defined by the following XPath expression:
//InfixExpression[@Operator='&&']/MethodCall[pmd-java:matchesSig("java.lang.Object#equals(java.lang.Object)")][not(StringLiteral)][not(VariableAccess[@CompileTimeConstant=true()])][ArgumentList/VariableAccess/@Name=..//InfixExpression[@Operator='!='][NullLiteral]/VariableAccess/@Name]Example(s):
publicclassTest{publicStringmethod1(){return"ok";}publicStringmethod2(){returnnull;}publicvoidmethod(Stringa){Stringb;// I don't know it method1() can be "null"// but I know "a" is not null..// I'd better write a.equals(method1())if(a!=null&&method1().equals(a)){// will trigger the rule//whatever}if(method1().equals(a)&&a!=null){// won't trigger the rule//whatever}if(a!=null&&method1().equals(b)){// won't trigger the rule//whatever}if(a!=null&&"LITERAL".equals(a)){// won't trigger the rule//whatever}if(a!=null&&!a.equals("go")){// won't trigger the rulea=method2();if(method1().equals(a)){//whatever}}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UnusedNullCheckInEquals"/>Since: PMD 3.2
Priority: Medium (3)
To make sure the full stacktrace is printed out, use the logging statement with two arguments: a String and a Throwable.
This rule only applies toApache Commons Logging.
This rule is defined by the following XPath expression:
//CatchClause/Block//MethodCall[pmd-java:matchesSig('org.apache.commons.logging.Log#_(java.lang.Object)')][ArgumentList[not(MethodCall)]//VariableAccess/@Name=ancestor::CatchClause/CatchParameter/@Name]Example(s):
publicclassMain{privatestaticfinalLog_LOG=LogFactory.getLog(Main.class);voidbar(){try{}catch(Exceptione){_LOG.error(e);//Wrong!}catch(OtherExceptionoe){_LOG.error(oe.getMessage(),oe);//Correct}}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UseCorrectExceptionLogging"/>Since: PMD 4.1
Priority: Medium (3)
Using ‘==’ or ‘!=’ to compare strings is only reliable if the interned string (String#intern())is used on both sides.
Use theequals() method instead.
This rule is defined by the following XPath expression:
//InfixExpression[@Operator=('==','!=')][count(*[pmd-java:typeIsExactly('java.lang.String')])=2]Example(s):
publicbooleantest(Strings){if(s=="one")returntrue;// unreliableif("two".equals(s))returntrue;// betterreturnfalse;}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UseEqualsToCompareStrings"/>Since: PMD 3.5
Priority: Medium (3)
An operation on an immutable object will not change the object itself since the result of the operation is a new object.Therefore, ignoring the result of such an operation is likely a mistake. The operation can probably be removed.
This rule recognizes the typesString,BigDecimal,BigInteger or any type fromjava.time.* as immutable.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.errorprone.UselessOperationOnImmutableRule
Example(s):
importjava.math.*;classTest{voidmethod1(){BigDecimalbd=newBigDecimal(10);bd.add(newBigDecimal(5));// this will trigger the rule}voidmethod2(){BigDecimalbd=newBigDecimal(10);bd=bd.add(newBigDecimal(5));// this won't trigger the rule}}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UselessOperationOnImmutable"/>Since: PMD 2.0
Priority: Medium (3)
When doingString::toLowerCase()/toUpperCase() conversions, use an explicit locale argument to specify the casetransformation rules.
UsingString::toLowerCase() without arguments implicitly usesLocale::getDefault().The problem is that the default locale depends on the current JVM setup (and usually on the system in whichit is running). Using the system default may be exactly what you want (e.g. if you are manipulating stringsyou got through standard input), but it may as well not be the case (e.g. if you are getting the string overthe network or a file, and the encoding is well-defined and independent of the environment). In the latter case,using the default locale makes the case transformation brittle, as it may yield unexpected results on a machinewhose locale has other case translation rules. For example, in Turkish, the uppercase form ofi isİ (U+0130,not ASCII) and notI (U+0049) as in English.
The rule is intended toforce developers to think about locales when dealing with strings. By taking aconscious decision about the choice of locale at the time of writing, you reduce the risk of surprisingbehaviour down the line, and communicate your intent to future readers.
This rule is defined by the following XPath expression:
//MethodCall[pmd-java:matchesSig("java.lang.String#toLowerCase()")orpmd-java:matchesSig("java.lang.String#toUpperCase()")][not(MethodCall[@MethodName="toHexString"])]Example(s):
// violation - implicitly system-dependent conversionif(x.toLowerCase().equals("list")){}// The above will not match "LIST" on a system with a Turkish locale.// It could be replaced withif(x.toLowerCase(Locale.US).equals("list")){}// or simplyif(x.equalsIgnoreCase("list")){}// ok - system independent conversionStringz=a.toLowerCase(Locale.ROOT);// ok - explicit system-dependent conversionStringz2=a.toLowerCase(Locale.getDefault());Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UseLocaleWithCaseConversions"/>Since: PMD 3.7
Priority: Medium (3)
In J2EE, the getClassLoader() method might not work as expected. UseThread.currentThread().getContextClassLoader() instead.
This rule is defined by the following XPath expression:
//MethodCall[pmd-java:matchesSig("java.lang.Class#getClassLoader()")]Example(s):
publicclassFoo{ClassLoadercl=Bar.class.getClassLoader();}Use this rule by referencing it:
<ruleref="category/java/errorprone.xml/UseProperClassLoader"/>