hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Bug fixes in UseCollectionIsEmpty: detect > 0, size() in expression is not valid ( .size() % mod == 0), NPE on jdk 6 source code

Browse Source 
git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4752 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Xavier Le Vourch
2006-10-26 23:20:16 +00:00
parent 2e3a00b084
commit 3e7b44fb80
2 changed files with 50 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
pmd/regress/test/net/sourceforge/pmd/rules/design/UseCollectionIsEmptyTest.java
View File

@ -34,6 +34,9 @@ public class UseCollectionIsEmptyTest extends SimpleAggregatorTst{
new TestDescriptor(TEST5, "fail, != 0", 1, rule),
new TestDescriptor(TEST6, "ok, !isEmpty", 0, rule),
new TestDescriptor(TEST7, "fail, 0 ==", 1, rule),
new TestDescriptor(TEST8, "fail, > 0", 1, rule),
new TestDescriptor(TEST9, "ok, in expression", 0, rule),
new TestDescriptor(TEST10, "ok, in expression", 0, rule),
});
}
@ -106,5 +109,39 @@ public class UseCollectionIsEmptyTest extends SimpleAggregatorTst{
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";}
"}";
private static final String TEST8 =
"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 TEST9 =
"public class Foo {" + PMD.EOL +
" public static int modulo = 2;" + PMD.EOL +
" public static boolean bar(List lst) {" + PMD.EOL +
" if(lst.size() % modulo == 0){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST10 =
"public class Foo {" + PMD.EOL +
" final Map map;" + PMD.EOL +
" public boolean bar(Foo other) {" + PMD.EOL +
" if (this.map.size() != other.map.size()){" + PMD.EOL +
" return true;" + PMD.EOL +
" }" + PMD.EOL +
" return false;" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

18
pmd/src/net/sourceforge/pmd/rules/AbstractInefficientZeroCheck.java
View File

@ -4,6 +4,7 @@ import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.ast.ASTEqualityExpression;
import net.sourceforge.pmd.ast.ASTLiteral;
import net.sourceforge.pmd.ast.ASTPrimitiveType;
import net.sourceforge.pmd.ast.ASTRelationalExpression;
import net.sourceforge.pmd.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.ast.SimpleNode;
import net.sourceforge.pmd.symboltable.NameOccurrence;
@ -39,9 +40,10 @@ public abstract class AbstractInefficientZeroCheck extends AbstractRule {
if (!isTargetMethod(occ)) {
continue;
}
ASTEqualityExpression equality = (ASTEqualityExpression) occ.getLocation().getFirstParentOfType(
ASTEqualityExpression.class);
if (equality != null && isCompareZero(equality)) {
SimpleNode expr = (SimpleNode) occ.getLocation().jjtGetParent().jjtGetParent().jjtGetParent();
if ((expr instanceof ASTEqualityExpression ||
(expr instanceof ASTRelationalExpression && ">".equals(expr.getImage())))
&& isCompareZero(expr)) {
addViolation(data, occ.getLocation());
}
}
@ -54,7 +56,7 @@ public abstract class AbstractInefficientZeroCheck extends AbstractRule {
* @param equality
* @return true if this is comparing to 0 else false
*/
private boolean isCompareZero(ASTEqualityExpression equality) {
private boolean isCompareZero(SimpleNode equality) {
return (checkComparison(equality, 0) || checkComparison(equality, 1));
}
@ -68,8 +70,12 @@ public abstract class AbstractInefficientZeroCheck extends AbstractRule {
* The ordinal in the equality expression to check
* @return true if the value in position i is 0, else false
*/
private boolean checkComparison(ASTEqualityExpression equality, int i) {
SimpleNode target = (SimpleNode) equality.jjtGetChild(i).jjtGetChild(0).jjtGetChild(0);
private boolean checkComparison(SimpleNode equality, int i) {
SimpleNode target = (SimpleNode) equality.jjtGetChild(i).jjtGetChild(0);
if (target.jjtGetNumChildren() == 0) {
return false;
}
target = (SimpleNode) target.jjtGetChild(0);
return (target instanceof ASTLiteral && "0".equals(target.getImage()));
}