I reproduced the issue and trace the problem back to the symbol table (More exactly a method of the NameOccurences class). I also added a test to reproduced the issue here. (Also added a little javadoc).

I'm not really although how to "assert" results in this test, not really sure how the symbol table should be expected to behave here.

No more time right now for this, i'll try see through this week. 

Note: I'll patch trunk when I'll have fix for good this issue.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/branches/pmd/4.2.x@6759 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Romain Pelisse
2008-12-21 15:36:56 +00:00
parent 8c64498325
commit 1b2063cf94
3 changed files with 77 additions and 10 deletions

View File

@ -217,4 +217,28 @@ public class RealClass {
</code>
</test-code>
<test-code regressionTest="false">
<description>False+ AvoidReassigningParameters (bug entry:2410201)</description>
<expected-problems>0</expected-problems>
<code>
<![CDATA[
public class PmdBug {
class Test {
public String field;
public Test t;
}
public void foo(String field) {
Test t = new Test();
t.field = field;
t.t.field = field;
t.field.toCharArray();
}
public static void main(String[] args) {
new PmdBug().foo("Hello world");
}
}
]]>
</code>
</test-code>
</test-data>

View File

@ -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);
}

View File

@ -72,40 +72,51 @@ public class NameOccurrence {
return node instanceof ASTExpression && node.jjtGetNumChildren() == 3;
}
/**
* <p>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:
* <code>
* obj.getMethod(); // Name "getMethod()" returns false, "obj" returns true
* </code>
* </p>
* @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();