diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/AvoidReassigningParameters.xml b/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/AvoidReassigningParameters.xml index c5208faf75..f434ada832 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/AvoidReassigningParameters.xml +++ b/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/AvoidReassigningParameters.xml @@ -217,4 +217,28 @@ public class RealClass { + + False+ AvoidReassigningParameters (bug entry:2410201) + 0 + + + + + diff --git a/pmd/regress/test/net/sourceforge/pmd/symboltable/NameOccurrencesTest.java b/pmd/regress/test/net/sourceforge/pmd/symboltable/NameOccurrencesTest.java index 42f07e060d..6bc7d95a81 100644 --- a/pmd/regress/test/net/sourceforge/pmd/symboltable/NameOccurrencesTest.java +++ b/pmd/regress/test/net/sourceforge/pmd/symboltable/NameOccurrencesTest.java @@ -11,6 +11,7 @@ import net.sourceforge.pmd.ast.ASTPrimaryExpression; import net.sourceforge.pmd.symboltable.NameFinder; import net.sourceforge.pmd.symboltable.NameOccurrence; +import org.junit.Ignore; import org.junit.Test; import java.util.List; @@ -42,6 +43,25 @@ public class NameOccurrencesTest extends STBBaseTst { assertEquals(thisNameOccurrence.getNameForWhichThisIsAQualifier(), occs.getNames().get(1)); } + @Test + @Ignore("Not really sure about should be assert or not") + public void testIsOnLeftHandSide() { + parseCode(TEST7); + List nodes = acu.findChildrenOfType(ASTPrimaryExpression.class); + NameFinder occs = new NameFinder((ASTPrimaryExpression) nodes.get(0)); + assertEquals("t", occs.getNames().get(0).getImage()); + assertEquals("x", occs.getNames().get(1).getImage()); + assertEquals("t", occs.getNames().get(2).getImage()); + //FIXME: Is this how Symbol table should behave ? + // Should it detect an other t occurences ? + assertEquals("t", occs.getNames().get(1).getImage()); + NameOccurrence t = occs.getNames().get(0); + NameOccurrence x = occs.getNames().get(1); + NameOccurrence otherTSymbol = occs.getNames().get(2); + assertTrue(t.isOnLeftHandSide()); + + } + @Test public void testSimpleVariableOccurrence() { parseCode(TEST3); @@ -53,6 +73,8 @@ public class NameOccurrencesTest extends STBBaseTst { assertTrue(occs.getNames().get(0).isOnLeftHandSide()); } + + @Test public void testQualifiedOccurrence() { parseCode(TEST4); @@ -131,6 +153,16 @@ public class NameOccurrencesTest extends STBBaseTst { " }" + PMD.EOL + "}"; + public static final String TEST7 = + "public class Foo {" + PMD.EOL + + " private Bar t;" + PMD.EOL + + " void foo() {" + PMD.EOL + + " t.x = 2;" + PMD.EOL + + " t.t.x = 2;" + PMD.EOL + + " }" + PMD.EOL + + "}"; + + public static junit.framework.Test suite() { return new junit.framework.JUnit4TestAdapter(NameOccurrencesTest.class); } diff --git a/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java b/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java index ad154a331e..7a21fe9613 100644 --- a/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java +++ b/pmd/src/net/sourceforge/pmd/symboltable/NameOccurrence.java @@ -72,40 +72,51 @@ public class NameOccurrence { return node instanceof ASTExpression && node.jjtGetNumChildren() == 3; } - + /** + *

A handy method to assert if the name is on the right hand side or the left hand side of + * an expression. One basic example: + * + * obj.getMethod(); // Name "getMethod()" returns false, "obj" returns true + * + *

+ * @return + */ public boolean isOnLeftHandSide() { // I detest this method with every atom of my being - SimpleNode primaryExpression; + // FIXME: I didn't wrote the previous comment, but based on it i added a FIXME ! + SimpleNode parentOfPrimaryExpression; if (location.jjtGetParent() instanceof ASTPrimaryExpression) { - primaryExpression = (SimpleNode) location.jjtGetParent().jjtGetParent(); + parentOfPrimaryExpression = (SimpleNode) location.jjtGetParent().jjtGetParent(); } else if (location.jjtGetParent().jjtGetParent() instanceof ASTPrimaryExpression) { - primaryExpression = (SimpleNode) location.jjtGetParent().jjtGetParent().jjtGetParent(); + parentOfPrimaryExpression = (SimpleNode) location.jjtGetParent().jjtGetParent().jjtGetParent(); } else { throw new RuntimeException("Found a NameOccurrence that didn't have an ASTPrimary Expression as parent or grandparent. Parent = " + location.jjtGetParent() + " and grandparent = " + location.jjtGetParent().jjtGetParent()); } - if (isStandAlonePostfix(primaryExpression)) { + if (isStandAlonePostfix(parentOfPrimaryExpression)) { return true; } - - if (primaryExpression.jjtGetNumChildren() <= 1) { + // + if (parentOfPrimaryExpression.jjtGetNumChildren() <= 1) { return false; } - - if (!(primaryExpression.jjtGetChild(1) instanceof ASTAssignmentOperator)) { + + if (!(parentOfPrimaryExpression.jjtGetChild(1) instanceof ASTAssignmentOperator)) { return false; } + if (isPartOfQualifiedName() /* or is an array type */) { return false; } - if (isCompoundAssignment(primaryExpression)) { + if (isCompoundAssignment(parentOfPrimaryExpression)) { return false; } return true; } + private boolean isCompoundAssignment(SimpleNode primaryExpression) { return ((ASTAssignmentOperator) (primaryExpression.jjtGetChild(1))).isCompound();