From 8ced020ba6fb9bd204d3a52131c8ddea09dfe3ee Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 5 Nov 2022 17:00:19 +0100 Subject: [PATCH] Post rebase fixes --- .../SimplifyTimeAnnotationCheck.java | 39 +++++++---- .../SimplifyTimeAnnotationCheckTest.java | 69 ++++++++++++++----- .../bugpatterns/SpringMvcAnnotationTest.java | 1 - 3 files changed, 75 insertions(+), 34 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheck.java index 0149430f42..52fe13a3db 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheck.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheck.java @@ -3,6 +3,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.errorprone.BugPattern.LinkType.CUSTOM; +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static java.util.Objects.requireNonNull; +import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; import com.google.auto.service.AutoService; import com.google.common.collect.ImmutableList; @@ -12,9 +17,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.errorprone.BugPattern; -import com.google.errorprone.BugPattern.LinkType; -import com.google.errorprone.BugPattern.SeverityLevel; -import com.google.errorprone.BugPattern.StandardTags; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher; @@ -35,6 +37,8 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; +import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher; +import tech.picnic.errorprone.bugpatterns.util.SourceCode; /** * A {@link BugChecker} which flags annotations with time attributes that can be written more @@ -42,16 +46,19 @@ */ @AutoService(BugChecker.class) @BugPattern( - name = "SimplifyTimeAnnotation", summary = "Simplifies annotations which express an amount of time using a `TimeUnit`", - linkType = LinkType.NONE, - severity = SeverityLevel.WARNING, - tags = StandardTags.SIMPLIFICATION) + link = BUG_PATTERNS_BASE_URL + "SimplifyTimeAnnotation", + linkType = CUSTOM, + severity = WARNING, + tags = SIMPLIFICATION) public final class SimplifyTimeAnnotationCheck extends BugChecker implements AnnotationTreeMatcher { private static final long serialVersionUID = 1L; private static final AnnotationAttributeMatcher ARGUMENT_SELECTOR = createAnnotationAttributeMatcher(); + /** Instantiates a new {@link SimplifyTimeAnnotationCheck} instance. */ + public SimplifyTimeAnnotationCheck() {} + @Override public Description matchAnnotation(AnnotationTree annotationTree, VisitorState state) { ImmutableList arguments = @@ -89,7 +96,7 @@ private static Optional trySimplification( ImmutableMap timeValues = annotationDescriptor.timeFields.stream() - .map(field -> Maps.immutableEntry(field, getValue(field, indexedAttributes))) + .map(field -> Map.entry(field, getValue(field, indexedAttributes))) .filter(entry -> entry.getValue().isPresent()) .collect(toImmutableMap(Map.Entry::getKey, entry -> entry.getValue().orElseThrow())); @@ -129,7 +136,7 @@ private static Optional trySimplification( .values())); return getExplicitAttributesFix( - annotation, simplifications, annotationDescriptor.timeUnitField, commonUnit); + annotation, simplifications, annotationDescriptor.timeUnitField, commonUnit, state); } private static boolean containsAnyAttributeOf( @@ -152,7 +159,7 @@ private static Fix getImplicitValueAttributeFix( TimeUnit newTimeUnit, VisitorState state) { String synthesizedAnnotation = - Util.treeToString(annotation, state) + SourceCode.treeToString(annotation, state) .replaceFirst( "\\(.+\\)", String.format("(value=%s, %s=%s)", newValue, timeUnitField, newTimeUnit.name())); @@ -166,15 +173,17 @@ private static Optional getExplicitAttributesFix( AnnotationTree annotation, Map simplifications, String timeUnitField, - TimeUnit newUnit) { + TimeUnit newUnit, + VisitorState state) { return simplifications.entrySet().stream() .map( simplificationEntry -> SuggestedFixes.updateAnnotationArgumentValues( - annotation, timeUnitField, ImmutableList.of(newUnit.name())) + annotation, state, timeUnitField, ImmutableList.of(newUnit.name())) .merge( SuggestedFixes.updateAnnotationArgumentValues( annotation, + state, simplificationEntry.getKey(), ImmutableList.of( String.valueOf(simplificationEntry.getValue().toUnit(newUnit)))))) @@ -215,8 +224,10 @@ private static VarSymbol getDefaultTimeUnit(AnnotationTree annotation, String ar MethodSymbol argumentSymbol = (MethodSymbol) Iterables.getOnlyElement( - scope.getSymbols(symbol -> symbol.getQualifiedName().contentEquals(argument))); - return (VarSymbol) argumentSymbol.getDefaultValue().getValue(); + ASTHelpers.scope(scope) + .getSymbols(symbol -> symbol.getQualifiedName().contentEquals(argument))); + return (VarSymbol) + requireNonNull(argumentSymbol.getDefaultValue(), "Default value missing").getValue(); } private static AnnotationAttributeMatcher createAnnotationAttributeMatcher() { diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheckTest.java index 960b528940..676ab22ecb 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheckTest.java @@ -19,11 +19,14 @@ void identification() { "import org.junit.jupiter.api.Timeout;", "", "interface A {", - " @Timeout(6) A noSimplification();", + " @Timeout(6)", + " A noSimplification();", " // BUG: Diagnostic contains:", - " @Timeout(60) A simple();", + " @Timeout(60)", + " A simple();", " // BUG: Diagnostic contains:", - " @Timeout(value = 60 * 1000, unit = TimeUnit.MILLISECONDS) A explicitUnit();", + " @Timeout(value = 60 * 1000, unit = TimeUnit.MILLISECONDS)", + " A explicitUnit();", "}") .doTest(); } @@ -37,8 +40,11 @@ void identificationBannedField() { "", "interface A {", " // BUG: Diagnostic contains:", - " @Scheduled(fixedDelay = 6_000) A scheduledFixedDelay();", - " @Scheduled(fixedDelay = 6_000, fixedRateString = \"\") A bannedAttribute();", + " @Scheduled(fixedDelay = 6_000)", + " A scheduledFixedDelay();", + "", + " @Scheduled(fixedDelay = 6_000, fixedRateString = \"\")", + " A bannedAttribute();", "}") .doTest(); } @@ -52,11 +58,20 @@ void replacement() { "import org.springframework.scheduling.annotation.Scheduled;", "", "interface A {", - " @Timeout(value = 60) A simple();", - " @Scheduled(fixedDelay = 6_000) A scheduledFixedDelay();", - " @Scheduled(fixedDelay = 5_000, initialDelay = 6_000, fixedRate = 7_000) A scheduledMultiple();", - " @Scheduled(fixedDelay = 60_000, initialDelay = 6_000, fixedRate = 7_000) A scheduledCommonUnit();", - " @Scheduled(fixedDelay = 5, initialDelay = 6_000, fixedRate = 7_000) A scheduledNoSimplification();", + " @Timeout(value = 60)", + " A simple();", + "", + " @Scheduled(fixedDelay = 6_000)", + " A scheduledFixedDelay();", + "", + " @Scheduled(fixedDelay = 5_000, initialDelay = 6_000, fixedRate = 7_000)", + " A scheduledMultiple();", + "", + " @Scheduled(fixedDelay = 60_000, initialDelay = 6_000, fixedRate = 7_000)", + " A scheduledCommonUnit();", + "", + " @Scheduled(fixedDelay = 5, initialDelay = 6_000, fixedRate = 7_000)", + " A scheduledNoSimplification();", "}") .addOutputLines( "out/A.java", @@ -67,11 +82,20 @@ void replacement() { "import org.springframework.scheduling.annotation.Scheduled;", "", "interface A {", - " @Timeout(value = 1, unit = MINUTES) A simple();", - " @Scheduled(timeUnit = SECONDS, fixedDelay = 6) A scheduledFixedDelay();", - " @Scheduled(timeUnit = SECONDS, fixedDelay = 5, initialDelay = 6, fixedRate = 7) A scheduledMultiple();", - " @Scheduled(timeUnit = SECONDS, fixedDelay = 60, initialDelay = 6, fixedRate = 7) A scheduledCommonUnit();", - " @Scheduled(fixedDelay = 5, initialDelay = 6_000, fixedRate = 7_000) A scheduledNoSimplification();", + " @Timeout(value = 1, unit = MINUTES)", + " A simple();", + "", + " @Scheduled(timeUnit = SECONDS, fixedDelay = 6)", + " A scheduledFixedDelay();", + "", + " @Scheduled(timeUnit = SECONDS, fixedDelay = 5, initialDelay = 6, fixedRate = 7)", + " A scheduledMultiple();", + "", + " @Scheduled(timeUnit = SECONDS, fixedDelay = 60, initialDelay = 6, fixedRate = 7)", + " A scheduledCommonUnit();", + "", + " @Scheduled(fixedDelay = 5, initialDelay = 6_000, fixedRate = 7_000)", + " A scheduledNoSimplification();", "}") .doTest(); } @@ -84,7 +108,8 @@ void replacementValueOnly() { "import org.junit.jupiter.api.Timeout;", "", "interface A {", - " @Timeout(60) A simple();", + " @Timeout(60)", + " A simple();", "}") .addOutputLines( "out/A.java", @@ -93,7 +118,8 @@ void replacementValueOnly() { "import org.junit.jupiter.api.Timeout;", "", "interface A {", - " @Timeout(value = 1, unit = MINUTES) A simple();", + " @Timeout(value = 1, unit = MINUTES)", + " A simple();", "}") .doTest(); } @@ -102,13 +128,18 @@ void replacementValueOnly() { void replacementFqcn() { refactoringTestHelper .addInputLines( - "in/A.java", "interface A {", " @org.junit.jupiter.api.Timeout(60) A simple();", "}") + "in/A.java", + "interface A {", + " @org.junit.jupiter.api.Timeout(60)", + " A simple();", + "}") .addOutputLines( "out/A.java", "import static java.util.concurrent.TimeUnit.MINUTES;", "", "interface A {", - " @org.junit.jupiter.api.Timeout(value = 1, unit = MINUTES) A simple();", + " @org.junit.jupiter.api.Timeout(value = 1, unit = MINUTES)", + " A simple();", "}") .doTest(); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java index e65d32c336..a1cad3380c 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java @@ -19,7 +19,6 @@ void identification() { "import static org.springframework.web.bind.annotation.RequestMethod.DELETE;", "import static org.springframework.web.bind.annotation.RequestMethod.GET;", "import static org.springframework.web.bind.annotation.RequestMethod.HEAD;", - "import static org.springframework.web.bind.annotation.RequestMethod.PATCH;", "import static org.springframework.web.bind.annotation.RequestMethod.POST;", "import static org.springframework.web.bind.annotation.RequestMethod.PUT;", "",