Since: PMD1.0
Avoid jumbled loop incrementers - it's usually a mistake, and it's confusing even if it's what's intended.
//ForStatement
[
ForUpdate/StatementExpressionList/StatementExpression/PostfixExpression/PrimaryExpression/PrimaryPrefix/Name/@Image
=
ancestor::ForStatement/ForInit//VariableDeclaratorId/@Image
]
public class JumbledIncrementerRule1 {
public void foo() {
for (int i = 0; i < 10; i++) {
for (int k = 0; k < 20; i++) {
System.out.println("Hello");
}
}
}
}
Since: PMD1.02
Some for loops can be simplified to while loops - this makes them more concise.
//ForStatement
[count(*) > 1]
[not(ForInit)]
[not(ForUpdate)]
[not(Type and Expression and Statement)]
public class Foo {
void bar() {
for (;true;) true; // No Init or Update part, may as well be: while (true)
}
}
Since: PMD0.4
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.basic.OverrideBothEqualsAndHashcodeRule
Example(s):
// this is bad
public class Bar {
public boolean equals(Object o) {
// do some comparison
}
}
// and so is this
public class Baz {
public int hashCode() {
// return some hash value
}
}
// this is OK
public class Foo {
public boolean equals(Object other) {
// do some comparison
}
public int hashCode() {
// return some hash value
}
}
Since: PMD1.04
Partially created objects can be returned by the Double Checked Locking pattern when used in Java. An optimizing JRE may assign a reference to the baz variable before it creates the object the reference is intended to point to. For more details see http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.basic.DoubleCheckedLockingRule
Example(s):
public class Foo {
Object baz;
Object bar() {
if(baz == null) { //baz may be non-null yet not fully created
synchronized(this){
if(baz == null){
baz = new Object();
}
}
}
return baz;
}
}
Since: PMD1.05
Avoid returning from a finally block - this can discard exceptions.
//FinallyStatement//ReturnStatement
public class Bar {
public String foo() {
try {
throw new Exception( "My Exception" );
} catch (Exception e) {
throw e;
} finally {
return "A. O. K."; // Very bad.
}
}
}
Since: PMD1.5
Do not use "if" statements that are always true or always false.
//IfStatement/Expression
[count(PrimaryExpression)=1]
/PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral
public class Foo {
public void close() {
if (true) {
// ...
}
}
}
Since: PMD1.2
Avoid instantiating Boolean objects; you can reference Boolean.TRUE, Boolean.FALSE, or call Boolean.valueOf() instead.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.basic.BooleanInstantiationRule
Example(s):
public class Foo {
Boolean bar = new Boolean("true"); // just do a Boolean bar = Boolean.TRUE;
Boolean buz = Boolean.valueOf(false); // just do a Boolean buz = Boolean.FALSE;
}
Since: PMD3.1
Sometimes two 'if' statements can be consolidated by separating their conditions with a boolean short-circuit operator.
//IfStatement[@Else='false']/Statement
/IfStatement[@Else='false']
|
//IfStatement[@Else='false']/Statement
/Block[count(BlockStatement)=1]/BlockStatement
/Statement/IfStatement[@Else='false']
public class Foo {
void bar() {
if (x) {
if (y) {
// do stuff
}
}
}
}
Since: PMD3.4
if you need to get an array of a class from your Collection, you should pass an array of the desidered class as the parameter of the toArray method. Otherwise you will get a ClassCastException.
//CastExpression[Type/ReferenceType/ClassOrInterfaceType[@Image !=
"Object"]]//PrimaryExpression
[
PrimaryPrefix/Name[ends-with(@Image, '.toArray')]
and
PrimarySuffix/Arguments[count(*) = 0]
and
count(PrimarySuffix) = 1
]
import java.util.ArrayList;
import java.util.Collection;
public class Test {
public static void main(String[] args) {
Collection c=new ArrayList();
Integer obj=new Integer(1);
c.add(obj);
// this would trigger the rule (and throw a ClassCastException
if executed)
Integer[] a=(Integer [])c.toArray();
// this wouldn't trigger the rule
Integer[] b=(Integer [])c.toArray(new Integer[c.size()]);
}
}
Since: PMD3.4
One might assume that "new BigDecimal(.1)" is exactly equal to .1, but it is actually equal to .1000000000000000055511151231257827021181583404541015625. This is so because .1 cannot be represented exactly as a double (or, for that matter, as a binary fraction of any finite length). Thus, the long value that is being passed in to the constructor is not exactly equal to .1, appearances notwithstanding. The (String) constructor, on the other hand, is perfectly predictable: 'new BigDecimal(".1")' is exactly equal to .1, as one would expect. Therefore, it is generally recommended that the (String) constructor be used in preference to this one.
//AllocationExpression[ClassOrInterfaceType[@Image="BigDecimal"]
and
./Arguments/ArgumentList
/Expression/PrimaryExpression/PrimaryPrefix/Literal[(not
(ends-with
(@Image,'"'))) and contains(@Image,".")]]
import java.math.BigDecimal;
public class Test {
public static void main(String[] args) {
// this would trigger the rule
BigDecimal bd=new BigDecimal(1.123);
// this wouldn't trigger the rule
BigDecimal bd=new BigDecimal("1.123");
// this wouldn't trigger the rule
BigDecimal bd=new BigDecimal(12);
}
}
Since: PMD3.5
The null check here is misplaced. if the variable is null you'll get a NullPointerException. Either the check is useless (the variable will never be "null") or it's incorrect.
//Expression
/*[self::ConditionalOrExpression or self::ConditionalAndExpression]
/descendant::PrimaryExpression/PrimaryPrefix
/Name[starts-with(@Image,
concat(ancestor::PrimaryExpression/following-sibling::EqualityExpression
[./PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
/PrimaryExpression/PrimaryPrefix
/Name[count(../../PrimarySuffix)=0]/@Image,"."))
]
public class Foo {
void bar() {
if (a.equals(baz) && a != null) {}
}
}
public class Foo {
void bar() {
if (a.equals(baz) || a == null) {}
}
}
Since: PMD3.6
Avoid using ThreadGroup; although it is intended to be used in a threaded environment it contains methods that are not thread safe.
//AllocationExpression/ClassOrInterfaceType[contains(@Image,'ThreadGroup')] |
//PrimarySuffix[contains(@Image, 'getThreadGroup')]
public class Bar {
void buz() {
ThreadGroup tg = new ThreadGroup("My threadgroup") ;
tg = new ThreadGroup(tg, "my thread group");
tg = Thread.currentThread().getThreadGroup();
tg = System.getSecurityManager().getThreadGroup();
}
}
Since: PMD3.8
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.basic.BrokenNullCheckRule
Example(s):
class Foo {
String bar(String string) {
// should be &&
if (string!=null || !string.equals(""))
return string;
// should be ||
if (string==null && string.equals(""))
return string;
}
}
Since: PMD3.9
Don't create instances of already existing BigInteger (BigInteger.ZERO, BigInteger.ONE) and for 1.5 on, BigInteger.TEN and BigDecimal (BigDecimal.ZERO, BigDecimal.ONE, BigDecimal.TEN)
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.basic.BigIntegerInstantiationRule
Example(s):
public class Test {
public static void main(String[] args) {
BigInteger bi=new BigInteger(1);
BigInteger bi2=new BigInteger("0");
BigInteger bi3=new BigInteger(0.0);
BigInteger bi4;
bi4=new BigInteger(0);
}
}
Since: PMD3.9
Integer literals should not start with zero. Zero means that the rest of literal will be interpreted as an octal value.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.basic.AvoidUsingOctalValuesRule
Example(s):
public class Foo {
int i = 012; // set i with 10 not 12
int j = 010; // set j with 8 not 10
k = i * j; // set k with 80 not 120
}
Since: PMD4.1
An application with hard coded IP may become impossible to deploy in some case. It never hurts to externalize IP adresses.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.basic.AvoidUsingHardCodedIPRule
Example(s):
public class Foo {
String ip = "127.0.0.1"; // This is a really bad idea !
}
This rule has the following properties:
| Name | Default Value | Description |
|---|---|---|
| pattern | ^"[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}"$ | Regular Expression |
Since: PMD4.1
Always check the return of one of the navigation method (next,previous,first,last) of a ResultSet. Indeed, if the value return is 'false', the developer should deal with it !
//Type/ReferenceType/ClassOrInterfaceType[
(@Image = 'ResultSet')
and
(../../../descendant::Name[ends-with(@Image,'executeQuery')])
and
(
(not (contains(
(./ancestor::Block/descendant::WhileStatement/descendant::Name/attribute::Image),
concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.next')
) ) )
and ( not ( contains(
(./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.next')
) ) )
and (not (contains(
(./ancestor::Block/descendant::WhileStatement/descendant::Name/attribute::Image),
concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.previous')
) ) )
and ( not ( contains(
(./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.previous')
) ) )
and ( not ( contains(
(./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.last')
) ) )
and ( not ( contains(
(./ancestor::Block/descendant::IfStatement/descendant::Name/attribute::Image),
concat(../../../VariableDeclarator/VariableDeclaratorId/attribute::Image,'.first')
) ) )
)
]
// This is NOT appropriate !
Statement stat = conn.createStatement();
ResultSet rst = stat.executeQuery("SELECT name FROM person");
rst.next(); // what if it returns a 'false' ?
String firstName = rst.getString(1);
// This is appropriate...
Statement stat = conn.createStatement();
ResultSet rst = stat.executeQuery("SELECT name FROM person");
if (rst.next())
{
String firstName = rst.getString(1);
}
else
{
// here you deal with the error ( at least log it)
}
Since: PMD4.2
Using multiple unary operators may be a bug, and/or is confusing. Check the usage is not a bug, or consider simplifying the expression.
This rule is defined by the following Java class:net.sourceforge.pmd.lang.java.rule.basic.AvoidMultipleUnaryOperatorsRule
Example(s):
// These are typo bugs, or at best needlessly complex and confusing:
int i = - -1;
int j = + - +1;
int z = ~~2;
boolean b = !!true;
boolean c = !!!true;
// These are better:
int i = 1;
int j = -1;
int z = 2;
boolean b = true;
boolean c = false;
// And these just make your brain hurt:
int i = ~-2;
int j = -~7;
Since: PMD5.0
No need to explicitly extend Object.
//ExtendsList/ClassOrInterfaceType[@Image='Object' or @Image='java.lang.Object']
public class Foo extends Object { // This is useless
}
Since: PMD5.0
The skip() method may skip a smaller number of bytes than requested. You need to 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.basic.CheckSkipResultRule
Example(s):
public class Foo {
private FileInputStream _s = new FileInputStream("file");
public void skip(int n) throws IOException {
_s.skip(n); // You are not sure that exactly n bytes are skipped
}
public void skipExactly(int n) throws IOException {
while (n != 0) {
long skipped = _s.skip(n);
if (skipped == 0)
throw new EOFException();
n -= skipped;
}
}
}