diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 165e1737ea..25bbe28f41 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2392,7 +2392,7 @@ void RSIGNEDSHIFT() #void: void Annotation(): {} { - "@" jjtThis.name=VoidName() [ AnnotationMemberList() ] + "@" ClassName() [ AnnotationMemberList() ] } void AnnotationMemberList(): @@ -2556,6 +2556,13 @@ String VoidName() #void: {return s.toString();} } +// Produces a ClassOrInterfaceType, possibly with an ambiguous LHS +void ClassName() #void: +{} +{ + AmbiguousName() { forceTypeContext(); } +} + // This is used to get JJTree to generate a node. // Variable references are always ambiguous // when they're parsed, so they're not created diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java index 94719f98dc..c9d5734939 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java @@ -9,45 +9,46 @@ import java.util.Iterator; import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.lang.ast.NodeStream; -import net.sourceforge.pmd.util.StringUtil; +import net.sourceforge.pmd.lang.java.symbols.JClassSymbol; /** * Represents an annotation. * * 
  *
- * Annotation ::= "@" Name {@link ASTAnnotationMemberList AnnotationMemberList}?
+ * Annotation ::= "@" {@link ASTClassOrInterfaceType ClassName} {@link ASTAnnotationMemberList AnnotationMemberList}?
  *
  *
*/ public final class ASTAnnotation extends AbstractJavaTypeNode implements TypeNode, ASTMemberValue, Iterable { - String name; - ASTAnnotation(int id) { super(id); } /** - * Returns the name of the annotation as it is used, - * eg {@code java.lang.Override} or {@code Override}. + * Returns the node that represents the name of the annotation. */ - public String getAnnotationName() { - return name; + public ASTClassOrInterfaceType getTypeNode() { + return (ASTClassOrInterfaceType) getChild(0); } - @Override - @Deprecated - public String getImage() { - return name; + /** + * Returns the symbol of the annotation type. + */ + public JClassSymbol getSymbol() { + // This cast would fail if you use a type parameter as an + // annotation name + return (JClassSymbol) getTypeNode().getReferencedSym(); } + /** * Returns the simple name of the annotation. */ public String getSimpleName() { - return StringUtil.substringAfterLast(getImage(), '.'); + return getTypeNode().getSimpleName(); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaDesignerBindings.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaDesignerBindings.java index d44fbd225d..9e96b9f21c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaDesignerBindings.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaDesignerBindings.java @@ -9,6 +9,7 @@ import java.util.Collections; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; @@ -31,7 +32,7 @@ public final class JavaDesignerBindings extends DefaultDesignerBindings { @Override public Attribute getMainAttribute(Node node) { if (node instanceof JavaNode) { - Attribute attr = ((JavaNode) node).acceptVisitor(MainAttrVisitor.INSTANCE, null); + Attribute attr = node.acceptVisitor(MainAttrVisitor.INSTANCE, null); if (attr != null) { return attr; } @@ -83,6 +84,11 @@ public final class JavaDesignerBindings extends DefaultDesignerBindings { return new Attribute(node, "SimpleName", node.getSimpleName()); } + @Override + public Attribute visit(ASTAnnotation node, Void data) { + return new Attribute(node, "SimpleName", node.getSimpleName()); + } + @Override public Attribute visit(ASTMethodDeclaration node, Void data) { return new Attribute(node, "Name", node.getName()); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/AnnotationSuppressionUtil.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/AnnotationSuppressionUtil.java index 2fda06d48e..ddd4647f35 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/AnnotationSuppressionUtil.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/AnnotationSuppressionUtil.java @@ -17,11 +17,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter; -import net.sourceforge.pmd.lang.java.ast.ASTLiteral; import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTStringLiteral; import net.sourceforge.pmd.lang.java.ast.Annotatable; -import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; /** * Helper methods to suppress violations based on annotations. @@ -107,9 +106,8 @@ final class AnnotationSuppressionUtil { // @formatter:on private static boolean annotationSuppresses(ASTAnnotation annotation, Rule rule) { - // if (SuppressWarnings.class.equals(getType())) { // typeres is not always on - if (TypeHelper.isA(annotation, SuppressWarnings.class)) { - for (ASTLiteral element : annotation.findDescendantsOfType(ASTLiteral.class)) { + if (annotation.getSymbol().getBinaryName().equals("java.lang.SuppressWarnings")) { + for (ASTStringLiteral element : annotation.findDescendantsOfType(ASTStringLiteral.class)) { if (element.hasImageEqualTo("\"PMD\"") || element.hasImageEqualTo( "\"PMD." + rule.getName() + "\"") // Check for standard annotations values diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java index 3539ed6f84..155e9acac2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java @@ -8,18 +8,20 @@ import static net.sourceforge.pmd.util.CollectionUtil.any; import java.lang.reflect.Array; import java.util.HashMap; -import java.util.List; import java.util.Map; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; import net.sourceforge.pmd.lang.java.ast.TypeNode; +import net.sourceforge.pmd.lang.java.symbols.JClassSymbol; +import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol; import net.sourceforge.pmd.lang.java.symboltable.TypedNameDeclaration; import net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader; import net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader.ClassLoaderWrapper; @@ -91,21 +93,21 @@ public final class TypeHelper { || clazzName.equals(n.getName()); } - private static boolean fallbackIsA(TypeNode n, String clazzName) { - if (n.getImage() != null && !n.getImage().contains(".") && clazzName.contains(".")) { - // simple name detected, check the imports to get the full name and use that for fallback - List imports = n.getRoot().findChildrenOfType(ASTImportDeclaration.class); - for (ASTImportDeclaration importDecl : imports) { - if (n.hasImageEqualTo(importDecl.getImportedSimpleName())) { - // found the import, compare the full names - return clazzName.equals(importDecl.getImportedName()); - } - } - } + private static boolean fallbackIsA(final TypeNode n, String clazzName) { + // Later we won't need a fallback. Symbols already contain subclass information. - // fall back on using the simple name of the class only - if (clazzName.equals(n.getImage()) || clazzName.endsWith("." + n.getImage())) { + if (n instanceof ASTAnyTypeDeclaration && ((ASTAnyTypeDeclaration) n).getBinaryName().equals(clazzName)) { return true; + } else if (n instanceof ASTClassOrInterfaceType || n instanceof ASTAnnotation) { + ASTClassOrInterfaceType classType; + if (n instanceof ASTAnnotation) { + classType = ((ASTAnnotation) n).getTypeNode(); + } else { + classType = (ASTClassOrInterfaceType) n; + } + + JTypeDeclSymbol sym = classType.getReferencedSym(); + return sym instanceof JClassSymbol && ((JClassSymbol) sym).getBinaryName().equals(clazzName); } if (n instanceof ASTClassOrInterfaceDeclaration) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java index 5142b05b96..fae0d56ac5 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java @@ -93,8 +93,7 @@ public class TypeHelperTest extends BaseNonParserTest { Assert.assertNull(annotation.getType()); Assert.assertTrue(TypeHelper.isA(annotation, "foo.Stuff")); Assert.assertFalse(TypeHelper.isA(annotation, "other.Stuff")); - // if the searched class name is not fully qualified, then the search should still be successfull - Assert.assertTrue(TypeHelper.isA(annotation, "Stuff")); + Assert.assertFalse(TypeHelper.isA(annotation, "Stuff")); } private void assertIsA(TypeNode node, Class type) { diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTest.kt index 1a9685cd76..0d13e0943c 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTAnnotationTest.kt @@ -31,18 +31,20 @@ class ASTAnnotationTest : ParserTestSpec({ "@F" should parseAs { child { - it::getAnnotationName shouldBe "F" it::getSimpleName shouldBe "F" + it::getTypeNode shouldBe classType("F") + it::getMemberList shouldBe null } } "@java.lang.Override" should parseAs { child { - it::getAnnotationName shouldBe "java.lang.Override" it::getSimpleName shouldBe "Override" + it::getTypeNode shouldBe qualClassType("java.lang.Override", disambiguated = false) + it::getMemberList shouldBe null } } @@ -56,9 +58,10 @@ class ASTAnnotationTest : ParserTestSpec({ "@F(\"ohio\")" should parseAs { child { - it::getAnnotationName shouldBe "F" it::getSimpleName shouldBe "F" + it::getTypeNode shouldBe classType("F") + it::getMemberList shouldBe child { shorthandMemberValue { stringLit("\"ohio\"") @@ -69,9 +72,10 @@ class ASTAnnotationTest : ParserTestSpec({ "@org.F({java.lang.Math.PI})" should parseAs { child { - it::getAnnotationName shouldBe "org.F" it::getSimpleName shouldBe "F" + it::getTypeNode shouldBe qualClassType("org.F", disambiguated = false) + it::getMemberList shouldBe child { shorthandMemberValue { child { @@ -87,9 +91,9 @@ class ASTAnnotationTest : ParserTestSpec({ "@org.F({@Aha, @Oh})" should parseAs { child { - it::getAnnotationName shouldBe "org.F" it::getSimpleName shouldBe "F" + it::getTypeNode shouldBe qualClassType("org.F", disambiguated = false) it::getMemberList shouldBe child { shorthandMemberValue { @@ -103,9 +107,9 @@ class ASTAnnotationTest : ParserTestSpec({ } "@org.F(@Oh)" should parseAs { child { - it::getAnnotationName shouldBe "org.F" it::getSimpleName shouldBe "F" + it::getTypeNode shouldBe qualClassType("org.F", disambiguated = false) it::getMemberList shouldBe child { shorthandMemberValue { @@ -124,9 +128,9 @@ class ASTAnnotationTest : ParserTestSpec({ "@F(a=\"ohio\")" should parseAs { child { - it::getAnnotationName shouldBe "F" it::getSimpleName shouldBe "F" + it::getTypeNode shouldBe classType("F") it::getMemberList shouldBe child { memberValuePair("a") { @@ -138,9 +142,9 @@ class ASTAnnotationTest : ParserTestSpec({ "@org.F(a={java.lang.Math.PI}, b=2)" should parseAs { child { - it::getAnnotationName shouldBe "org.F" it::getSimpleName shouldBe "F" + it::getTypeNode shouldBe qualClassType("org.F", disambiguated = false) it::getMemberList shouldBe child { memberValuePair("a") { @@ -167,6 +171,8 @@ class ASTAnnotationTest : ParserTestSpec({ child { + it::getTypeNode shouldBe classType("TestAnnotation") + it::getMemberList shouldBe child { shorthandMemberValue { @@ -174,6 +180,8 @@ class ASTAnnotationTest : ParserTestSpec({ child { annotation { + it::getTypeNode shouldBe classType("SuppressWarnings") + it::getMemberList shouldBe child { shorthandMemberValue { child {} @@ -181,6 +189,9 @@ class ASTAnnotationTest : ParserTestSpec({ } } annotation { + + it::getTypeNode shouldBe classType("SuppressWarnings") + it::getMemberList shouldBe child { memberValuePair("value") { it::isShorthand shouldBe false @@ -191,6 +202,9 @@ class ASTAnnotationTest : ParserTestSpec({ } } annotation { + + it::getTypeNode shouldBe classType("SuppressWarnings") + it::getMemberList shouldBe child { shorthandMemberValue { child { diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt index c15d0803bf..c2263b891d 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt @@ -7,17 +7,13 @@ import io.kotlintest.matchers.collections.shouldBeEmpty import io.kotlintest.matchers.types.shouldBeInstanceOf import io.kotlintest.shouldNotBe import net.sourceforge.pmd.internal.util.IteratorUtil -import net.sourceforge.pmd.lang.ast.GenericToken import net.sourceforge.pmd.lang.ast.Node import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken import net.sourceforge.pmd.lang.ast.test.NodeSpec import net.sourceforge.pmd.lang.ast.test.ValuedNodeSpec import net.sourceforge.pmd.lang.ast.test.shouldBe -import net.sourceforge.pmd.lang.ast.test.shouldMatch import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType.* -import java.util.* -import kotlin.reflect.KCallable fun > C?.shouldContainAtMostOneOf(vararg expected: T) { this shouldNotBe null @@ -69,9 +65,9 @@ fun TreeNodeWrapper.annotation(spec: ValuedNodeSpec.annotation(name: String, spec: NodeSpec = EmptyAssertions) = +fun TreeNodeWrapper.annotation(simpleName: String, spec: NodeSpec = EmptyAssertions) = child(ignoreChildren = spec == EmptyAssertions) { - it::getAnnotationName shouldBe name + it::getSimpleName shouldBe simpleName spec() } @@ -375,10 +371,16 @@ fun TreeNodeWrapper.classType(simpleName: String, contents: NodeSpec.qualClassType(canoName: String, contents: NodeSpec = EmptyAssertions) = +fun TreeNodeWrapper.qualClassType(canoName: String, disambiguated: Boolean = true, contents: NodeSpec = EmptyAssertions) = child(ignoreChildren = contents == EmptyAssertions) { - it::getImage shouldBe canoName - it::getSimpleName shouldBe canoName.substringAfterLast('.') + val simpleName = canoName.substringAfterLast('.') + if (disambiguated) { + it::getImage shouldBe canoName + it::getSimpleName shouldBe simpleName + } else { + it::getImage shouldBe simpleName + it::getTypeImage shouldBe canoName + } contents() } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/table/internal/TypeParamScopingTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/table/internal/TypeParamScopingTest.kt index d0df3f1954..a3cda4a767 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/table/internal/TypeParamScopingTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/table/internal/TypeParamScopingTest.kt @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.java.symbols.table.internal +import io.kotlintest.matchers.types.shouldBeSameInstanceAs import io.kotlintest.shouldBe import net.sourceforge.pmd.lang.ast.test.shouldBe import net.sourceforge.pmd.lang.ast.test.shouldBeA @@ -190,6 +191,7 @@ class TypeParamScopingTest : ParserTestSpec({ val annot = acu.descendants(ASTAnnotation::class.java).first()!! annot.symbolTable.shouldResolveTypeTo("T", t.symbol) //not t2 + annot.symbol.shouldBeSameInstanceAs(t.symbol) } doTest("Local class shadows type param") {