diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index 9f455e78..710ce049 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -178,6 +178,11 @@ slf4j-api test + + org.springframework + spring-core + provided + org.springframework spring-test diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java index 718caa28..e3ff7d3a 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java @@ -3,7 +3,9 @@ package tech.picnic.errorprone.bugpatterns; import static com.google.errorprone.BugPattern.LinkType.NONE; import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static java.util.function.Predicate.not; +import com.google.auto.common.AnnotationMirrors; import com.google.auto.service.AutoService; import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; @@ -14,17 +16,24 @@ import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.matchers.AnnotationMatcherUtils; import com.google.errorprone.matchers.Description; +import com.google.errorprone.predicates.TypePredicate; +import com.google.errorprone.predicates.TypePredicates; +import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.NewArrayTree; import com.sun.source.tree.Tree.Kind; +import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.Symbol; import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.function.BiFunction; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Stream; +import javax.lang.model.element.AnnotationValue; import tech.picnic.errorprone.bugpatterns.util.SourceCode; /** A {@link BugChecker} which flags annotations that could be written more concisely. */ @@ -40,6 +49,7 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot private static final ImmutableSet>> FIX_FACTORIES = ImmutableSet.of( + CanonicalAnnotationSyntax::useSpringAlias, CanonicalAnnotationSyntax::dropRedundantParentheses, CanonicalAnnotationSyntax::dropRedundantValueAttribute, CanonicalAnnotationSyntax::dropRedundantCurlies); @@ -54,6 +64,69 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot .orElse(Description.NO_MATCH); } + private static Optional useSpringAlias(AnnotationTree tree, VisitorState state) { + boolean canDropAttributeName = tree.getArguments().size() == 1; + + // XXX: The reduce is repeated. Fix. + return tree.getArguments().stream() + .flatMap(arg -> useBetterAlias(arg, canDropAttributeName, state).stream()) + .reduce(SuggestedFix.Builder::merge) + .map(SuggestedFix.Builder::build); + } + + private static Optional useBetterAlias( + ExpressionTree tree, boolean canDropAttributeName, VisitorState state) { + if (tree.getKind() != Kind.ASSIGNMENT) { + return Optional.empty(); + } + + AssignmentTree assignment = (AssignmentTree) tree; + ExpressionTree element = assignment.getVariable(); + if (canDropAttributeName == "value".equals(SourceCode.treeToString(element, state))) { + return Optional.empty(); + } + + return getAlias(element, state) + .map( + alias -> + "value".equals(alias) + ? SuggestedFix.builder() + .replace( + assignment, SourceCode.treeToString(assignment.getExpression(), state)) + : SuggestedFix.builder().replace(element, alias)); + } + + // XXX: Also add support for non-default `AliasFor#annotation` values! + private static Optional getAlias(ExpressionTree tree, VisitorState state) { + Symbol sym = ASTHelpers.getSymbol(tree); + if (sym == null) { + return Optional.empty(); + } + + // XXX: Extract. + TypePredicate isAliasFor = + TypePredicates.isExactType("org.springframework.core.annotation.AliasFor"); + TypePredicate isAnnotation = TypePredicates.isExactType("java.lang.annotation.Annotation"); + + return sym.getAnnotationMirrors().stream() + .filter(m -> isAliasFor.apply(m.type, state)) + .filter( + m -> + isAnnotation.apply( + ((Attribute.Class) AnnotationMirrors.getAnnotationValue(m, "annotation")) + .classType, + state)) + .flatMap( + m -> + Stream.of( + AnnotationMirrors.getAnnotationValue(m, "value"), + AnnotationMirrors.getAnnotationValue(m, "attribute"))) + .map(AnnotationValue::getValue) + .map(Object::toString) + .filter(not(String::isEmpty)) + .findFirst(); + } + private static Optional dropRedundantParentheses(AnnotationTree tree, VisitorState state) { if (!tree.getArguments().isEmpty()) { /* Parentheses are necessary. */ diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxTest.java index a6ae6c67..0429b6d2 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxTest.java @@ -11,6 +11,24 @@ final class CanonicalAnnotationSyntaxTest { private final BugCheckerRefactoringTestHelper refactoringTestHelper = BugCheckerRefactoringTestHelper.newInstance(CanonicalAnnotationSyntax.class, getClass()); + // XXX: Merge with code below + @Test + public void testIdentificationSpring() { + compilationTestHelper + .addSourceLines( + "pkg/A.java", + "package pkg;", + "", + "import org.springframework.web.bind.annotation.RequestMapping;", + "", + "// BUG: Diagnostic contains:", + // "@RequestMapping(path = \"/path\")", + "@RequestMapping(value = \"/path\", consumes = \"text/plain\")", + "class A {", + "}") + .doTest(); + } + @Test void identification() { compilationTestHelper