This commit is contained in:
Stephan Schroevers
2020-12-09 08:48:34 +01:00
parent 17035a1623
commit 7d9ffd8cb0
3 changed files with 96 additions and 0 deletions

View File

@@ -178,6 +178,11 @@
<artifactId>slf4j-api</artifactId> <artifactId>slf4j-api</artifactId>
<scope>test</scope> <scope>test</scope>
</dependency> </dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-core</artifactId>
<scope>provided</scope>
</dependency>
<dependency> <dependency>
<groupId>org.springframework</groupId> <groupId>org.springframework</groupId>
<artifactId>spring-test</artifactId> <artifactId>spring-test</artifactId>

View File

@@ -3,7 +3,9 @@ package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.NONE; import static com.google.errorprone.BugPattern.LinkType.NONE;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; 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.auto.service.AutoService;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern; 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.fixes.SuggestedFix;
import com.google.errorprone.matchers.AnnotationMatcherUtils; import com.google.errorprone.matchers.AnnotationMatcherUtils;
import com.google.errorprone.matchers.Description; 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.AnnotationTree;
import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.AssignmentTree;
import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.NewArrayTree; import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.Tree.Kind; 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.ArrayList;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.function.BiFunction; import java.util.function.BiFunction;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import java.util.stream.Stream;
import javax.lang.model.element.AnnotationValue;
import tech.picnic.errorprone.bugpatterns.util.SourceCode; import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} which flags annotations that could be written more concisely. */ /** 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<BiFunction<AnnotationTree, VisitorState, Optional<Fix>>> private static final ImmutableSet<BiFunction<AnnotationTree, VisitorState, Optional<Fix>>>
FIX_FACTORIES = FIX_FACTORIES =
ImmutableSet.of( ImmutableSet.of(
CanonicalAnnotationSyntax::useSpringAlias,
CanonicalAnnotationSyntax::dropRedundantParentheses, CanonicalAnnotationSyntax::dropRedundantParentheses,
CanonicalAnnotationSyntax::dropRedundantValueAttribute, CanonicalAnnotationSyntax::dropRedundantValueAttribute,
CanonicalAnnotationSyntax::dropRedundantCurlies); CanonicalAnnotationSyntax::dropRedundantCurlies);
@@ -54,6 +64,69 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot
.orElse(Description.NO_MATCH); .orElse(Description.NO_MATCH);
} }
private static Optional<Fix> 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<SuggestedFix.Builder> 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<String> 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<Fix> dropRedundantParentheses(AnnotationTree tree, VisitorState state) { private static Optional<Fix> dropRedundantParentheses(AnnotationTree tree, VisitorState state) {
if (!tree.getArguments().isEmpty()) { if (!tree.getArguments().isEmpty()) {
/* Parentheses are necessary. */ /* Parentheses are necessary. */

View File

@@ -11,6 +11,24 @@ final class CanonicalAnnotationSyntaxTest {
private final BugCheckerRefactoringTestHelper refactoringTestHelper = private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(CanonicalAnnotationSyntax.class, getClass()); 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 @Test
void identification() { void identification() {
compilationTestHelper compilationTestHelper