Skip to content

Commit

Permalink
Post rebase fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202
Stephan202 committed Nov 5, 2022
1 parent 389928c commit 8ced020
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 34 deletions.
39 changes: 25 additions & 14 deletions ...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -35,23 +37,28 @@
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
* concisely.
*/
@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<ExpressionTree> arguments =
Expand Down Expand Up @@ -89,7 +96,7 @@ private static Optional<Fix> trySimplification(

ImmutableMap<String, Number> 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()));

Expand Down Expand Up @@ -129,7 +136,7 @@ private static Optional<Fix> trySimplification(
.values()));

return getExplicitAttributesFix(
annotation, simplifications, annotationDescriptor.timeUnitField, commonUnit);
annotation, simplifications, annotationDescriptor.timeUnitField, commonUnit, state);
}

private static boolean containsAnyAttributeOf(
Expand All @@ -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()));
Expand All @@ -166,15 +173,17 @@ private static Optional<Fix> getExplicitAttributesFix(
AnnotationTree annotation,
Map<String, TimeSimplifier.Simplification> 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))))))
Expand Down Expand Up @@ -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() {
Expand Down
69 changes: 50 additions & 19 deletions ...rib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeAnnotationCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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",
Expand All @@ -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();
}
Expand All @@ -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",
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand Down
1 change: 0 additions & 1 deletion ...one-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;",
"",
Expand Down

0 comments on commit 8ced020

Please sign in to comment.