Implemented patch 1577540 from Jason Bennet

Patch is for Feature Request ] 1434274 - UseCollectionIsEmpty
Thanks Jason!

Some small tweaks: Moved some null checks around for performance, added JUnit test.


git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4680 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Allan Caplan
2006-10-16 14:40:51 +00:00
parent d366791ce8
commit 2378d27100
6 changed files with 188 additions and 2 deletions

View File

@ -2,6 +2,7 @@
New rules:
Basic ruleset: BigIntegerInstantiation(1.4 and 1.5)
Codesize ruleset: NPathComplexity
Design ruleset: UseCollectionIsEmpty
Fixed bug 1570915 - AvoidRethrowingException no longer reports a false positive for certain nested exceptions.
Fixed bug 1571324 - UselessStringValueOf no longer reports a false positive for additive expressions.
Fixed bug 1573795 - PreserveStackTrace doesn't throw CastClassException on exception with 0 args

View File

@ -0,0 +1,100 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package test.net.sourceforge.pmd.rules.design;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Rule;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
/**
* Adding this test to validate current working code doesn't break I've been
* trying to locate the article referenced. The below code stresses the NPath
* rule, and according to its current style, runs 2 tests, one pass and one
* fail.
*
* @author Allan Caplan
*
*/
public class UseCollectionIsEmptyTest extends SimpleAggregatorTst{
private Rule rule;
public void setUp() {
rule = findRule("design", "UseCollectionIsEmpty");
}
public void testAll() {
runTests(new TestDescriptor[]{
new TestDescriptor(TEST1, "fail, == 0", 1, rule),
new TestDescriptor(TEST2, "ok, isEmpty", 0, rule),
new TestDescriptor(TEST3, "fail, != 0", 1, rule),
new TestDescriptor(TEST4, "ok, !isEmpty", 0, rule),
new TestDescriptor(TEST5, "fail, != 0", 1, rule),
new TestDescriptor(TEST6, "ok, !isEmpty", 0, rule),
});
}
private static final String TEST1 =
"public class Foo {" + PMD.EOL +
" public static boolean bar(List lst) {" + PMD.EOL +
" if(lst.size() == 0){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST2 =
"public class Foo {" + PMD.EOL +
" public static boolean bar(List lst) {" + PMD.EOL +
" if(lst.isEmpty()){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST3 =
"public class Foo {" + PMD.EOL +
" public static boolean bar(List lst) {" + PMD.EOL +
" if(lst.size() != 0){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST4 =
"public class Foo {" + PMD.EOL +
" public static boolean bar(List lst) {" + PMD.EOL +
" if(!lst.isEmpty()){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST5 =
"public class Foo {" + PMD.EOL +
" public static boolean bar(List lst, boolean b) {" + PMD.EOL +
" if(lst.size() == 0 && b){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST6 =
"public class Foo {" + PMD.EOL +
" public static boolean bar(List lst, boolean b) {" + PMD.EOL +
" if(lst.isEmpty() && b){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -1271,7 +1271,8 @@ public class Foo {
<rule name="PreserveStackTrace"
message="Caught exception is rethrown, original stack trace may be lost"
class="net.sourceforge.pmd.rules.design.PreserveStackTrace">
class="net.sourceforge.pmd.rules.design.PreserveStackTrace"
externalInfoUrl="http://pmd.sourceforge.net/rules/design.html#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 make it difficult to
@ -1300,5 +1301,36 @@ public class Foo {
]]>
</example>
</rule>
<rule name="UseCollectionIsEmpty"
message="Substitute calls to size() == 0 (or size() != 0) with calls to isEmpty()"
class="net.sourceforge.pmd.rules.design.UseCollectionIsEmpty"
externalInfoUrl="http://pmd.sourceforge.net/rules/design.html#UseCollectionIsEmpty">
<description>
The isEmpty() method on java.util.Collection is provided to see if a collection has any elements.
Comparing the value of size() to 0 merely duplicates existing behavior.
</description>
<properties></properties>
<priority>3</priority>
<example>
<![CDATA[
public class Foo {
void good() {
List foo = getList();
if (foo.isEmpty()) {
// blah
}
}
void bad() {
List foo = getList();
if (foo.size() == 0) {
// blah
}
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -8,4 +8,5 @@ This ruleset contains links to rules that are new in PMD v3.9
<rule ref="rulesets/basic.xml/BigIntegerInstantiation_1.5"/>
<rule ref="rulesets/codesize.xml/NPathComplexity"/>
<rule ref="rulesets/design.xml/UseCollectionIsEmpty"/>
</ruleset>

View File

@ -0,0 +1,52 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.rules.design;
import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.ast.ASTEqualityExpression;
import net.sourceforge.pmd.ast.ASTLiteral;
import net.sourceforge.pmd.ast.ASTName;
import net.sourceforge.pmd.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.ast.SimpleJavaNode;
/**
* Detect structures like "foo.size() == 0" and suggest replacing them with
* foo.isEmpty(). Will also find != 0 (replacable with !isEmpty()).
*
* @author Jason Bennett
*/
public class UseCollectionIsEmpty extends AbstractRule {
public Object visit(ASTEqualityExpression node, Object data) {
SimpleJavaNode leftSide = (SimpleJavaNode) node.jjtGetChild(0);
SimpleJavaNode rightSide = (SimpleJavaNode) node.jjtGetChild(1);
ASTPrimaryPrefix leftPrefix = (ASTPrimaryPrefix) leftSide.getFirstChildOfType(ASTPrimaryPrefix.class);
ASTPrimarySuffix leftSuffix = (ASTPrimarySuffix) leftSide.getFirstChildOfType(ASTPrimarySuffix.class);
ASTName leftName = null;
if (leftPrefix != null) {
leftName = (ASTName) leftPrefix.getFirstChildOfType(ASTName.class);
}
if (leftName == null || leftSuffix == null) {
return data;
}
ASTPrimaryPrefix rightPrefix = (ASTPrimaryPrefix) rightSide.getFirstChildOfType(ASTPrimaryPrefix.class);
ASTLiteral rightLiteral = null;
if (rightPrefix != null) {
rightLiteral = (ASTLiteral) rightPrefix.getFirstChildOfType(ASTLiteral.class);
}
if (rightLiteral != null && leftSuffix.isArguments() && leftSuffix.getArgumentCount() == 0
&& leftName.getImage().endsWith(".size") && rightLiteral.hasImageEqualTo("0")) {
addViolation(data, node);
}
return data;
}
}

View File

@ -55,7 +55,7 @@
<li>Peter Van de Voorde - Rewrote the 'create rule XML' functionality in the designer utility</li>
<li>Josh Devins - Reported bug with annotation parsing</li>
<li>Alan Berg - Reported bug in Ant task</li>
<li>Jason Bennett - Fix for UnnecessaryLocalBeforeReturn, wrote NPathComplexity rule, patches to improve CyclomaticComplexity rule</li>
<li>Jason Bennett - Fix for UnnecessaryLocalBeforeReturn, wrote NPathComplexity rule, patches to improve CyclomaticComplexity rule, Implemented UseCollectionIsEmpty</li>
<li>Brent Fisher - SummaryHTML report improvements</li>
<li>George Thomas - Wrote AvoidRethrowingException rule</li>
<li>Robert Simmons - Reported bug in optimizations package along with suggestions for fix</li>