<rule class="net.sourceforge.pmd.rules.CloseResource" message="Ensure that resources like this {0} object are closed after use" name="CloseResource">
<description>
Ensure that resources (like Connection, Statement, and ResultSet objects) are always closed after use
</description>
<example><![CDATA[
public class Bar {
public void foo() {
Connection c = pool.getConnection();
try {
// do stuff
} catch (SQLException ex) {
// handle exception
} finally {
// oops, should close the connection using ‘close‘!
// c.close();
}
}
}
]]></example>
<priority>1</priority>
<properties>
<property name="types" value="Connection,Statement,ResultSet"/>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.IdempotentOperations" message="Avoid idempotent operations (like assigning a variable to itself)" name="IdempotentOperations">
<description>
Avoid idempotent operations - they are silly.
</description>
<example><![CDATA[
public class Foo {
public void bar() {
int x = 2;
x = x;
}
}
]]></example>
<priority>1</priority>
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.design.ImmutableField" message="Private field ‘‘{0}‘‘ could be made final; it is only initialized in the declaration or constructor." name="ImmutableField">
<description>
Identifies private fields whose values never change once they are initialized either in the declaration of the field or by
a constructor. This aids in converting existing classes to immutable classes.
</description>
<example><![CDATA[
public class Foo {
private int x; // could be final
public Foo() {
x = 7;
}
public void foo() {
int a = x + 2;
}
}
]]></example>
<priority>1</priority>
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.design.AssignmentToNonFinalStatic" message="Possible unsafe assignment to a non-final static field in a constructor." name="AssignmentToNonFinalStatic">
<description>
Identifies a possible unsafe usage of a static field.
</description>
<example><![CDATA[
public class StaticField {
static int x;
public FinalFields(int y) {
x = y; // unsafe
}
}
]]></example>
<priority>1</priority>
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Use block level rather than method level synchronization" name="AvoidSynchronizedAtMethodLevel">
<description>
Method level synchronization can backfire when new code is added to the method. Block-level
synchronization helps to ensure that only the code that needs synchronization gets it.
</description>
<example><![CDATA[
public class Foo {
// Try to avoid this
synchronized void foo() {
}
// Prefer this:
void bar() {
synchronized(this) {
}
}
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//MethodDeclaration[@Synchronized=‘true‘]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="An instanceof check is being performed on the caught exception. Create a separate catch clause for this exception type." name="AvoidInstanceofChecksInCatchClause">
<description>
Each caught exception type should be handled in its own catch clause.
</description>
<example><![CDATA[
try { // Avoid this
// do something
} catch (Exception ee) {
if (ee instanceof IOException) {
cleanup();
}
}
try { // Prefer this:
// do something
} catch (IOException ee) {
cleanup();
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//CatchStatement/FormalParameter
/following-sibling::Block//InstanceOfExpression/PrimaryExpression/PrimaryPrefix
/Name[
@Image = ./ancestor::Block/preceding-sibling::FormalParameter
/VariableDeclaratorId/@Image
]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="This abstract class does not have any abstract methods" name="AbstractClassWithoutAbstractMethod">
<description>
The abstract class does not contain any abstract methods. An abstract class suggests
an incomplete implementation, which is to be completed by subclasses implementing the
abstract methods. If the class is intended to be used as a base class only (not to be instantiated
direcly) a protected constructor can be provided prevent direct instantiation.
</description>
<example><![CDATA[
public abstract class Foo {
void int method1() { ... }
void int method2() { ... }
// consider using abstract methods or removing
// the abstract modifier and adding protected constructors
}
]]></example>
<priority>3</priority> <!-- Priority 1 to 3. Changed by ATP3RXS -->
<properties>
<property name="xpath">
<value><![CDATA[
//ClassOrInterfaceDeclaration
[@Abstract=‘true‘
and count( .//MethodDeclaration[@Abstract=‘true‘] )=0 ]
[count(ImplementsList)=0]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="No need to check for null before an instanceof" name="SimplifyConditional">
<description>
No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument.
</description>
<example><![CDATA[
class Foo {
void bar(Object x) {
if (x != null && x instanceof Bar) {
// just drop the "x != null" check
}
}
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//Expression
[ConditionalOrExpression
[EqualityExpression[@Image=‘==‘]
//NullLiteral
and
UnaryExpressionNotPlusMinus
[@Image=‘!‘]//InstanceOfExpression[PrimaryExpression
//Name/@Image = ancestor::ConditionalOrExpression/EqualityExpression
//PrimaryPrefix/Name/@Image]]
or
ConditionalAndExpression
[EqualityExpression[@Image=‘!=‘]//NullLiteral
and
InstanceOfExpression
[PrimaryExpression[count(PrimarySuffix[@ArrayDereference=‘true‘])=0]
//Name/@Image = ancestor::ConditionalAndExpression
/EqualityExpression//PrimaryPrefix/Name/@Image]]]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.design.CompareObjectsWithEquals" message="Use equals() to compare object references." name="CompareObjectsWithEquals">
<description>
Use equals() to compare object references; avoid comparing them with ==.
</description>
<example><![CDATA[
class Foo {
boolean bar(String a, String b) {
return a == b;
}
}
]]></example>
<priority>1</priority>
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.design.UnnecessaryLocalBeforeReturn" message="Consider simply returning the value vs storing it in local variable ‘‘{0}‘‘" name="UnnecessaryLocalBeforeReturn">
<description>
Avoid unnecessarily creating local variables
</description>
<example><![CDATA[
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just ‘return doSomething();‘
}
}
]]></example>
<priority>3</priority> <!-- Priority 1 to 3. Changed by ATP3RXS -->
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Singleton is not thread safe" name="NonThreadSafeSingleton">
<description>
Non-thread safe singletons can result in bad state changes. If possible,
get rid of static singletons by directly instantiating the object. Static
singletons are usually not needed as only a single instance exists anyway.
Other possible fixes are to synchronize the entire method or to use an
initialize-on-demand holder class (do not use the double-check idiom).
See Effective Java, item 48.
</description>
<example><![CDATA[
private static Foo foo = null;
//multiple simultaneous callers may see partially initialized objects
public static Foo getFoo() {
if (foo==null)
foo = new Foo();
return foo;
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//MethodDeclaration[$checkNonStaticMethods=‘true‘ or @Static=‘true‘][@Synchronized=‘false‘]
//IfStatement[not (ancestor::SynchronizedStatement)]
[Expression/EqualityExpression
[PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
[PrimaryExpression/PrimaryPrefix/Name[@Image=
//FieldDeclaration[$checkNonStaticFields=‘true‘ or @Static=‘true‘]
/VariableDeclarator/VariableDeclaratorId/@Image]]]
[Statement//StatementExpression[AssignmentOperator]
[PrimaryExpression/PrimaryPrefix/Name/@Image=
ancestor::ClassOrInterfaceBody//FieldDeclaration[$checkNonStaticFields=‘true‘ or @Static=‘true‘]
/VariableDeclarator/VariableDeclaratorId/@Image]]
]]></value>
</property>
<property name="checkNonStaticFields" value="false"/>
<property name="checkNonStaticMethods" value="true"/>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Document empty method" name="UncommentedEmptyMethod">
<description>
Uncommented Empty Method finds instances where a method does not contain
statements, but there is no comment. By explicitly commenting empty methods
it is easier to distinguish between intentional (commented) and unintentional
empty methods.
</description>
<example><![CDATA[
public void doSomething() {
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//MethodDeclaration/Block[count(BlockStatement) = 0 and @containsComment = ‘false‘]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Document empty constructor" name="UncommentedEmptyConstructor">
<description>
Uncommented Empty Constructor finds instances where a constructor does not
contain statements, but there is no comment. By explicitly commenting empty
constructors it is easier to distinguish between intentional (commented)
and unintentional empty constructors.
</description>
<example><![CDATA[
public Foo() {
super();
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//ConstructorDeclaration[count(BlockStatement) = 0 and ($ignoreExplicitConstructorInvocation = ‘true‘ or not(ExplicitConstructorInvocation)) and @containsComment = ‘false‘]
]]></value>
</property>
<property name="ignoreExplicitConstructorInvocation" value="false"/>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.design.PreserveStackTrace" message="Caught exception is rethrown, original stack trace may be lost" name="PreserveStackTrace">
<description>
Throwing a new exception from a catch block without passing the original exception into the
new Exception will cause the true stack trace to be lost, and can cause problems
debugging problems
</description>
<example><![CDATA[
public class Foo {
void good() {
try{
Integer.parseInt("a");
} catch(Exception e){
throw new Exception(e);
}
}
void bad() {
try{
Integer.parseInt("a");
} catch(Exception e){
throw new Exception(e.getMessage());
}
}
}
]]></example>
<priority>2</priority> <!-- Priority 5 to 2. Changed by ATP3RXS -->
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.junit.JUnitAssertionsShouldIncludeMessage" message="JUnit assertions should include a message" name="JUnitAssertionsShouldIncludeMessage">
<description>
JUnit assertions should include a message - i.e., use the three argument version of
assertEquals(), not the two argument version.
</description>
<example><![CDATA[
public class Foo extends TestCase {
public void testSomething() {
assertEquals("foo", "bar");
// Use the form:
// assertEquals("Foo does not equals bar", "foo", "bar");
// instead
}
}
]]></example>
<priority>2</priority> <!-- Priority 1 to 2. Changed by ATP1AXR -->
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.junit.JUnitTestsShouldContainAsserts" message="JUnit tests should include assert() or fail()" name="JUnitTestsShouldIncludeAssert">
<description>
JUnit tests should include at least one assertion. This makes the tests more robust, and
using assert with messages provide the developer a clearer idea of what the test does.
</description>
<example><![CDATA[
public class Foo extends TestCase {
public void testSomething() {
Bar b = findBar();
// This is better than having a NullPointerException
// assertNotNull("bar not found", b);
b.work();
}
}
]]></example>
<priority>2</priority> <!-- Priority 1 to 2. Changed by ATP1AXR -->
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.junit.TestClassWithoutTestCases" message="This class name ends with ‘Test‘ but contains no test cases" name="TestClassWithoutTestCases">
<description>
Test classes end with the suffix Test. Having a non-test class with that name is
not a good practice, since most people will assume it is a test case. Test
classes have test methods named testXXX.
</description>
<example><![CDATA[
//Consider changing the name of the class if it is not a test
//Consider adding test methods if it is a test
public class CarTest {
public static void main(String[] args) {
// do something
}
// code
}
]]></example>
<priority>2</priority> <!-- Priority 1 to 2. Changed by ATP3RXS -->
<properties/>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="assertTrue(true) or similar statements are unnecessary" name="UnnecessaryBooleanAssertion">
<description>
A JUnit test assertion with a boolean literal is unnecessary since it always will eval to the same thing.
Consider using flow control (in case of assertTrue(false) or similar) or simply removing
statements like assertTrue(true) and assertFalse(false). If you just want a test to halt, use the fail method.
</description>
<example><![CDATA[
public class SimpleTest extends TestCase {
public void testX() {
// Why on earth would you write this?
assertTrue(true);
}
}
]]></example>
<priority>3</priority> <!-- Priority 1 to 3. Changed by ATP3RXS -->
<properties>
<property name="xpath">
<value><![CDATA[
//StatementExpression
[
.//Name[@Image=‘assertTrue‘ or @Image=‘assertFalse‘]
and
PrimaryExpression/PrimarySuffix/Arguments/ArgumentList
/Expression/PrimaryExpression/PrimaryPrefix
/Literal/BooleanLiteral
or
(
.//Name[@Image=‘assertTrue‘ or @Image=‘assertFalse‘]
and
PrimaryExpression/PrimarySuffix/Arguments/ArgumentList
/Expression/UnaryExpressionNotPlusMinus[@Image=‘!‘]
/PrimaryExpression/PrimaryPrefix[Literal/BooleanLiteral or Name[count(../../*)=1]])
]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Use assertEquals(x, y) instead of assertTrue(x.equals(y))" name="UseAssertEqualsInsteadOfAssertTrue">
<description>
This rule detects JUnit assertions in object equality. These assertions
should be made by more specific methods, like assertEquals.
</description>
<example><![CDATA[
public class FooTest extends TestCase {
void testCode() {
Object a, b;
assertTrue(a.equals(b)); // bad usage
assertEquals(?a should equals b?, a, b); // good usage
}
}
]]></example>
<priority>7</priority> <!-- Priority 1 to 7. Changed by ATP3RXS -->
<properties>
<property name="xpath">
<value><![CDATA[
//PrimaryExpression[
PrimaryPrefix/Name[@Image = ‘assertTrue‘]
][
PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Name
[ends-with(@Image, ‘.equals‘)]
]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Use assertSame(x, y) instead of assertTrue(x==y), or assertNotSame(x,y) vs assertFalse(x==y)" name="UseAssertSameInsteadOfAssertTrue">
<description>
This rule detects JUnit assertions in object references equality. These assertions
should be made by more specific methods, like assertSame, assertNotSame.
</description>
<example><![CDATA[
public class FooTest extends TestCase {
void testCode() {
Object a, b;
assertTrue(a==b); // bad usage
assertSame(a, b); // good usage
}
}
]]></example>
<priority>7</priority> <!-- Priority 1 to 7. Changed by ATP3RXS -->
<properties>
<property name="xpath">
<value><![CDATA[
//PrimaryExpression [
PrimaryPrefix/Name
[@Image = ‘assertTrue‘ or @Image = ‘assertFalse‘]
]
[PrimarySuffix/Arguments
/ArgumentList/Expression
/EqualityExpression[count(//NullLiteral) = 0]]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Use assertNull(x) instead of assertTrue(x==null), or assertNotNull(x) vs assertFalse(x==null)" name="UseAssertNullInsteadOfAssertTrue">
<description>
This rule detects JUnit assertions in object references equality. These assertions
should be made by more specific methods, like assertNull, assertNotNull.
</description>
<example><![CDATA[
public class FooTest extends TestCase {
void testCode() {
Object a = doSomething();
assertTrue(a==null); // bad usage
assertNull(a); // good usage
assertTrue(a != null); // bad usage
assertNotNull(a); // good usage
}
}
]]></example>
<priority>7</priority> <!-- Priority 1 to 7. Changed by ATP3RXS -->
<properties>
<property name="xpath">
<value><![CDATA[
//PrimaryExpression[
PrimaryPrefix/Name[@Image = ‘assertTrue‘ or @Image = ‘assertFalse‘]
][
PrimarySuffix/Arguments/ArgumentList[
Expression/EqualityExpression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral
]
]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="assertTrue(!expr) can be replaced by assertFalse(expr)" name="SimplifyBooleanAssertion">
<description>
Avoid negation in an assertTrue or assertFalse test.
For example, rephrase:
assertTrue(!expr);
as:
assertFalse(expr);
</description>
<example><![CDATA[
public class SimpleTest extends TestCase {
public void testX() {
assertTrue("not empty", !r.isEmpty()); // replace with assertFalse("not empty", r.isEmpty())
assertFalse(!r.isEmpty()); // replace with assertTrue(r.isEmpty())
}
}
]]></example>
<priority>3</priority> <!-- Priority 1 to 3. Changed by ATP3RXS -->
<properties>
<property name="xpath">
<value><![CDATA[
//StatementExpression
[
.//Name[@Image=‘assertTrue‘ or @Image=‘assertFalse‘]
and
PrimaryExpression/PrimarySuffix/Arguments/ArgumentList
/Expression/UnaryExpressionNotPlusMinus[@Image=‘!‘]
/PrimaryExpression/PrimaryPrefix
]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Use the correct logging statement for logging exceptions" name="UseCorrectExceptionLogging">
<description>
To make sure the full stacktrace is printed out, use the logging statement with 2 arguments: a String and a Throwable.
</description>
<example><![CDATA[
public class Main {
private static final Log _LOG = LogFactory.getLog( Main.class );
void bar() {
try {
} catch( Exception e ) {
_LOG.error( e ); //Wrong!
} catch( OtherException oe ) {
_LOG.error( oe.getMessage(), oe ); //Correct
}
}
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//CatchStatement/Block/BlockStatement/Statement/StatementExpression
/PrimaryExpression[PrimaryPrefix/Name[starts-with(@Image,
concat(//ClassOrInterfaceBodyDeclaration/FieldDeclaration
[Type//ClassOrInterfaceType[@Image=‘Log‘]]
/VariableDeclarator/VariableDeclaratorId/@Image, ‘.‘))]]
[PrimarySuffix/Arguments[@ArgumentCount=‘1‘]]
[PrimarySuffix/Arguments//Name/@Image = ancestor::CatchStatement/FormalParameter/VariableDeclaratorId/@Image] ]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.BeanMembersShouldSerializeRule" message="Found non-transient, non-static member. Please mark as transient or provide accessors." name="BeanMembersShouldSerialize">
<description>
If a class is a bean, or is referenced by a bean, directly or indirectly
it needs to be serializable. Member variables need to be marked as transient,
marked as static, or have accessor methods in the class. Marking variables
as transient is the safest and easiest modification. Accessor methods should
follow the Java naming conventions, i.e.if you have a variable foo, you should
provide getFoo and setFoo methods.
</description>
<example><![CDATA[
private transient int someFoo;//good, it‘s transient
private static int otherFoo;// also OK
private int moreFoo;// OK, has proper accessors, see below
private int badFoo;//bad, should be marked transient
private void setMoreFoo(int moreFoo){
this.moreFoo = moreFoo;
}
private int getMoreFoo(){
return this.moreFoo;
}
]]></example>
<priority>1</priority>
<properties>
<property name="prefix" value=""/>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="System.out.print is used" name="SystemPrintln">
<description>
System.(out|err).print is used, consider using a logger.
</description>
<example><![CDATA[
class Foo{
Logger log = Logger.getLogger(Foo.class.getName());
public void testA () {
System.out.println("Entering test");
// Better use this
log.fine("Entering test");
}
}
]]></example>
<priority>1</priority>
<properties>
<property name="xpath">
<value><![CDATA[
//Name[
starts-with(@Image, ‘System.out.print‘)
or
starts-with(@Image, ‘System.err.print‘)
]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.XPathRule" message="Avoid printStackTrace(); use a logger call instead." name="AvoidPrintStackTrace">
<description>
Avoid printStackTrace(); use a logger call instead.
</description>
<example><![CDATA[
class Foo {
void bar() {
try {
// do something
} catch (Exception e) {
e.printStackTrace();
}
}
}
]]></example>
<priority>1</priority> <!-- Priority 5 to 1. Changed by ATP1AXR -->
<properties>
<property name="xpath">
<value><![CDATA[
//PrimaryExpression
[PrimaryPrefix/Name[contains(@Image,‘printStackTrace‘)]]
[PrimarySuffix[not(boolean(Arguments/ArgumentList/Expression))]]
]]></value>
</property>
</properties>
</rule>
<rule class="net.sourceforge.pmd.rules.sunsecure.MethodReturnsInternalArray" message="Returning ‘‘{0}‘‘ may expose an internal array" name="MethodReturnsInternalArray">
<description>
Exposing internal arrays directly allows the user to modify some code that could be critical.
It is safer to return a copy of the array.
</description>
<example><![CDATA[
public class SecureSystem {
UserData [] ud;
public UserData [] getUserData() {
// Don‘t return directly the internal array, return a copy
return ud;
}
}
]]></example>
<priority>1</priority>
<properties/>
</rule>