Fixed false positives in LocalVariableCouldBeFinal.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4791 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Xavier Le Vourch
2006-11-14 02:22:40 +00:00
parent 2c4bdd28aa
commit 17043cf32f
3 changed files with 48 additions and 17 deletions

View File

@ -33,6 +33,7 @@ Fixed several JDK 1.5 parsing bugs.
Fixed several rules (exceptions on jdk 1.5 and jdk 1.6 source code).
Fixed array handling in AvoidReassigningParameters and UnusedFormalParameter.
Fixed bug in UselessOverridingMethod: false + when adding synchronization.
Fixed false positives in LocalVariableCouldBeFinal.
Rules can now call RuleContext.getSourceType() if they need to make different checks on JDK 1.4 and 1.5 code.
CloseResource rule now checks code without java.sql import.
ArrayIsStoredDirectly rule now checks Constructors

View File

@ -35,6 +35,7 @@ public class LocalVariableCouldBeFinalTest extends SimpleAggregatorTst {
new TestDescriptor(TEST7, "TEST7", 0, rule),
new TestDescriptor(TEST8, "TEST8", 0, rule),
new TestDescriptor(TEST9, "TEST9", 1, rule),
new TestDescriptor(TEST10, "TEST10", 0, rule),
});
}
@ -116,4 +117,16 @@ public class LocalVariableCouldBeFinalTest extends SimpleAggregatorTst {
" }" + PMD.EOL +
"}";
private static final String TEST10 =
"public class Foo {" + PMD.EOL +
" public void test1() {" + PMD.EOL +
" int a = 0;" + PMD.EOL +
" int b = 0;" + PMD.EOL +
" int c = 0;" + PMD.EOL +
" ++(a);" + PMD.EOL +
" --(b);" + PMD.EOL +
" (c)++;" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -5,22 +5,25 @@
*/
package net.sourceforge.pmd.rules.optimization;
import java.util.Iterator;
import java.util.List;
import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.ast.ASTAssignmentOperator;
import net.sourceforge.pmd.ast.ASTExpression;
import net.sourceforge.pmd.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.ast.ASTName;
import net.sourceforge.pmd.ast.ASTPostfixExpression;
import net.sourceforge.pmd.ast.ASTPreDecrementExpression;
import net.sourceforge.pmd.ast.ASTPreIncrementExpression;
import net.sourceforge.pmd.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.ast.Node;
import net.sourceforge.pmd.ast.SimpleNode;
import java.util.Iterator;
import java.util.List;
/**
* Base class with utility methods for optimization rules
*
@ -57,9 +60,7 @@ public class AbstractOptimizationRule extends AbstractRule implements Rule {
List preinc = md.findChildrenOfType(ASTPreIncrementExpression.class);
if (preinc != null && !preinc.isEmpty()) {
for (Iterator it = preinc.iterator(); it.hasNext();) {
ASTPreIncrementExpression ie = (ASTPreIncrementExpression) it.next();
Node prefix = ie.jjtGetChild(0).jjtGetChild(0);
if (prefix.jjtGetNumChildren() > 0 && ((ASTName) prefix.jjtGetChild(0)).hasImageEqualTo(varName)) {
if (isName((Node)it.next(), varName)) {
return true;
}
}
@ -69,9 +70,7 @@ public class AbstractOptimizationRule extends AbstractRule implements Rule {
List predec = md.findChildrenOfType(ASTPreDecrementExpression.class);
if (predec != null && !predec.isEmpty()) {
for (Iterator it = predec.iterator(); it.hasNext();) {
ASTPreDecrementExpression de = (ASTPreDecrementExpression) it.next();
Node prefix = de.jjtGetChild(0).jjtGetChild(0);
if (prefix.jjtGetNumChildren() > 0 && ((ASTName) prefix.jjtGetChild(0)).hasImageEqualTo(varName)) {
if (isName((Node)it.next(), varName)) {
return true;
}
}
@ -81,15 +80,8 @@ public class AbstractOptimizationRule extends AbstractRule implements Rule {
if (pf != null && !pf.isEmpty()) {
for (Iterator it = pf.iterator(); it.hasNext();) {
ASTPostfixExpression pe = (ASTPostfixExpression) it.next();
if ((pe.hasImageEqualTo("++") || pe.hasImageEqualTo("--"))) {
SimpleNode first = (SimpleNode) pe.jjtGetChild(0);
SimpleNode second = (SimpleNode) first.jjtGetChild(0);
if (second.jjtGetNumChildren() == 0 || !(second.jjtGetChild(0) instanceof ASTName)) {
continue;
}
ASTName name = (ASTName) second.jjtGetChild(0);
if (name.hasImageEqualTo(varName)) {
if (isName(pe, varName)) {
return true;
}
}
@ -98,6 +90,31 @@ public class AbstractOptimizationRule extends AbstractRule implements Rule {
return false;
}
private final boolean isName(Node node, String varName) {
Node first = node.jjtGetChild(0);
Node second = first.jjtGetChild(0);
if (second.jjtGetNumChildren() == 0) {
return false;
}
Node third = second.jjtGetChild(0);
if (!(third instanceof ASTName)) {
if (first instanceof ASTPrimaryExpression) {
// deals with extra parenthesis:
// "(varName)++" instead of "varName++" for instance
if (first.jjtGetNumChildren() != 1) {
return false;
}
if (second instanceof ASTPrimaryPrefix && second.jjtGetNumChildren() == 1 &&
third instanceof ASTExpression && third.jjtGetNumChildren() == 1) {
return isName(third, varName);
}
}
return false;
}
ASTName name = (ASTName) third;
return name.hasImageEqualTo(varName);
}
private final boolean variableAssigned(final String varName, final List assignments) {
if (assignments == null || assignments.isEmpty()) {