From dd8d094b5a2291da8ae5653bbee71fe4dc16a138 Mon Sep 17 00:00:00 2001
From: Nathan Kooij <nathan.kooij@teampicnic.com>
Date: Tue, 28 Sep 2021 09:39:08 +0200
Subject: [PATCH] Suggest a fix

---
 .../SimplifyTimeBasedAnnotationCheck.java     | 40 +++++++++++++++----
 .../SimplifyTimeBasedAnnotationCheckTest.java |  5 ++-
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheck.java
index 84552719..51092348 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheck.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheck.java
@@ -14,6 +14,8 @@ import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
 import com.google.errorprone.fixes.Fix;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.AnnotationTree;
@@ -54,7 +56,7 @@ public final class SimplifyTimeBasedAnnotationCheck extends BugChecker
     }
 
     return trySimplification(annotationTree, arguments)
-        .map(SimplifyTimeBasedAnnotationCheck::describeMatch)
+        .map(fix -> describeMatch(annotationTree, fix))
         .orElse(Description.NO_MATCH);
   }
 
@@ -73,8 +75,36 @@ public final class SimplifyTimeBasedAnnotationCheck extends BugChecker
     Number value = getValue(annotation, indexedArguments);
     TimeUnit timeUnit = getTimeUnit(annotation, indexedArguments);
 
-    Optional<Simplification> simplification = simplifyUnit(value, timeUnit);
-    return Optional.empty();
+    return simplifyUnit(value, timeUnit)
+        .map(
+            simplification ->
+                SuggestedFixes.updateAnnotationArgumentValues(
+                        annotation,
+                        getTimeUnitArgumentName(annotation),
+                        ImmutableList.of(simplification.getUnit().name()))
+                    .merge(
+                        valueFix(
+                            annotation,
+                            getValueArgumentName(annotation),
+                            simplification.getValue()))
+                    .addStaticImport(
+                        TimeUnit.class.getName() + '.' + simplification.getUnit().name())
+                    .build());
+  }
+
+  private static SuggestedFix valueFix(
+      AnnotationTree annotation, String parameterName, Number value) {
+    if (!parameterName.equals("value")) {
+      return SuggestedFixes.updateAnnotationArgumentValues(
+              annotation, getValueArgumentName(annotation), ImmutableList.of(value.toString()))
+          .build();
+    }
+
+    // XXX: Fix this. Maybe synthesize the entire annotation in case of "value" and arguments.size()
+    // == 1.
+    return SuggestedFix.builder()
+        .replace(annotation.getArguments().get(0), parameterName + " = " + value)
+        .build();
   }
 
   private static Number getValue(
@@ -120,10 +150,6 @@ public final class SimplifyTimeBasedAnnotationCheck extends BugChecker
     return (VarSymbol) argumentSymbol.getDefaultValue().getValue();
   }
 
-  private static Description describeMatch(Fix fix) {
-    return Description.NO_MATCH;
-  }
-
   private static AnnotationAttributeMatcher getMatcher() {
     ImmutableList<String> toMatch =
         ANNOTATION_ATTRIBUTES.entries().stream()
diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheckTest.java
index bc41691b..4be83869 100644
--- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheckTest.java
+++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SimplifyTimeBasedAnnotationCheckTest.java
@@ -20,6 +20,7 @@ public final class SimplifyTimeBasedAnnotationCheckTest {
             "import org.junit.jupiter.api.Timeout;",
             "",
             "interface A {",
+            "  @Timeout(6) A noSimplification();",
             "  // BUG: Diagnostic contains:",
             "  @Timeout(60) A simple();",
             "  // BUG: Diagnostic contains:",
@@ -40,10 +41,12 @@ public final class SimplifyTimeBasedAnnotationCheckTest {
             "}")
         .addOutputLines(
             "out/A.java",
+            "import static java.util.concurrent.TimeUnit.MINUTES;",
+            "",
             "import org.junit.jupiter.api.Timeout;",
             "",
             "interface A {",
-            "  @Timeout(1, unit = TimeUnit.MINUTES) A simple();",
+            "  @Timeout(value = 1, unit = MINUTES) A simple();",
             "}")
         .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }