Upgrade Error Prone 2.19.1 -> 2.20.0 (#685)

Summary of key changes:
- The `MissingRefasterAnnotation` check was contributed to Error Prone,
  and so is deleted here (multiple checks with the same name are not
  supported).
- Similarly, Error Prone now supports the `-XepAllSuggestionsAsWarnings`
  flag out of the box. So the `ErrorProneFork` class is deleted, as it
  has currently no further use.

While there, include a tweak to `run-mutation-tests.sh`.

Fixes #686.

See:
- https://github.com/google/error-prone/releases/tag/v2.20.0
- https://github.com/google/error-prone/compare/v2.19.1...v2.20.0
- https://github.com/PicnicSupermarket/error-prone/compare/v2.19.1-picnic-1...v2.20.0-picnic-1

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MissingRefasterAnnotation.java

@@ -1,59 +0,0 @@
-package tech.picnic.errorprone.bugpatterns;
-
-import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
-import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
-import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
-import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
-import static com.google.errorprone.matchers.Matchers.annotations;
-import static com.google.errorprone.matchers.Matchers.anyOf;
-import static com.google.errorprone.matchers.Matchers.isType;
-import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
-
-import com.google.auto.service.AutoService;
-import com.google.errorprone.BugPattern;
-import com.google.errorprone.VisitorState;
-import com.google.errorprone.bugpatterns.BugChecker;
-import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
-import com.google.errorprone.matchers.Description;
-import com.google.errorprone.matchers.MultiMatcher;
-import com.google.errorprone.util.ASTHelpers;
-import com.sun.source.tree.AnnotationTree;
-import com.sun.source.tree.ClassTree;
-import com.sun.source.tree.MethodTree;
-import com.sun.source.tree.Tree;
-
-/** A {@link BugChecker} that flags likely missing Refaster annotations. */
-@AutoService(BugChecker.class)
-@BugPattern(
-    summary = "The Refaster rule contains a method without any Refaster annotations",
-    link = BUG_PATTERNS_BASE_URL + "MissingRefasterAnnotation",
-    linkType = CUSTOM,
-    severity = WARNING,
-    tags = LIKELY_ERROR)
-public final class MissingRefasterAnnotation extends BugChecker implements ClassTreeMatcher {
-  private static final long serialVersionUID = 1L;
-  private static final MultiMatcher<Tree, AnnotationTree> REFASTER_ANNOTATION =
-      annotations(
-          AT_LEAST_ONE,
-          anyOf(
-              isType("com.google.errorprone.refaster.annotation.Placeholder"),
-              isType("com.google.errorprone.refaster.annotation.BeforeTemplate"),
-              isType("com.google.errorprone.refaster.annotation.AfterTemplate")));
-
-  /** Instantiates a new {@link MissingRefasterAnnotation} instance. */
-  public MissingRefasterAnnotation() {}
-
-  @Override
-  public Description matchClass(ClassTree tree, VisitorState state) {
-    long methodTypes =
-        tree.getMembers().stream()
-            .filter(member -> member.getKind() == Tree.Kind.METHOD)
-            .map(MethodTree.class::cast)
-            .filter(method -> !ASTHelpers.isGeneratedConstructor(method))
-            .map(method -> REFASTER_ANNOTATION.matches(method, state))
-            .distinct()
-            .count();
-
-    return methodTypes < 2 ? Description.NO_MATCH : buildDescription(tree).build();
-  }
-}

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java

@@ -164,6 +164,8 @@ final class CollectionRules {
   }
 
   /** Prefer {@link ArrayList#ArrayList(Collection)} over the Guava alternative. */
+  @SuppressWarnings(
+      "NonApiType" /* Matching against `List` would unnecessarily constrain the rule. */)
   static final class NewArrayListFromCollection<T> {
     @BeforeTemplate
     ArrayList<T> before(Collection<T> collection) {

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MissingRefasterAnnotationTest.java

@@ -1,89 +0,0 @@
-package tech.picnic.errorprone.bugpatterns;
-
-import static com.google.common.base.Predicates.containsPattern;
-
-import com.google.errorprone.CompilationTestHelper;
-import org.junit.jupiter.api.Test;
-
-final class MissingRefasterAnnotationTest {
-  @Test
-  void identification() {
-    CompilationTestHelper.newInstance(MissingRefasterAnnotation.class, getClass())
-        .expectErrorMessage(
-            "X",
-            containsPattern("The Refaster rule contains a method without any Refaster annotations"))
-        .addSourceLines(
-            "A.java",
-            "import com.google.errorprone.refaster.annotation.AfterTemplate;",
-            "import com.google.errorprone.refaster.annotation.AlsoNegation;",
-            "import com.google.errorprone.refaster.annotation.BeforeTemplate;",
-            "import java.util.Map;",
-            "",
-            "class A {",
-            "  // BUG: Diagnostic matches: X",
-            "  static final class MethodLacksBeforeTemplateAnnotation {",
-            "    @BeforeTemplate",
-            "    boolean before1(String string) {",
-            "      return string.equals(\"\");",
-            "    }",
-            "",
-            "    // @BeforeTemplate is missing",
-            "    boolean before2(String string) {",
-            "      return string.length() == 0;",
-            "    }",
-            "",
-            "    @AfterTemplate",
-            "    @AlsoNegation",
-            "    boolean after(String string) {",
-            "      return string.isEmpty();",
-            "    }",
-            "  }",
-            "",
-            "  // BUG: Diagnostic matches: X",
-            "  static final class MethodLacksAfterTemplateAnnotation {",
-            "    @BeforeTemplate",
-            "    boolean before(String string) {",
-            "      return string.equals(\"\");",
-            "    }",
-            "",
-            "    // @AfterTemplate is missing",
-            "    boolean after(String string) {",
-            "      return string.isEmpty();",
-            "    }",
-            "  }",
-            "",
-            "  // BUG: Diagnostic matches: X",
-            "  abstract class MethodLacksPlaceholderAnnotation<K, V> {",
-            "    // @Placeholder is missing",
-            "    abstract V function(K key);",
-            "",
-            "    @BeforeTemplate",
-            "    void before(Map<K, V> map, K key) {",
-            "      if (!map.containsKey(key)) {",
-            "        map.put(key, function(key));",
-            "      }",
-            "    }",
-            "",
-            "    @AfterTemplate",
-            "    void after(Map<K, V> map, K key) {",
-            "      map.computeIfAbsent(key, k -> function(k));",
-            "    }",
-            "  }",
-            "",
-            "  static final class ValidRefasterRule {",
-            "    @BeforeTemplate",
-            "    void unusedPureFunctionCall(Object o) {",
-            "      o.toString();",
-            "    }",
-            "  }",
-            "",
-            "  static final class NotARefasterRule {",
-            "    @Override",
-            "    public String toString() {",
-            "      return \"This is not a Refaster rule\";",
-            "    }",
-            "  }",
-            "}")
-        .doTest();
-  }
-}

pom.xml

@@ -201,7 +201,7 @@
         <version.auto-value>1.10.1</version.auto-value>
         <version.error-prone>${version.error-prone-orig}</version.error-prone>
         <version.error-prone-fork>v${version.error-prone-orig}-picnic-1</version.error-prone-fork>
-        <version.error-prone-orig>2.19.1</version.error-prone-orig>
+        <version.error-prone-orig>2.20.0</version.error-prone-orig>
         <version.error-prone-slf4j>0.1.18</version.error-prone-slf4j>
         <version.guava-beta-checker>1.0</version.guava-beta-checker>
         <version.jdk>11</version.jdk>
@@ -972,7 +972,7 @@
                             <banDuplicateClasses>
                                 <dependencies>
                                     <dependency>
-                                        <groupId>org.checkerframework</groupId>
+                                        <groupId>io.github.eisop</groupId>
                                         <artifactId>dataflow-errorprone</artifactId>
                                         <!-- This package is contained in
                                         Checker Framework's `checker-qual` and
@@ -1720,6 +1720,9 @@
                                     -Xep:Java7ApiChecker:OFF
                                     <!-- We don't target JDK 8. -->
                                     -Xep:Java8ApiChecker:OFF
+                                    <!-- XXX: Triggers an IOOBE. See
+                                    https://github.com/google/error-prone/pull/3976. -->
+                                    -Xep:MemberName:OFF
                                     <!-- We don't target Android. -->
                                     -Xep:StaticOrDefaultInterfaceMethod:OFF
                                     <!-- We generally discourage `var` use. -->

refaster-runner/pom.xml

@@ -45,6 +45,7 @@
         <dependency>
             <groupId>${project.groupId}</groupId>
             <artifactId>refaster-support</artifactId>
+            <scope>runtime</scope>
         </dependency>
         <dependency>
             <groupId>com.google.auto.service</groupId>

refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java

@@ -22,7 +22,6 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
-import tech.picnic.errorprone.refaster.ErrorProneFork;
 
 final class RefasterTest {
   private final CompilationTestHelper compilationHelper =
@@ -80,74 +79,66 @@ final class RefasterTest {
     SeverityLevel defaultSeverity = BugCheckerInfo.create(Refaster.class).defaultSeverity();
 
     /* { arguments, expectedSeverities } */
-    return Stream.concat(
-        Stream.of(
-            arguments(
-                ImmutableList.of(), ImmutableList.of(defaultSeverity, WARNING, ERROR, SUGGESTION)),
-            arguments(ImmutableList.of("-Xep:Refaster:OFF"), ImmutableList.of()),
-            arguments(
-                ImmutableList.of("-Xep:Refaster:DEFAULT"),
-                ImmutableList.of(defaultSeverity, WARNING, ERROR, SUGGESTION)),
-            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(defaultSeverity, WARNING, WARNING, SUGGESTION)),
-            arguments(
-                ImmutableList.of("-Xep:Refaster:OFF", "-XepAllErrorsAsWarnings"),
-                ImmutableList.of()),
-            arguments(
-                ImmutableList.of("-Xep:Refaster:DEFAULT", "-XepAllErrorsAsWarnings"),
-                ImmutableList.of(defaultSeverity, WARNING, WARNING, SUGGESTION)),
-            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))),
-        ErrorProneFork.isErrorProneForkAvailable()
-            ? 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)),
-                arguments(
-                    ImmutableList.of(
-                        "-Xep:Refaster:OFF",
-                        "-XepAllErrorsAsWarnings",
-                        "-XepAllSuggestionsAsWarnings"),
-                    ImmutableList.of()),
-                arguments(
-                    ImmutableList.of(
-                        "-Xep:Refaster:DEFAULT",
-                        "-XepAllErrorsAsWarnings",
-                        "-XepAllSuggestionsAsWarnings"),
-                    ImmutableList.of(WARNING, WARNING, WARNING, WARNING)),
-                arguments(
-                    ImmutableList.of(
-                        "-Xep:Refaster:WARN",
-                        "-XepAllErrorsAsWarnings",
-                        "-XepAllSuggestionsAsWarnings"),
-                    ImmutableList.of(WARNING, WARNING, WARNING, WARNING)),
-                arguments(
-                    ImmutableList.of(
-                        "-Xep:Refaster:ERROR",
-                        "-XepAllErrorsAsWarnings",
-                        "-XepAllSuggestionsAsWarnings"),
-                    ImmutableList.of(WARNING, WARNING, WARNING, WARNING)))
-            : Stream.empty());
+    return Stream.of(
+        arguments(
+            ImmutableList.of(), ImmutableList.of(defaultSeverity, WARNING, ERROR, SUGGESTION)),
+        arguments(ImmutableList.of("-Xep:Refaster:OFF"), ImmutableList.of()),
+        arguments(
+            ImmutableList.of("-Xep:Refaster:DEFAULT"),
+            ImmutableList.of(defaultSeverity, WARNING, ERROR, SUGGESTION)),
+        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(defaultSeverity, WARNING, WARNING, SUGGESTION)),
+        arguments(
+            ImmutableList.of("-Xep:Refaster:OFF", "-XepAllErrorsAsWarnings"), ImmutableList.of()),
+        arguments(
+            ImmutableList.of("-Xep:Refaster:DEFAULT", "-XepAllErrorsAsWarnings"),
+            ImmutableList.of(defaultSeverity, WARNING, WARNING, SUGGESTION)),
+        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)),
+        arguments(
+            ImmutableList.of("-XepAllSuggestionsAsWarnings"),
+            ImmutableList.of(WARNING, WARNING, ERROR, WARNING)),
+        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)),
+        arguments(
+            ImmutableList.of("-XepAllErrorsAsWarnings", "-XepAllSuggestionsAsWarnings"),
+            ImmutableList.of(WARNING, WARNING, WARNING, WARNING)),
+        arguments(
+            ImmutableList.of(
+                "-Xep:Refaster:OFF", "-XepAllErrorsAsWarnings", "-XepAllSuggestionsAsWarnings"),
+            ImmutableList.of()),
+        arguments(
+            ImmutableList.of(
+                "-Xep:Refaster:DEFAULT", "-XepAllErrorsAsWarnings", "-XepAllSuggestionsAsWarnings"),
+            ImmutableList.of(WARNING, WARNING, WARNING, WARNING)),
+        arguments(
+            ImmutableList.of(
+                "-Xep:Refaster:WARN", "-XepAllErrorsAsWarnings", "-XepAllSuggestionsAsWarnings"),
+            ImmutableList.of(WARNING, WARNING, WARNING, WARNING)),
+        arguments(
+            ImmutableList.of(
+                "-Xep:Refaster:ERROR", "-XepAllErrorsAsWarnings", "-XepAllSuggestionsAsWarnings"),
+            ImmutableList.of(WARNING, WARNING, WARNING, WARNING)));
   }
 
   /**

refaster-support/src/main/java/tech/picnic/errorprone/refaster/AnnotatedCompositeCodeTransformer.java

@@ -148,8 +148,7 @@ public abstract class AnnotatedCompositeCodeTransformer implements CodeTransform
 
   private static SeverityLevel overrideSeverity(SeverityLevel severity, Context context) {
     ErrorProneOptions options = context.get(ErrorProneOptions.class);
-    SeverityLevel minSeverity =
-        ErrorProneFork.isSuggestionsAsWarningsEnabled(options) ? WARNING : SUGGESTION;
+    SeverityLevel minSeverity = options.isSuggestionsAsWarnings() ? WARNING : SUGGESTION;
     SeverityLevel maxSeverity = options.isDropErrorsToWarnings() ? WARNING : ERROR;
 
     return Comparators.max(Comparators.min(severity, minSeverity), maxSeverity);

refaster-support/src/main/java/tech/picnic/errorprone/refaster/ErrorProneFork.java

@@ -1,56 +0,0 @@
-package tech.picnic.errorprone.refaster;
-
-import com.google.errorprone.ErrorProneOptions;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
-import java.util.Arrays;
-import java.util.Optional;
-
-/**
- * Utility class that enables the runtime to determine whether Picnic's fork of Error Prone is on
- * the classpath.
- *
- * @see <a href="https://github.com/PicnicSupermarket/error-prone">Picnic's Error Prone fork</a>
- */
-public final class ErrorProneFork {
-  private static final Optional<Method> ERROR_PRONE_OPTIONS_IS_SUGGESTIONS_AS_WARNINGS_METHOD =
-      Arrays.stream(ErrorProneOptions.class.getDeclaredMethods())
-          .filter(m -> Modifier.isPublic(m.getModifiers()))
-          .filter(m -> "isSuggestionsAsWarnings".equals(m.getName()))
-          .findFirst();
-
-  private ErrorProneFork() {}
-
-  /**
-   * Tells whether Picnic's fork of Error Prone is available.
-   *
-   * @return {@code true} iff classpath introspection indicates the presence of Error Prone
-   *     modifications that are assumed to be present only in Picnic's fork.
-   */
-  public static boolean isErrorProneForkAvailable() {
-    return ERROR_PRONE_OPTIONS_IS_SUGGESTIONS_AS_WARNINGS_METHOD.isPresent();
-  }
-
-  /**
-   * Tells whether the custom {@code -XepAllSuggestionsAsWarnings} flag is set.
-   *
-   * @param options The currently active Error Prone options.
-   * @return {@code true} iff {@link #isErrorProneForkAvailable() the Error Prone fork is available}
-   *     and the aforementioned flag is set.
-   * @see <a href="https://github.com/google/error-prone/pull/3301">google/error-prone#3301</a>
-   */
-  public static boolean isSuggestionsAsWarningsEnabled(ErrorProneOptions options) {
-    return ERROR_PRONE_OPTIONS_IS_SUGGESTIONS_AS_WARNINGS_METHOD
-        .filter(m -> Boolean.TRUE.equals(invoke(m, options)))
-        .isPresent();
-  }
-
-  private static Object invoke(Method method, Object obj, Object... args) {
-    try {
-      return method.invoke(obj, args);
-    } catch (IllegalAccessException | InvocationTargetException e) {
-      throw new IllegalStateException(String.format("Failed to invoke method '%s'", method), e);
-    }
-  }
-}

refaster-support/src/test/java/tech/picnic/errorprone/refaster/ErrorProneForkTest.java

@@ -1,67 +0,0 @@
-package tech.picnic.errorprone.refaster;
-
-import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assumptions.assumeTrue;
-
-import com.google.errorprone.BugPattern;
-import com.google.errorprone.CompilationTestHelper;
-import com.google.errorprone.ErrorProneOptions;
-import com.google.errorprone.VisitorState;
-import com.google.errorprone.bugpatterns.BugChecker;
-import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
-import com.google.errorprone.matchers.Description;
-import com.sun.source.tree.ClassTree;
-import org.junit.jupiter.api.Test;
-
-final class ErrorProneForkTest {
-  @Test
-  void isErrorProneForkAvailable() {
-    assertThat(ErrorProneFork.isErrorProneForkAvailable())
-        .isEqualTo(Boolean.TRUE.toString().equals(System.getProperty("error-prone-fork-in-use")));
-  }
-
-  @Test
-  void isSuggestionsAsWarningsEnabledWithoutFlag() {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
-        .addSourceLines(
-            "A.java",
-            "// BUG: Diagnostic contains: Suggestions as warnings enabled: false",
-            "class A {}")
-        .doTest();
-  }
-
-  @Test
-  void isSuggestionsAsWarningsEnabledWithFlag() {
-    assumeTrue(
-        ErrorProneFork.isErrorProneForkAvailable(),
-        "Picnic's Error Prone fork is not on the classpath");
-
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
-        .setArgs("-XepAllSuggestionsAsWarnings")
-        .addSourceLines(
-            "A.java",
-            "// BUG: Diagnostic contains: Suggestions as warnings enabled: true",
-            "class A {}")
-        .doTest();
-  }
-
-  /**
-   * A {@link BugChecker} that reports the result of {@link
-   * ErrorProneFork#isSuggestionsAsWarningsEnabled(ErrorProneOptions)}.
-   */
-  @BugPattern(summary = "Flags classes with a custom error message", severity = ERROR)
-  public static final class TestChecker extends BugChecker implements ClassTreeMatcher {
-    private static final long serialVersionUID = 1L;
-
-    @Override
-    public Description matchClass(ClassTree tree, VisitorState state) {
-      return buildDescription(tree)
-          .setMessage(
-              String.format(
-                  "Suggestions as warnings enabled: %s",
-                  ErrorProneFork.isSuggestionsAsWarningsEnabled(state.errorProneOptions())))
-          .build();
-    }
-  }
-}

run-mutation-tests.sh

@@ -1,8 +1,8 @@
 #!/usr/bin/env bash
 
-# Executes Pitest to determine the code base' mutation test coverage. The set
-# of tests executed can optionally be restricted by name. The results are found
-# in each Maven module's `target/pit-reports` directory.
+# Executes Pitest to determine the code's mutation test coverage. The set of
+# tests executed can optionally be restricted by name. The results are found in
+# each Maven module's `target/pit-reports` directory.
 
 set -e -u -o pipefail
 
@@ -11,7 +11,7 @@ if [ "${#}" -gt 1 ]; then
   exit 1
 fi
 
-targetTests=${1:-*}
+targetTests="${1:-*}"
 
 mvn clean test pitest:mutationCoverage \
   -DargLine.xmx=2048m \