diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 9cb3d3263b..3a0ae22294 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -36,6 +36,7 @@ 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. +Fixed false positives in MethodArgumentCouldBeFinal. 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 diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/optimization/xml/MethodArgumentCouldBeFinal.xml b/pmd/regress/test/net/sourceforge/pmd/rules/optimization/xml/MethodArgumentCouldBeFinal.xml index 380d97627f..6dc4ac753b 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/optimization/xml/MethodArgumentCouldBeFinal.xml +++ b/pmd/regress/test/net/sourceforge/pmd/rules/optimization/xml/MethodArgumentCouldBeFinal.xml @@ -137,6 +137,19 @@ public class Foo { public void bar(int a) { x[--a] = 1; } +} + ]]> + + + + 0 + diff --git a/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java b/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java index 14c6bcfdea..ade27ee221 100644 --- a/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java +++ b/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java @@ -124,20 +124,35 @@ public class NameOccurrence { } public boolean isSelfAssignment() { - if (location.jjtGetParent().jjtGetParent().jjtGetParent() instanceof ASTPreDecrementExpression || location.jjtGetParent().jjtGetParent().jjtGetParent() instanceof ASTPreIncrementExpression || location.jjtGetParent().jjtGetParent().jjtGetParent() instanceof ASTPostfixExpression) { - return true; - } - - if (location.jjtGetParent().jjtGetParent().jjtGetParent() instanceof ASTStatementExpression) { - ASTStatementExpression exp = (ASTStatementExpression) location.jjtGetParent().jjtGetParent().jjtGetParent(); - if (exp.jjtGetNumChildren() >= 2 && exp.jjtGetChild(1) instanceof ASTAssignmentOperator) { - ASTAssignmentOperator op = (ASTAssignmentOperator) exp.jjtGetChild(1); - if (op.isCompound()) { - return true; + Node l = location; + while (true) { + Node p = l.jjtGetParent(); + Node gp = p.jjtGetParent(); + Node node = gp.jjtGetParent(); + if (node instanceof ASTPreDecrementExpression || node instanceof ASTPreIncrementExpression || node instanceof ASTPostfixExpression) { + return true; + } + + if (node instanceof ASTStatementExpression) { + ASTStatementExpression exp = (ASTStatementExpression) node; + if (exp.jjtGetNumChildren() >= 2 && exp.jjtGetChild(1) instanceof ASTAssignmentOperator) { + ASTAssignmentOperator op = (ASTAssignmentOperator) exp.jjtGetChild(1); + if (op.isCompound()) { + return true; + } } } + + // deal with extra parenthesis: "(i)++" + if (p instanceof ASTPrimaryPrefix && p.jjtGetNumChildren() == 1 && + gp instanceof ASTPrimaryExpression && gp.jjtGetNumChildren() == 1&& + node instanceof ASTExpression && node.jjtGetNumChildren() == 1) { + l = node; + continue; + } + + return false; } - return false; } public boolean isThisOrSuper() {