Support AllSuggestionsAsWarnings and add a suggestion

PicnicSupermarket · Sep 29, 2022 · e27f947 · e27f947
1 parent b5fd131
commit e27f947
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 33 deletions.
diff --git a/...c/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java b/...c/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java
@@ -22,6 +22,10 @@
 import com.sun.tools.javac.util.Context;
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.Optional;
 import java.util.function.Function;
@@ -123,11 +127,25 @@ private static <A extends Annotation> Optional<A> getAnnotationValue(
   }
 
   private static SeverityLevel overrideSeverity(SeverityLevel severity, Context context) {
-    // XXX: Respect `-XepAllSuggestionsAsWarnings` when using the Picnic Error Prone Fork!
-    SeverityLevel minSeverity = SUGGESTION;
-    SeverityLevel maxSeverity =
-        context.get(ErrorProneOptions.class).isDropErrorsToWarnings() ? WARNING : ERROR;
+    ErrorProneOptions errorProneOptions = context.get(ErrorProneOptions.class);
+    SeverityLevel minSeverity = allSuggestionsAsWarnings(errorProneOptions) ? WARNING : SUGGESTION;
+    SeverityLevel maxSeverity = errorProneOptions.isDropErrorsToWarnings() ? WARNING : ERROR;
 
     return Comparators.max(Comparators.min(severity, minSeverity), maxSeverity);
   }
+
+  private static boolean allSuggestionsAsWarnings(ErrorProneOptions errorProneOptions) {
+    try {
+      Optional<Method> isSuggestionsAsWarningsMethod =
+          Arrays.stream(errorProneOptions.getClass().getDeclaredMethods())
+              .filter(m -> Modifier.isPublic(m.getModifiers()))
+              .filter(m -> m.getName().equals("isSuggestionsAsWarnings"))
+              .findFirst();
+      return isSuggestionsAsWarningsMethod.isPresent()
+          && Boolean.TRUE.equals(
+              isSuggestionsAsWarningsMethod.orElseThrow().invoke(errorProneOptions));
+    } catch (IllegalAccessException | InvocationTargetException e) {
+      return false;
+    }
+  }
 }
diff --git a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java
@@ -11,6 +11,8 @@
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
 import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
 import com.google.errorprone.CompilationTestHelper;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
@@ -71,36 +73,55 @@ void identification() {
         .doTest();
   }
 
-  // XXX: Add test cases for `-XepAllSuggestionsAsWarnings`, conditional on the tests running
-  // against the Picnic Error Prone fork.
   private static Stream<Arguments> reportedSeverityTestCases() {
     /* { arguments, expectedSeverities } */
-    return Stream.of(
-        arguments(ImmutableList.of(), ImmutableList.of("Note", "warning", "error", "Note")),
-        arguments(ImmutableList.of("-Xep:Refaster:OFF"), ImmutableList.of()),
-        arguments(
-            ImmutableList.of("-Xep:Refaster:DEFAULT"),
-            ImmutableList.of("Note", "warning", "error", "Note")),
-        arguments(
-            ImmutableList.of("-Xep:Refaster:WARN"),
-            ImmutableList.of("warning", "warning", "warning", "warning")),
-        arguments(
-            ImmutableList.of("-Xep:Refaster:ERROR"),
-            ImmutableList.of("error", "error", "error", "error")),
-        arguments(
-            ImmutableList.of("-XepAllErrorsAsWarnings"),
-            ImmutableList.of("Note", "warning", "warning", "Note")),
-        arguments(
-            ImmutableList.of("-Xep:Refaster:OFF", "-XepAllErrorsAsWarnings"), ImmutableList.of()),
-        arguments(
-            ImmutableList.of("-Xep:Refaster:DEFAULT", "-XepAllErrorsAsWarnings"),
-            ImmutableList.of("Note", "warning", "warning", "Note")),
-        arguments(
-            ImmutableList.of("-Xep:Refaster:WARN", "-XepAllErrorsAsWarnings"),
-            ImmutableList.of("warning", "warning", "warning", "warning")),
-        arguments(
-            ImmutableList.of("-Xep:Refaster:ERROR", "-XepAllErrorsAsWarnings"),
-            ImmutableList.of("warning", "warning", "warning", "warning")));
+
+    Stream<Arguments> forkTestCases =
+        isBuiltWithErrorProneFork()
+            ? Stream.of(
+                arguments(
+                    ImmutableList.of("-Xep:Refaster:OFF", "-XepAllSuggestionsAsWarnings"),
+                    ImmutableList.of()),
+                arguments(
+                    ImmutableList.of("-Xep:Refaster:DEFAULT", "-XepAllSuggestionsAsWarnings"),
+                    ImmutableList.of("warning", "warning", "error", "warning")),
+                arguments(
+                    ImmutableList.of("-Xep:Refaster:WARN", "-XepAllSuggestionsAsWarnings"),
+                    ImmutableList.of("warning", "warning", "warning", "warning")),
+                arguments(
+                    ImmutableList.of("-Xep:Refaster:ERROR", "-XepAllSuggestionsAsWarnings"),
+                    ImmutableList.of("error", "error", "error", "error")))
+            : Stream.empty();
+
+    return Stream.concat(
+        Stream.of(
+            arguments(ImmutableList.of(), ImmutableList.of("Note", "warning", "error", "Note")),
+            arguments(ImmutableList.of("-Xep:Refaster:OFF"), ImmutableList.of()),
+            arguments(
+                ImmutableList.of("-Xep:Refaster:DEFAULT"),
+                ImmutableList.of("Note", "warning", "error", "Note")),
+            arguments(
+                ImmutableList.of("-Xep:Refaster:WARN"),
+                ImmutableList.of("warning", "warning", "warning", "warning")),
+            arguments(
+                ImmutableList.of("-Xep:Refaster:ERROR"),
+                ImmutableList.of("error", "error", "error", "error")),
+            arguments(
+                ImmutableList.of("-XepAllErrorsAsWarnings"),
+                ImmutableList.of("Note", "warning", "warning", "Note")),
+            arguments(
+                ImmutableList.of("-Xep:Refaster:OFF", "-XepAllErrorsAsWarnings"),
+                ImmutableList.of()),
+            arguments(
+                ImmutableList.of("-Xep:Refaster:DEFAULT", "-XepAllErrorsAsWarnings"),
+                ImmutableList.of("Note", "warning", "warning", "Note")),
+            arguments(
+                ImmutableList.of("-Xep:Refaster:WARN", "-XepAllErrorsAsWarnings"),
+                ImmutableList.of("warning", "warning", "warning", "warning")),
+            arguments(
+                ImmutableList.of("-Xep:Refaster:ERROR", "-XepAllErrorsAsWarnings"),
+                ImmutableList.of("warning", "warning", "warning", "warning"))),
+        forkTestCases);
   }
 
   /**
@@ -203,4 +224,20 @@ void restrictedReplacement() {
             "}")
         .doTest(TestMode.TEXT_MATCH);
   }
+
+  private static boolean isBuiltWithErrorProneFork() {
+    Class<?> clazz;
+    try {
+      clazz =
+          Class.forName(
+              "com.google.errorprone.ErrorProneOptions",
+              /* initialize= */ false,
+              Thread.currentThread().getContextClassLoader());
+    } catch (ClassNotFoundException e) {
+      return false;
+    }
+    return Arrays.stream(clazz.getDeclaredMethods())
+        .filter(m -> Modifier.isPublic(m.getModifiers()))
+        .anyMatch(m -> m.getName().equals("isSuggestionsAsWarnings"));
+  }
 }
diff --git a/...upport/src/main/java/tech/picnic/errorprone/refaster/test/RefasterTemplateCollection.java b/...upport/src/main/java/tech/picnic/errorprone/refaster/test/RefasterTemplateCollection.java
@@ -205,7 +205,7 @@ private void reportViolations(
   private static String extractRefasterTemplateName(Description description) {
     String message = description.getRawMessage();
     int index = message.indexOf(':');
-    checkState(index >= 0, "String '%s' does not contain character '%s'", message, ':');
+    checkState(index >= 0, "String '%s' does not contain character ':'", message);
     return getSubstringAfterFinalDelimiter('.', message.substring(0, index));
   }