diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index a6a7f618d3..96f236c32f 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -2,6 +2,7 @@ Fixed bug 1002: False +: FinalFieldCouldBeStatic on inner class Fixed bug 1005: False + for ConstructorCallsOverridableMethod - overloaded methods +Fixed bug 1032: ImmutableField Rule: Private field in inner class gives false positive Fixed bug 1064: Exception running PrematureDeclaration Fixed bug 1068: CPD fails on broken symbolic links Fixed bug 1073: Hard coded violation messages CommentSize diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ImmutableFieldRule.java b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ImmutableFieldRule.java index a79dfb65fd..da3107ec69 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ImmutableFieldRule.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ImmutableFieldRule.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.Set; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; @@ -113,7 +114,8 @@ public class ImmutableFieldRule extends AbstractJavaRule { private List findAllConstructors(ASTClassOrInterfaceDeclaration node) { List cons = new ArrayList(); - node.findDescendantsOfType(ASTConstructorDeclaration.class, cons, false); + node.getFirstChildOfType(ASTClassOrInterfaceBody.class) + .findDescendantsOfType(ASTConstructorDeclaration.class, cons, false); return cons; } } diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java index 02a17da086..0b456d092f 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java @@ -122,7 +122,10 @@ public class SingularFieldRule extends AbstractJavaRule { if (decl == null) { decl = method; continue; - } else if (decl != method) { + } else if (decl != method + // handle inner classes + && decl.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class) + == method.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class)) { violation = false; break; //Optimization } diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java b/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java index 3cd8d1a4c8..b7461dfa2c 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java @@ -68,6 +68,14 @@ public class ClassScope extends AbstractScope { List nameOccurrences = variableNames.get(decl); if (nameOccurrences == null) { // TODO may be a class name + + // search inner classes + for (ClassNameDeclaration innerClass : classNames.keySet()) { + Scope innerClassScope = innerClass.getScope(); + if (innerClassScope.contains(occurrence)) { + innerClassScope.addVariableNameOccurrence(occurrence); + } + } } else { nameOccurrences.add(occurrence); Node n = occurrence.getLocation(); @@ -155,26 +163,38 @@ public class ClassScope extends AbstractScope { } ImageFinderFunction finder = new ImageFinderFunction(images); Applier.apply(finder, variableNames.keySet().iterator()); - return finder.getDecl(); + NameDeclaration result = finder.getDecl(); + + // search inner classes + if (result == null && !classNames.isEmpty()) { + for (ClassNameDeclaration innerClass : classNames.keySet()) { + Applier.apply(finder, innerClass.getScope().getVariableDeclarations().keySet().iterator()); + result = finder.getDecl(); + if (result != null) { + break; + } + } + } + return result; } public String toString() { - String res = "ClassScope (" + className + "): "; + StringBuilder res = new StringBuilder("ClassScope (").append(className).append("): "); if (!classNames.isEmpty()) { - res += "(" + glomNames(classNames.keySet()) + ")"; + res.append("Inner Classes ").append(glomNames(classNames.keySet())).append("; "); } if (!methodNames.isEmpty()) { for (MethodNameDeclaration mnd: methodNames.keySet()) { - res += mnd.toString(); + res.append(mnd.toString()); int usages = methodNames.get(mnd).size(); - res += "(begins at line " + mnd.getNode().getBeginLine() + ", " + usages + " usages)"; - res += ","; + res.append("(begins at line ").append(mnd.getNode().getBeginLine()).append(", ").append(usages).append(" usages)"); + res.append(", "); } } if (!variableNames.isEmpty()) { - res += "(" + glomNames(variableNames.keySet()) + ")"; + res.append("Variables ").append(glomNames(variableNames.keySet())); } - return res; + return res.toString(); } private String clipClassName(String s) { diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/NameFinder.java b/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/NameFinder.java index 93559641f6..2b1634dbb9 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/NameFinder.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/java/symboltable/NameFinder.java @@ -7,6 +7,7 @@ import java.util.ArrayList; import java.util.List; import java.util.StringTokenizer; +import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.lang.java.ast.ASTArguments; import net.sourceforge.pmd.lang.java.ast.ASTMemberSelector; import net.sourceforge.pmd.lang.java.ast.ASTName; @@ -71,7 +72,8 @@ public class NameFinder { public String toString() { StringBuilder result = new StringBuilder(); for (NameOccurrence occ: names) { - result.append(occ.getImage()); + result.append(occ); + result.append(PMD.EOL); } return result.toString(); } diff --git a/pmd/src/test/java/net/sourceforge/pmd/symboltable/AcceptanceTest.java b/pmd/src/test/java/net/sourceforge/pmd/symboltable/AcceptanceTest.java index 16eddd8a7b..d3f7dac314 100644 --- a/pmd/src/test/java/net/sourceforge/pmd/symboltable/AcceptanceTest.java +++ b/pmd/src/test/java/net/sourceforge/pmd/symboltable/AcceptanceTest.java @@ -125,6 +125,16 @@ public class AcceptanceTest extends STBBaseTst { assertEquals(9, usages.get(1).getLocation().getBeginLine()); } + @Test + public void testInnerOuterClass() { + parseCode(TEST_INNER_CLASS); + ASTVariableDeclaratorId vdi = acu.findDescendantsOfType(ASTVariableDeclaratorId.class).get(0); + List usages = vdi.getUsages(); + assertEquals(2, usages.size()); + assertEquals(5, usages.get(0).getLocation().getBeginLine()); + assertEquals(10, usages.get(1).getLocation().getBeginLine()); + } + private static final String TEST_DEMO = "public class Foo {" + PMD.EOL + " void bar(ArrayList buz) { " + PMD.EOL + @@ -175,6 +185,20 @@ public class AcceptanceTest extends STBBaseTst { " }" + PMD.EOL + "}" + PMD.EOL; + public static final String TEST_INNER_CLASS = + "public class Outer {" + PMD.EOL + + " private static class Inner {" + PMD.EOL + + " private int i;" + PMD.EOL + + " private Inner(int i) {" + PMD.EOL + + " this.i = i;" + PMD.EOL + + " }" + PMD.EOL + + " }" + PMD.EOL + + " public int modify(int i) {" + PMD.EOL + + " Inner in = new Inner(i);" + PMD.EOL + + " return in.i;" + PMD.EOL + + " }" + PMD.EOL + + "}" + PMD.EOL; + public static junit.framework.Test suite() { return new junit.framework.JUnit4TestAdapter(AcceptanceTest.class); } diff --git a/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ImmutableField.xml b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ImmutableField.xml index 4524342674..ffc6a2ceef 100644 --- a/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ImmutableField.xml +++ b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ImmutableField.xml @@ -330,6 +330,38 @@ public class pmd_inc { public void inc_test(int val) { this.test += val; } +} + ]]> + + + #946 ImmutableField false + + 0 + void run(final E entity, final Class> m) throws MyException { + this.counter += 1; + } +} +]]> + + + #1032 ImmutableField Rule: Private field in inner class gives false positive + 0 +