diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/AssertionUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/AssertionUtil.java index 12397636eb..c4947441b1 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/AssertionUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/AssertionUtil.java @@ -21,6 +21,28 @@ public final class AssertionUtil { return PACKAGE_PATTERN.matcher(name).matches(); } + /** + * Returns true if the charsequence is a valid java identifier. + * + * @param name Name (non-null) + * + * @throws NullPointerException If the name is null + */ + public static boolean isJavaIdentifier(CharSequence name) { + int len = name.length(); + if (len == 0 || !Character.isJavaIdentifierStart(name.charAt(0))) { + return false; + } + + for (int i = 1; i < len; i++) { + if (!Character.isJavaIdentifierPart(name.charAt(i))) { + return false; + } + } + + return true; + } + public static int requireOver1(String name, final int value) { if (value < 1) { throw mustBe(name, value, ">= 1"); diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 728705fb7a..17f6d13d10 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2393,7 +2393,7 @@ void RSIGNEDSHIFT() #void: void Annotation(): {} { - "@" jjtThis.name=VoidName() [ AnnotationMemberList() ] + "@" ClassName() [ AnnotationMemberList() ] } void AnnotationMemberList(): @@ -2557,6 +2557,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/ASTAmbiguousName.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAmbiguousName.java index 7272ccdd4b..ff5fa4d87c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAmbiguousName.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAmbiguousName.java @@ -173,14 +173,10 @@ public final class ASTAmbiguousName extends AbstractJavaExpr implements ASTRefer } /** - * Delete this name from the children of the parent. The image of - * this name is prepended to the image of the parent. + * Delete this name from the children of the parent. */ - void deleteInParentPrependImage(char delim) { + void deleteInParent() { AbstractJavaNode parent = (AbstractJavaNode) getParent(); - String image = parent.getImage(); - parent.setImage(getName() + delim + image); - parent.removeChildAtIndex(this.getIndexInParent()); } 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 677f395991..498e55b516 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,47 @@ 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. This is reported as an error by the + // disambiguation pass + 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/ast/ASTCatchParameter.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCatchParameter.java index f8d46c21a5..f74529dc9e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCatchParameter.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCatchParameter.java @@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.java.ast; import org.checkerframework.checker.nullness.qual.NonNull; +import net.sourceforge.pmd.lang.ast.NodeStream; + /** * Formal parameter of a {@linkplain ASTCatchClause catch clause} @@ -50,7 +52,7 @@ public final class ASTCatchParameter extends AbstractJavaNode /** Returns the name of this parameter. */ public String getName() { - return getVarId().getVariableName(); + return getVarId().getName(); } @@ -59,7 +61,21 @@ public final class ASTCatchParameter extends AbstractJavaNode * {@link ASTUnionType UnionType}. */ public ASTType getTypeNode() { - return children(ASTType.class).first(); + return (ASTType) getChild(1); + } + + + /** + * Returns a stream of all declared exception types (expanding a union + * type if present). + */ + public NodeStream getAllExceptionTypes() { + ASTType typeNode = getTypeNode(); + if (typeNode instanceof ASTUnionType) { + return typeNode.children(ASTClassOrInterfaceType.class); + } else { + return NodeStream.of((ASTClassOrInterfaceType) typeNode); + } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java index 1dfe4e6b48..ade5b6330d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java @@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.java.ast; import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.annotation.Experimental; +import net.sourceforge.pmd.internal.util.AssertionUtil; import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken; import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol; @@ -31,31 +32,44 @@ import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol; */ // @formatter:on public final class ASTClassOrInterfaceType extends AbstractJavaTypeNode implements ASTReferenceType { + // todo rename to ASTClassType private JTypeDeclSymbol symbol; - ASTClassOrInterfaceType(ASTAmbiguousName lhs, String image) { + private String simpleName; + + // Note that this is only populated during disambiguation, if + // the ambiguous qualifier is resolved to a package name + private boolean isFqcn; + + ASTClassOrInterfaceType(ASTAmbiguousName lhs, String simpleName) { super(JavaParserImplTreeConstants.JJTCLASSORINTERFACETYPE); + assert lhs != null : "Null LHS"; + this.addChild(lhs, 0); - this.setImage(image); + this.simpleName = simpleName; + assertSimpleNameOk(); } ASTClassOrInterfaceType(ASTAmbiguousName simpleName) { super(JavaParserImplTreeConstants.JJTCLASSORINTERFACETYPE); - this.setImage(simpleName.getImage()); - } + this.simpleName = simpleName.getName(); + assertSimpleNameOk(); + } // Just for one usage in Symbol table + @Deprecated public ASTClassOrInterfaceType(String simpleName) { super(JavaParserImplTreeConstants.JJTCLASSORINTERFACETYPE); - this.setImage(simpleName); + this.simpleName = simpleName; } - ASTClassOrInterfaceType(ASTClassOrInterfaceType lhs, String image, JavaccToken firstToken, JavaccToken identifier) { + ASTClassOrInterfaceType(@Nullable ASTClassOrInterfaceType lhs, boolean isFqcn, JavaccToken firstToken, JavaccToken identifier) { super(JavaParserImplTreeConstants.JJTCLASSORINTERFACETYPE); - this.setImage(image); + this.setImage(identifier.getImage()); + this.isFqcn = isFqcn; if (lhs != null) { this.addChild(lhs, 0); } @@ -68,6 +82,35 @@ public final class ASTClassOrInterfaceType extends AbstractJavaTypeNode implemen super(id); } + @Override + protected void setImage(String image) { + this.simpleName = image; + assertSimpleNameOk(); + } + + @Deprecated + @Override + public String getImage() { + return null; + } + + private void assertSimpleNameOk() { + assert this.simpleName != null + && this.simpleName.indexOf('.') < 0 + && AssertionUtil.isJavaIdentifier(this.simpleName) + : "Invalid simple name '" + this.simpleName + "'"; + } + + /** + * Returns true if the type was written with a full package qualification. + * For example, {@code java.lang.Override}. For nested types, only the + * leftmost type is considered fully qualified. Eg in {@code p.Outer.Inner}, + * this method will return true for the type corresponding to {@code p.Outer}, + * but false for the enclosing {@code p.Outer.Inner}. + */ + public boolean isFullyQualified() { + return isFqcn; + } void setSymbol(JTypeDeclSymbol symbol) { this.symbol = symbol; @@ -109,10 +152,11 @@ public final class ASTClassOrInterfaceType extends AbstractJavaTypeNode implemen /** - * Returns the simple name of this type. + * Returns the simple name of this type. Use the {@linkplain #getReferencedSym() symbol} + * to get more information. */ public String getSimpleName() { - return AstImplUtil.getLastSegment(getImage(), '.'); + return simpleName; } /** @@ -125,11 +169,15 @@ public final class ASTClassOrInterfaceType extends AbstractJavaTypeNode implemen /** * For now this returns the name of the type with all the segments, * without annotations or type parameters. + * + * @deprecated This is useless and won't be implemented. We can use the + * symbol, or better the upcoming type API to pretty print the type. */ @Override @Experimental + @Deprecated public String getTypeImage() { - return children(ASTType.class).firstOpt().map(s -> s.getTypeImage() + ".").orElse("") + getImage(); + return children(ASTType.class).firstOpt().map(s -> s.getTypeImage() + ".").orElse("") + getSimpleName(); } /** @@ -139,7 +187,10 @@ public final class ASTClassOrInterfaceType extends AbstractJavaTypeNode implemen * * @return {@code true} if this node referencing a type in the same * compilation unit, {@code false} otherwise. + * + * @deprecated This may be removed once type resolution is afoot */ + @Deprecated public boolean isReferenceToClassSameCompilationUnit() { ASTCompilationUnit root = getFirstParentOfType(ASTCompilationUnit.class); for (ASTClassOrInterfaceDeclaration c : root.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class, true)) { @@ -155,9 +206,7 @@ public final class ASTClassOrInterfaceType extends AbstractJavaTypeNode implemen return false; } - - public boolean isAnonymousClass() { - return getParent().getFirstChildOfType(ASTClassOrInterfaceBody.class) != null; + void setFullyQualified() { + this.isFqcn = true; } - } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java index b36f609b79..953b09060b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTList.java @@ -99,6 +99,10 @@ public abstract class ASTList extends AbstractJavaNode imple return list == null ? NodeStream.empty() : list.toStream(); } + public static int sizeOrZero(@Nullable ASTList list) { + return list == null ? 0 : list.size(); + } + /** * Super type for *nonempty* lists that *only* have nodes of type {@code } * as a child. diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTThrowStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTThrowStatement.java index 0940a98298..150a8e8840 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTThrowStatement.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTThrowStatement.java @@ -32,23 +32,4 @@ public final class ASTThrowStatement extends AbstractStatement implements ASTSwi return (ASTExpression) getFirstChild(); } - /** - * Gets the image of the first ASTClassOrInterfaceType child or - * null if none is found. Note that when the statement is - * something like throw new Exception, this method returns 'Exception' and - * if the throw statement is like throw e: this method returns 'e'. A - * special case of returning null is when the throws is like - * throw this.e or throw this. - * - * This is too specific - * - *

TODO - use symbol table (?)

- * - * @return the image of the first ASTClassOrInterfaceType node found or - * null - */ - public String getFirstClassOrInterfaceTypeImage() { - final ASTClassOrInterfaceType t = getFirstDescendantOfType(ASTClassOrInterfaceType.class); - return t == null ? null : t.getImage(); - } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java index 759dc6ba3d..e385c9a809 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java @@ -4,10 +4,10 @@ package net.sourceforge.pmd.lang.java.ast; -import java.util.List; - import org.checkerframework.checker.nullness.qual.Nullable; +import net.sourceforge.pmd.lang.ast.NodeStream; + /** * Try statement node. @@ -64,8 +64,8 @@ public final class ASTTryStatement extends AbstractStatement { * Returns the catch statement nodes of this try statement. * If there are none, returns an empty list. */ - public List getCatchClauses() { - return findChildrenOfType(ASTCatchClause.class); + public NodeStream getCatchClauses() { + return children(ASTCatchClause.class); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java index dc50fb70a9..9c3d954198 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AstDisambiguationPass.java @@ -28,6 +28,7 @@ import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol; import net.sourceforge.pmd.lang.java.symbols.table.JSymbolTable; import net.sourceforge.pmd.lang.java.symbols.table.internal.JavaResolvers; import net.sourceforge.pmd.lang.java.types.JClassType; +import net.sourceforge.pmd.lang.java.symbols.table.internal.SemanticChecksLogger; /** * This implements name disambiguation following JLS§6.5.2. @@ -294,6 +295,7 @@ final class AstDisambiguationPass { visitChildren(type, ctx); if (type.getReferencedSym() != null) { + postProcess(type, ctx.processor); return null; } @@ -319,9 +321,27 @@ final class AstDisambiguationPass { } assert type.getReferencedSym() != null : "Null symbol for " + type; + + postProcess(type, processor); return null; } + private void postProcess(ASTClassOrInterfaceType type, JavaAstProcessor processor) { + JTypeDeclSymbol sym = type.getReferencedSym(); + if (type.getParent() instanceof ASTAnnotation) { + if (!(sym instanceof JClassSymbol && (sym.isUnresolved() || ((JClassSymbol) sym).isAnnotation()))) { + processor.getLogger().error(type, SemanticChecksLogger.EXPECTED_ANNOTATION_TYPE); + } + return; + } + + int actualArity = ASTList.sizeOrZero(type.getTypeArguments()); + int expectedArity = sym instanceof JClassSymbol ? ((JClassSymbol) sym).getTypeParameterCount() : 0; + if (actualArity != 0 && actualArity != expectedArity) { + processor.getLogger().error(type, SemanticChecksLogger.MALFORMED_GENERIC_TYPE, expectedArity, actualArity); + } + } + @NonNull private JTypeDeclSymbol setArity(ASTClassOrInterfaceType type, JavaAstProcessor processor, String canonicalName) { int arity = ASTList.orEmpty(type.getTypeArguments()).size(); @@ -373,11 +393,11 @@ final class AstDisambiguationPass { JTypeDeclSymbol typeResult = resolveSingleTypeName(symTable, firstIdent.getImage(), ctx, name); if (typeResult != null) { - return resolveType(null, typeResult, firstIdent.getImage(), firstIdent, tokens, name, isPackageOrTypeOnly, ctx); + return resolveType(null, typeResult, false, firstIdent, tokens, name, isPackageOrTypeOnly, ctx); } // otherwise, first is reclassified as package name. - return resolvePackage(firstIdent, firstIdent.getImage(), tokens, name, isPackageOrTypeOnly, ctx); + return resolvePackage(firstIdent, new StringBuilder(firstIdent.getImage()), tokens, name, isPackageOrTypeOnly, ctx); } private static JTypeDeclSymbol resolveSingleTypeName(JSymbolTable symTable, String image, ReferenceCtx ctx, JavaNode errorLoc) { @@ -436,7 +456,7 @@ final class AstDisambiguationPass { */ private static ASTExpression resolveType(final @Nullable ASTClassOrInterfaceType qualifier, // lhs final JTypeDeclSymbol sym, // symbol for the type - final String image, // image of the new ClassType, possibly with a prepended package name + final boolean isFqcn, // whether this is a fully-qualified name final JavaccToken identifier, // ident of the simple name of the symbol final Iterator remaining, // rest of tokens, starting with following '.' final ASTAmbiguousName ambig, // original ambiguous name @@ -446,7 +466,7 @@ final class AstDisambiguationPass { TokenUtils.expectKind(identifier, JavaTokenKinds.IDENTIFIER); - final ASTClassOrInterfaceType type = new ASTClassOrInterfaceType(qualifier, image, ambig.getFirstToken(), identifier); + final ASTClassOrInterfaceType type = new ASTClassOrInterfaceType(qualifier, isFqcn, ambig.getFirstToken(), identifier); type.setSymbol(sym); if (!remaining.hasNext()) { // done @@ -474,7 +494,7 @@ final class AstDisambiguationPass { } if (inner != null) { - return resolveType(type, inner, nextSimpleName, nextIdent, remaining, ambig, isPackageOrTypeOnly, ctx); + return resolveType(type, inner, false, nextIdent, remaining, ambig, isPackageOrTypeOnly, ctx); } // no inner type, yet we have a lhs that is a type... @@ -500,7 +520,7 @@ final class AstDisambiguationPass { * class, then we report it and return the original ambiguous name. */ private static ASTExpression resolvePackage(JavaccToken identifier, - String packageImage, + StringBuilder packageImage, Iterator remaining, ASTAmbiguousName ambig, boolean isPackageOrTypeOnly, @@ -526,16 +546,17 @@ final class AstDisambiguationPass { JavaccToken nextIdent = skipToNextIdent(remaining); - String canonical = packageImage + '.' + nextIdent.getImage(); + packageImage.append('.').append(nextIdent.getImage()); + String canonical = packageImage.toString(); // Don't interpret periods as nested class separators (this will be handled by resolveType). // Otherwise lookup of a fully qualified name would be quadratic JClassSymbol nextClass = processor.getSymResolver().resolveClassFromBinaryName(canonical); if (nextClass != null) { - return resolveType(null, nextClass, canonical, nextIdent, remaining, ambig, isPackageOrTypeOnly, ctx); + return resolveType(null, nextClass, true, nextIdent, remaining, ambig, isPackageOrTypeOnly, ctx); } else { - return resolvePackage(nextIdent, canonical, remaining, ambig, isPackageOrTypeOnly, ctx); + return resolvePackage(nextIdent, packageImage, remaining, ambig, isPackageOrTypeOnly, ctx); } } @@ -543,17 +564,19 @@ final class AstDisambiguationPass { * Force resolution of the ambiguous name as a package name. * The parent type's image is set to a package name + simple name. */ - private static void forceResolveAsFullPackageNameOfParent(String packageImage, ASTAmbiguousName ambig, JavaAstProcessor processor) { + private static void forceResolveAsFullPackageNameOfParent(StringBuilder packageImage, ASTAmbiguousName ambig, JavaAstProcessor processor) { ASTClassOrInterfaceType parent = (ASTClassOrInterfaceType) ambig.getParent(); - String fullName = packageImage + '.' + parent.getSimpleName(); + packageImage.append('.').append(parent.getSimpleName()); + String fullName = packageImage.toString(); JClassSymbol parentClass = processor.getSymResolver().resolveClassFromCanonicalName(fullName); if (parentClass == null) { processor.getLogger().warning(parent, CANNOT_RESOLVE_AMBIGUOUS_NAME, fullName, Fallback.TYPE); parentClass = processor.makeUnresolvedReference(fullName); } parent.setSymbol(parentClass); - ambig.deleteInParentPrependImage('.'); + parent.setFullyQualified(); + ambig.deleteInParent(); } private static JavaccToken skipToNextIdent(Iterator remaining) { 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 a6e82f04cf..195d8e6c5c 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 @@ -11,7 +11,9 @@ 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.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression; @@ -41,7 +43,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; } @@ -135,9 +137,24 @@ 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(ASTClassOrInterfaceType 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()); } + + @Override + public Attribute visit(ASTVariableDeclaratorId node, Void data) { + return new Attribute(node, "Name", node.getName()); + } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java index d912a4be07..a86a433dd0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java @@ -77,7 +77,7 @@ public class IdenticalCatchBranchesRule extends AbstractJavaRule { @Override public Object visit(ASTTryStatement node, Object data) { - List catchStatements = node.getCatchClauses(); + List catchStatements = node.getCatchClauses().toList(); Set> equivClasses = equivalenceClasses(catchStatements); for (List identicalStmts : equivClasses) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExceptionAsFlowControlRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExceptionAsFlowControlRule.java index db771f0b1f..578f86f802 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExceptionAsFlowControlRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExceptionAsFlowControlRule.java @@ -4,14 +4,15 @@ package net.sourceforge.pmd.lang.java.rule.design; -import java.util.List; - +import net.sourceforge.pmd.lang.ast.NodeStream; import net.sourceforge.pmd.lang.java.ast.ASTCatchClause; +import net.sourceforge.pmd.lang.java.ast.ASTCatchParameter; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; -import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter; +import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall; +import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement; import net.sourceforge.pmd.lang.java.ast.ASTTryStatement; -import net.sourceforge.pmd.lang.java.ast.ASTType; +import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; /** @@ -23,24 +24,32 @@ public class ExceptionAsFlowControlRule extends AbstractJavaRule { @Override public Object visit(ASTThrowStatement node, Object data) { - ASTTryStatement parent = node.getFirstParentOfType(ASTTryStatement.class); - if (parent == null) { + JavaNode firstTryOrCatch = node.ancestors().map(NodeStream.asInstanceOf(ASTTryStatement.class, ASTCatchClause.class)).first(); + NodeStream enclosingTries = node.ancestors(ASTTryStatement.class); + if (firstTryOrCatch instanceof ASTCatchClause) { + // if the exception is thrown in a catch block, then the + // first try we're looking for is the next one + enclosingTries = enclosingTries.drop(1); + } + if (enclosingTries.isEmpty()) { return data; } - for (parent = parent.getFirstParentOfType(ASTTryStatement.class); parent != null; parent = parent - .getFirstParentOfType(ASTTryStatement.class)) { - List list = parent.findDescendantsOfType(ASTCatchClause.class); - for (ASTCatchClause catchStmt : list) { - ASTFormalParameter fp = (ASTFormalParameter) catchStmt.getChild(0); - ASTType type = fp.getFirstDescendantOfType(ASTType.class); - ASTClassOrInterfaceType name = type.getFirstDescendantOfType(ASTClassOrInterfaceType.class); - if (node.getFirstClassOrInterfaceTypeImage() != null - && node.getFirstClassOrInterfaceTypeImage().equals(name.getImage())) { - addViolation(data, name); - } - } + ASTExpression expr = node.getExpr(); + ASTClassOrInterfaceType thrownType; + if (expr instanceof ASTConstructorCall) { + // todo when typeres is up we can just use the static type of the expression + thrownType = ((ASTConstructorCall) expr).getTypeNode(); + } else { + return data; } + + enclosingTries.flatMap(ASTTryStatement::getCatchClauses) + .map(ASTCatchClause::getParameter) + .flatMap(ASTCatchParameter::getAllExceptionTypes) + // todo when type res is up: use a subtyping test + .filter(ex -> ex.getReferencedSym().equals(thrownType.getReferencedSym())) + .forEach(ex -> addViolation(data, ex)); return data; } 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 f66ad8b77d..9a0f9397b5 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 @@ -107,18 +107,15 @@ final class AnnotationSuppressionUtil { // @formatter:on private static boolean annotationSuppresses(ASTAnnotation annotation, Rule rule) { - // if (TypeHelper.symbolEquals(annotation.getTypeMirror(), SuppressWarnings.class)) { // fixme annot name disambiguation - if ("SuppressWarnings".equals(annotation.getSimpleName())) { - - for (ASTStringLiteral lit : annotation.getValuesForName(ASTMemberValuePair.VALUE_ATTR) - .filterIs(ASTStringLiteral.class)) { - String value = lit.getConstValue(); - if ("PMD".equals(value) - || ("PMD." + rule.getName()).equals(value) - || "all".equals(value) - || "serial".equals(value) && SERIAL_RULES.contains(rule.getName()) - || "unused".equals(value) && UNUSED_RULES.contains(rule.getName()) - ) { + 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 + || element.hasImageEqualTo("\"all\"") + || element.hasImageEqualTo("\"serial\"") && SERIAL_RULES.contains(rule.getName()) + || element.hasImageEqualTo("\"unused\"") && UNUSED_RULES.contains(rule.getName()) + || element.hasImageEqualTo("\"all\"")) { return true; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SemanticChecksLogger.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SemanticChecksLogger.java index 9b5e935a4e..03ab8dad22 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SemanticChecksLogger.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SemanticChecksLogger.java @@ -50,6 +50,9 @@ public interface SemanticChecksLogger { */ String MALFORMED_GENERIC_TYPE = "Maformed generic type: expected {0} type arguments, got {1}"; + // this is an error + String EXPECTED_ANNOTATION_TYPE = "Expected an annotation type"; + /** * An ambiguous name is completely ambiguous. We don't have info * about it at all, classpath is incomplete or code is incorrect. 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 a0a45643f5..d77df543c8 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,19 +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 java.util.Objects; 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; @@ -102,21 +103,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/ast/ASTThrowStatementTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTThrowStatementTest.java deleted file mode 100644 index c31912118a..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTThrowStatementTest.java +++ /dev/null @@ -1,33 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; - -import org.junit.Test; - -/** - * Created on Jan 19, 2005 - * @author mgriffa - */ -public class ASTThrowStatementTest extends BaseParserTest { - - @Test - public final void testGetFirstASTNameImageNull() { - ASTThrowStatement t = java.getNodes(ASTThrowStatement.class, NULL_NAME).get(0); - assertNull(t.getFirstClassOrInterfaceTypeImage()); - } - - @Test - public final void testGetFirstASTNameImageNew() { - ASTThrowStatement t = java.getNodes(ASTThrowStatement.class, OK_NAME).get(0); - assertEquals("FooException", t.getFirstClassOrInterfaceTypeImage()); - } - - private static final String NULL_NAME = "public class Test {\n void bar() {\n throw e;\n }\n}"; - - private static final String OK_NAME = "public class Test {\n void bar() {\n throw new FooException();\n }\n}"; -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorIdTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorIdTest.java index d0230b626b..ed393898dd 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorIdTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorIdTest.java @@ -26,7 +26,7 @@ public class ASTVariableDeclaratorIdTest extends BaseParserTest { ASTVariableDeclaratorId id = acu.findDescendantsOfType(ASTVariableDeclaratorId.class).get(0); ASTClassOrInterfaceType name = (ASTClassOrInterfaceType) id.getTypeNameNode(); - assertEquals("String", name.getImage()); + assertEquals("String", name.getSimpleName()); } @Test @@ -35,7 +35,7 @@ public class ASTVariableDeclaratorIdTest extends BaseParserTest { ASTVariableDeclaratorId id = acu.findDescendantsOfType(ASTVariableDeclaratorId.class).get(0); ASTClassOrInterfaceType name = (ASTClassOrInterfaceType) id.getTypeNode(); - assertEquals("String", name.getImage()); + assertEquals("String", name.getSimpleName()); } @Test diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java index 66e096b052..64f2bb3d66 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java @@ -30,6 +30,7 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.JavaLanguageModule; import net.sourceforge.pmd.lang.java.JavaParsingHelper; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.rule.XPathRule; import net.sourceforge.pmd.lang.rule.xpath.JaxenXPathRuleQuery; import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery; @@ -182,8 +183,8 @@ public class XPathRuleTest extends RuleTst { xpathRuleQuery.setVersion(XPathRuleQuery.XPATH_1_0); List nodes = xpathRuleQuery.evaluate(cu, ruleContext); assertEquals(2, nodes.size()); - assertEquals("Bar", nodes.get(0).getImage()); - assertEquals("Baz", nodes.get(1).getImage()); + assertEquals("Bar", ((JavaNode) nodes.get(0)).getText().toString()); + assertEquals("Baz", ((JavaNode) nodes.get(1)).getText().toString()); // XPATH version 2.0 xpathRuleQuery = new SaxonXPathRuleQuery(); @@ -192,8 +193,8 @@ public class XPathRuleTest extends RuleTst { xpathRuleQuery.setVersion(XPathRuleQuery.XPATH_2_0); nodes = xpathRuleQuery.evaluate(cu, ruleContext); assertEquals(2, nodes.size()); - assertEquals("Bar", nodes.get(0).getImage()); - assertEquals("Baz", nodes.get(1).getImage()); + assertEquals("Bar", ((JavaNode) nodes.get(0)).getText().toString()); + assertEquals("Baz", ((JavaNode) nodes.get(1)).getText().toString()); } private static Report getReportForTestString(Rule r, String test) throws PMDException { 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..b1a0c8741f 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") + 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") + 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") 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") 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") 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/ASTConstructorCallTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTConstructorCallTest.kt index 07a6b9a757..c274f33d85 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTConstructorCallTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTConstructorCallTest.kt @@ -12,12 +12,9 @@ class ASTConstructorCallTest : ParserTestSpec({ constructorCall { it::isQualifiedInstanceCreation shouldBe false - it::getTypeNode shouldBe child { - it::getTypeImage shouldBe "Foo" - } - - it::getArguments shouldBe child { + it::getTypeNode shouldBe classType("Foo") + it::getArguments shouldBe argList(1) { variableAccess("a") } } @@ -31,9 +28,7 @@ class ASTConstructorCallTest : ParserTestSpec({ unspecifiedChild() } - it::getTypeNode shouldBe child { - it::getTypeImage shouldBe "Foo" - + it::getTypeNode shouldBe classType("Foo") { it::getTypeArguments shouldBe child { unspecifiedChild() } @@ -49,8 +44,7 @@ class ASTConstructorCallTest : ParserTestSpec({ it::getExplicitTypeArguments shouldBe null - it::getTypeNode shouldBe child { - it::getTypeImage shouldBe "Foo" + it::getTypeNode shouldBe classType("Foo") { annotation("Lol") @@ -84,9 +78,7 @@ class ASTConstructorCallTest : ParserTestSpec({ it::getQualifier shouldBe ambiguousName("a.g") } - it::getTypeNode shouldBe child { - it::getTypeImage shouldBe "Foo" - } + it::getTypeNode shouldBe classType("Foo") it::getArguments shouldBe argList { @@ -102,9 +94,7 @@ class ASTConstructorCallTest : ParserTestSpec({ it::getQualifier shouldBe variableAccess("a") - it::getTypeNode shouldBe child { - it::getTypeImage shouldBe "Foo" - } + it::getTypeNode shouldBe classType("Foo") it::getArguments shouldBe child { @@ -153,8 +143,7 @@ class ASTConstructorCallTest : ParserTestSpec({ it::getExplicitTypeArguments shouldBe null - it::getTypeNode shouldBe child { - it::getTypeImage shouldBe "Foo" + it::getTypeNode shouldBe classType("Foo") { annotation("Lol") diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTFieldAccessTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTFieldAccessTest.kt index d93223fc5f..3b80819213 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTFieldAccessTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTFieldAccessTest.kt @@ -16,10 +16,7 @@ class ASTFieldAccessTest : ParserTestSpec({ fieldAccess("foo") { it::getQualifier shouldBe child { - it::getQualifier shouldBe child { - it.typeArguments shouldBe null - it.typeImage shouldBe "Type" - } + it::getQualifier shouldBe classType("Type") } } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSuperExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSuperExpressionTest.kt index d25bd99bfa..fcc563f20a 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSuperExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSuperExpressionTest.kt @@ -42,9 +42,7 @@ class ASTSuperExpressionTest : ParserTestSpec({ methodCall("foo") { it::getQualifier shouldBe child { - it::getQualifier shouldBe child { - it::getImage shouldBe "Type" - } + it::getQualifier shouldBe classType("Type") } unspecifiedChild() @@ -55,8 +53,7 @@ class ASTSuperExpressionTest : ParserTestSpec({ methodCall("foo") { it::getQualifier shouldBe child { - it::getQualifier shouldBe child { - it::getImage shouldBe "ASTThisExpression" + it::getQualifier shouldBe qualClassType("net.sourceforge.pmd.lang.java.ast.ASTThisExpression") { it::getTypeArguments shouldBe null it::getQualifier shouldBe null diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTThisExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTThisExpressionTest.kt index d0558f2410..110aba2410 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTThisExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTThisExpressionTest.kt @@ -20,16 +20,13 @@ class ASTThisExpressionTest : ParserTestSpec({ inContext(ExpressionParsingCtx) { "Type.this" should parseAs { thisExpr { - child { - it::getImage shouldBe "Type" - } + classType("Type") } } "net.sourceforge.pmd.lang.java.ast.ASTThisExpression.this" should parseAs { thisExpr { - child { - it::getImage shouldBe "ASTThisExpression" + qualClassType("net.sourceforge.pmd.lang.java.ast.ASTThisExpression") { it::getTypeArguments shouldBe null it::getQualifier shouldBe null diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTypeTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTypeTest.kt index 94ea3d1f2f..44b767ab19 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTypeTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTypeTest.kt @@ -16,8 +16,6 @@ class ASTTypeTest : ParserTestSpec({ "java.util.List" should parseAs { classType("List") { - it::getTypeImage shouldBe "java.util.List" - it::getImage shouldBe "List" it::getTypeArguments shouldBe null it::getQualifier shouldBe null @@ -31,25 +29,19 @@ class ASTTypeTest : ParserTestSpec({ classType("List") { it::getQualifier shouldBe null - it::getImage shouldBe "List" it::getAmbiguousLhs shouldBe child { it::getName shouldBe "java.util" } - it::getTypeArguments shouldBe child { - child { - it::getTypeImage shouldBe "F" - } + it::getTypeArguments shouldBe typeArgList(1) { + classType("F") } } } "foo" should parseAs { classType("foo") { - it::getTypeImage shouldBe "foo" - it::getImage shouldBe "foo" - it::getAmbiguousLhs shouldBe null it::getQualifier shouldBe null } @@ -82,21 +74,15 @@ class ASTTypeTest : ParserTestSpec({ annotation("C") annotation("K") - - it::getTypeImage shouldBe "java.util.Map" } } "java.util.Map.@Foo Entry" should parseAs { classType("Entry") { - it::getTypeImage shouldBe "java.util.Map.Entry" - it::getQualifier shouldBe null it::getAmbiguousLhs shouldBe child { - it::getTypeImage shouldBe "java.util.Map" - it::getImage shouldBe "java.util.Map" it::getName shouldBe "java.util.Map" } @@ -120,14 +106,10 @@ class ASTTypeTest : ParserTestSpec({ "Foo.@A Bar.Brew" should parseAs { classType("Brew") { - it::getTypeImage shouldBe "Foo.Bar.Brew" - it::getQualifier shouldBe classType("Bar") { - it::getTypeImage shouldBe "Foo.Bar" it::getTypeArguments shouldBe null it::getQualifier shouldBe classType("Foo") { - it::getTypeImage shouldBe "Foo" it::getTypeArguments shouldBe typeArgList { classType("K") 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 6a0d754c43..c2cec258f0 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 @@ -5,19 +5,16 @@ import io.kotlintest.Matcher import io.kotlintest.Result import io.kotlintest.matchers.collections.shouldBeEmpty import io.kotlintest.matchers.types.shouldBeInstanceOf +import io.kotlintest.shouldBe 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.types.JPrimitiveType.PrimitiveTypeKind import net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind.* -import java.util.* -import kotlin.reflect.KCallable fun > C?.shouldContainAtMostOneOf(vararg expected: T) { this shouldNotBe null @@ -69,9 +66,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() } @@ -370,9 +367,9 @@ fun TreeNodeWrapper.classType(simpleName: String, contents: NodeSpec.qualClassType(canoName: String, contents: NodeSpec = EmptyAssertions) = child(ignoreChildren = contents == EmptyAssertions) { - it::getCanonicalName shouldBe canoName - it::getImage shouldBe canoName - it::getSimpleName shouldBe canoName.substringAfterLast('.') + val simpleName = canoName.substringAfterLast('.') + it::getSimpleName shouldBe simpleName + it.text.toString() shouldBe canoName contents() } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TypeDisambiguationTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TypeDisambiguationTest.kt index e21fa2b55c..4b00f6646d 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TypeDisambiguationTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TypeDisambiguationTest.kt @@ -1,11 +1,13 @@ package net.sourceforge.pmd.lang.java.ast import io.kotlintest.shouldBe +import net.sourceforge.pmd.lang.ast.test.* import net.sourceforge.pmd.lang.ast.test.shouldBe -import net.sourceforge.pmd.lang.ast.test.shouldBeA -import net.sourceforge.pmd.lang.ast.test.shouldMatchN import net.sourceforge.pmd.lang.java.symbols.JClassSymbol import net.sourceforge.pmd.lang.java.symbols.table.internal.SemanticChecksLogger +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull class TypeDisambiguationTest : ParserTestSpec({ @@ -24,11 +26,12 @@ class TypeDisambiguationTest : ParserTestSpec({ val (f1) = acu.descendants(ASTFieldDeclaration::class.java).toList() f1.typeNode.shouldMatchNode { + it::isFullyQualified shouldBe false it::getSimpleName shouldBe "Inner" - it::getImage shouldBe "Inner" it::getReferencedSym shouldBe inner it::getAmbiguousLhs shouldBe null it::getQualifier shouldBe classType("Foo") { + it::isFullyQualified shouldBe false it::getReferencedSym shouldBe foo } } @@ -40,18 +43,19 @@ class TypeDisambiguationTest : ParserTestSpec({ inContext(TypeParsingCtx) { "javasymbols.testdata.Statics" should parseAs { - classType("Statics") { - it::getImage shouldBe "javasymbols.testdata.Statics" + qualClassType("javasymbols.testdata.Statics") { + it::isFullyQualified shouldBe true it::getQualifier shouldBe null it::getAmbiguousLhs shouldBe null } } "javasymbols.testdata.Statics.PublicStatic" should parseAs { - classType("PublicStatic") { + qualClassType("javasymbols.testdata.Statics.PublicStatic") { + it::isFullyQualified shouldBe false - it::getQualifier shouldBe classType("Statics") { - it::getImage shouldBe "javasymbols.testdata.Statics" + it::getQualifier shouldBe qualClassType("javasymbols.testdata.Statics") { + it::isFullyQualified shouldBe true it::getQualifier shouldBe null it::getAmbiguousLhs shouldBe null } @@ -133,4 +137,101 @@ class TypeDisambiguationTest : ParserTestSpec({ refInScratch.referencedSym shouldBe aMem } } + + parserTest("Malformed types") { + val logger = enableProcessing() + + val acu = parser.parse(""" + package p; + class Scratch { + static class K {} + static class Foo {} + class Inner {} // non-static + + Scratch.Foo m0; // ok + Scratch.K m1; // error + Scratch.K m2; // ok + Scratch.Foo m3; // error + Scratch.Foo m4; // error + Scratch.Foo m5; // raw type, ok + + Scratch s0; // ok + Scratch s1; // error + Scratch s2; // raw type, ok + + // Scratch.Foo m6; // todo error: Foo is static + // Scratch.Foo m7; // todo error: Foo is static + + // Scratch.Inner m; // ok, fully parameterized + // Scratch.Inner m; // todo error: Scratch must be parameterized + // Scratch.Inner m; // ok, raw type + } + """) + + val (m0, m1, m2, m3, m4, m5, s0, s1, s2) = + acu.descendants(ASTFieldDeclaration::class.java).map { it.typeNode as ASTClassOrInterfaceType }.toList() + + fun assertErrored(t: ASTClassOrInterfaceType, expected: Int, actual: Int) { + val errs = logger.errors[SemanticChecksLogger.MALFORMED_GENERIC_TYPE]?.filter { it.first == t } ?: emptyList() + assertEquals(errs.size, 1, "`${t.text}` should have produced a single error") + errs.single().second.toList() shouldBe listOf(expected, actual) + } + + fun assertNoError(t: ASTClassOrInterfaceType) { + val err = logger.errors[SemanticChecksLogger.MALFORMED_GENERIC_TYPE]?.firstOrNull { it.first == t } + assertNull(err, "`${t.text}` should not have produced an error") + } + + assertNoError(m0) + assertErrored(m1, expected = 0, actual = 1) + assertNoError(m2) + assertErrored(m3, expected = 2, actual = 1) + assertErrored(m4, expected = 2, actual = 3) + assertNoError(m5) + + assertNoError(s0) + assertErrored(s1, expected = 1, actual = 2) + assertNoError(s2) + } + + parserTest("Invalid annotations") { + val logger = enableProcessing() + + val acu = parser.parse(""" + package p; + class C { + @interface A { } + interface I { } + + + @T + @C + @I + @Unresolved + @A + int field; + } + """) + + val (aT, aC, aI, aUnresolved, aOk) = + acu.descendants(ASTAnnotation::class.java).map { it.typeNode }.toList() + + fun assertErrored(t: ASTClassOrInterfaceType) { + val errs = logger.errors[SemanticChecksLogger.EXPECTED_ANNOTATION_TYPE]?.filter { it.first == t } ?: emptyList() + assertEquals(errs.size, 1, "`${t.text}` should have produced a single error") + errs.single().second.toList() shouldBe emptyList() + } + + fun assertNoError(t: ASTClassOrInterfaceType) { + val err = logger.errors[SemanticChecksLogger.MALFORMED_GENERIC_TYPE]?.firstOrNull { it.first == t } + assertNull(err, "`${t.text}` should not have produced an error") + } + + assertNoError(aUnresolved) + assertNoError(aOk) + + assertErrored(aT) + assertErrored(aC) + assertErrored(aI) + } }) 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 c559ece244..7ac0cf4629 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 @@ -134,8 +135,8 @@ class TypeParamScopingTest : ParserTestSpec({ class Foo { - @ /*Foo#*/ T // type params are not in scope in modifier list - void foo(T pt, X px) { + @ /*Foo#*/ Y // type params of the method are not in scope in modifier list + void foo(T pt, X px) { T vt; X vx; @@ -145,12 +146,14 @@ class TypeParamScopingTest : ParserTestSpec({ X vx2; } } + + @interface Y { } } """) // type parameters - val (x, t, t2) = acu.descendants(ASTTypeParameter::class.java).toList() + val (x, t, t2, y2) = acu.descendants(ASTTypeParameter::class.java).toList() // parameters val (pt, px) = acu.descendants(ASTFormalParameter::class.java).map { it.typeNode }.toList() @@ -159,7 +162,7 @@ class TypeParamScopingTest : ParserTestSpec({ val (vt, vx, vx2) = acu.descendants(ASTLocalVariableDeclaration::class.java).map { it.typeNode }.toList() // classes - val (_, localX) = acu.descendants(ASTClassOrInterfaceDeclaration::class.java).toList() + val (_, localX, annotY) = acu.descendants(ASTAnyTypeDeclaration::class.java).toList() doTest("TParams of class are in scope inside method tparam declaration") { @@ -189,7 +192,8 @@ class TypeParamScopingTest : ParserTestSpec({ val annot = acu.descendants(ASTAnnotation::class.java).first()!! - annot.symbolTable.shouldResolveTypeTo("T", t.symbol) //not t2 + annot.symbolTable.shouldResolveTypeTo("Y", annotY.symbol) // not the Y of the method + annot.symbol.shouldBeSameInstanceAs(annotY.symbol) } doTest("Local class shadows type param") {