phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[java] Fix class scopes for anon inner classes

Browse Source 
- A new ClassScope was being created for each annon inner class method / field,
    which is incorrect.
 - I reworked the AccessorMethodGeneration to compare scopes directly, since
    I was missing anonymous classes completely.
 - Fixes #274
This commit is contained in:
Juan Martín Sotuyo Dodero
2017-02-26 16:48:31 -03:00
committed by Andreas Dangel
parent 9291125bd0
commit 7a1eb4f26e
5 changed files with 80 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceBody.java
View File

@ -16,9 +16,17 @@ public class ASTClassOrInterfaceBody extends AbstractJavaNode {
/**
* Accept the visitor. *
* Accept the visitor.
*/
public Object jjtAccept(JavaParserVisitor visitor, Object data) {
return visitor.visit(this, data);
}
public boolean isAnonymousInnerClass() {
return jjtGetParent() instanceof ASTAllocationExpression;
}
public boolean isEnumChild() {
return jjtGetParent() instanceof ASTEnumConstant;
}
}

10
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/AccessorMethodGenerationRule.java
View File

@ -7,10 +7,7 @@ package net.sourceforge.pmd.lang.java.rule.design;
import java.util.List;
import java.util.Map;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.AbstractJavaAccessNode;
@ -60,13 +57,10 @@ public class AccessorMethodGenerationRule extends AbstractJavaRule {
}
for (final NameOccurrence no : occurrences) {
Node n = no.getLocation();
while (n != null && !(n instanceof ASTClassOrInterfaceDeclaration) && !(n instanceof ASTEnumDeclaration)) {
n = n.jjtGetParent();
}
ClassScope usedAtScope = no.getLocation().getScope().getEnclosingScope(ClassScope.class);
// Are we within the same class that defines the field / method?
if (!n.getImage().equals(classScope.getClassName())) {
if (!classScope.equals(usedAtScope)) {
addViolation(data, no.getLocation());
}
}

20
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ScopeAndDeclarationFinder.java
View File

@ -11,7 +11,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
import net.sourceforge.pmd.lang.java.ast.ASTCatchStatement;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
@ -124,7 +124,7 @@ public class ScopeAndDeclarationFinder extends JavaParserVisitorAdapter {
ClassNameDeclaration classNameDeclaration = new ClassNameDeclaration(node);
s.addDeclaration(classNameDeclaration);
if (node instanceof ASTClassOrInterfaceBodyDeclaration) {
if (node instanceof ASTClassOrInterfaceBody) {
addScope(new ClassScope(classNameDeclaration), node);
} else {
addScope(new ClassScope(node.getImage(), classNameDeclaration), node);
@ -183,14 +183,14 @@ public class ScopeAndDeclarationFinder extends JavaParserVisitorAdapter {
}
@Override
public Object visit(ASTClassOrInterfaceBodyDeclaration node, Object data) {
if (node.isAnonymousInnerClass() || node.isEnumChild()) {
createClassScope(node);
cont(node);
} else {
super.visit(node, data);
}
return data;
public Object visit(ASTClassOrInterfaceBody node, Object data) {
if (node.isAnonymousInnerClass() || node.isEnumChild()) {
createClassScope(node);
cont(node);
} else {
super.visit(node, data);
}
return data;
}
@Override

37
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/ScopeAndDeclarationFinderTest.java
View File

@ -6,14 +6,17 @@ package net.sourceforge.pmd.lang.java.symboltable;
import java.util.List;
import java.util.Set;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.java.JavaLanguageModule;
import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
import org.junit.Assert;
import org.junit.Test;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.java.JavaLanguageModule;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
public class ScopeAndDeclarationFinderTest extends STBBaseTst {
/**
@ -44,4 +47,28 @@ public class ScopeAndDeclarationFinderTest extends STBBaseTst {
Assert.assertEquals(1, scope.getVariableDeclarations().get(decl).size());
}
}
@Test
public void testAnnonInnerClassScoping() {
String source = "public class Foo {" + PMD.EOL
+ " public static final Creator<Foo> CREATOR = new Creator<Foo>() {" + PMD.EOL
+ " @Override public Foo createFromParcel(Parcel source) {" + PMD.EOL
+ " return new Foo();" + PMD.EOL
+ " }" + PMD.EOL
+ " @Override public Foo[] newArray(int size) {" + PMD.EOL
+ " return new Foo[size];" + PMD.EOL
+ " }" + PMD.EOL
+ " };" + PMD.EOL
+ "}" + PMD.EOL;
parseCode(source, LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getVersion("1.6"));
ClassScope cs = (ClassScope) acu.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getScope();
Assert.assertEquals(1, cs.getClassDeclarations().size()); // There should be 1 anonymous class
List<ASTMethodDeclarator> methods = acu.findDescendantsOfType(ASTMethodDeclarator.class);
Assert.assertEquals(2, methods.size());
ClassScope scope1 = methods.get(0).getScope().getEnclosingScope(ClassScope.class);
ClassScope scope2 = methods.get(1).getScope().getEnclosingScope(ClassScope.class);
Assert.assertSame(scope1, scope2);
}
}

27
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/AccessorMethodGeneration.xml
View File

@ -163,6 +163,33 @@ public class Foo {
/* package */ void outerPackage() {
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
#274 - Method inside static inner class incorrectly reported as generating accessor methods
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo implements Parcelable {
public static final Creator<Foo> CREATOR = new Creator<Foo>() {
@Override
public Foo createFromParcel(Parcel source) {
return new Foo(source.readString(),
getBooleanForInt(source.readInt()),
source.readLong());
}
@Override
public Foo[] newArray(int size) {
return new Foo[size];
}
private boolean getBooleanForInt(int value) {
return value == 1;
}
};
}
]]></code>
</test-code>