Introduce ErrorProneRuntimeClasspath check

documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java

@@ -67,7 +67,7 @@ private static final class BugPatternTestCollector
                 "com.google.errorprone.CompilationTestHelper",
                 "com.google.errorprone.BugCheckerRefactoringTestHelper")
             .named("newInstance")
-            .withParameters("java.lang.Class", "java.lang.Class");
+            .withParameters(Class.class.getCanonicalName(), Class.class.getCanonicalName());
     private static final Matcher<ExpressionTree> IDENTIFICATION_SOURCE_LINES =
         instanceMethod()
             .onDescendantOf("com.google.errorprone.CompilationTestHelper")

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

@@ -7,6 +7,9 @@
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
 
 import com.google.auto.service.AutoService;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
@@ -17,7 +20,11 @@
 import com.google.errorprone.matchers.Matcher;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.stream.Collector;
+import java.util.stream.Collectors;
 import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
 
 /**
@@ -38,7 +45,7 @@
 public final class CollectorMutability extends BugChecker implements MethodInvocationTreeMatcher {
   private static final long serialVersionUID = 1L;
   private static final Matcher<ExpressionTree> COLLECTOR_METHOD =
-      staticMethod().onClass("java.util.stream.Collectors");
+      staticMethod().onClass(Collectors.class.getCanonicalName());
   private static final Matcher<ExpressionTree> LIST_COLLECTOR =
       staticMethod().anyClass().named("toList");
   private static final Matcher<ExpressionTree> MAP_COLLECTOR =
@@ -59,8 +66,8 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
     if (LIST_COLLECTOR.matches(tree, state)) {
       return suggestToCollectionAlternatives(
           tree,
-          "com.google.common.collect.ImmutableList.toImmutableList",
-          "java.util.ArrayList",
+          ImmutableList.class.getCanonicalName() + ".toImmutableList",
+          ArrayList.class.getCanonicalName(),
           state);
     }
 
@@ -71,8 +78,8 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
     if (SET_COLLECTOR.matches(tree, state)) {
       return suggestToCollectionAlternatives(
           tree,
-          "com.google.common.collect.ImmutableSet.toImmutableSet",
-          "java.util.HashSet",
+          ImmutableSet.class.getCanonicalName() + ".toImmutableSet",
+          HashSet.class.getCanonicalName(),
           state);
     }
 
@@ -87,7 +94,7 @@ private Description suggestToCollectionAlternatives(
     SuggestedFix.Builder mutableFix = SuggestedFix.builder();
     String toCollectionSelect =
         SuggestedFixes.qualifyStaticImport(
-            "java.util.stream.Collectors.toCollection", mutableFix, state);
+            Collectors.class.getCanonicalName() + ".toCollection", mutableFix, state);
     String mutableCollection = SuggestedFixes.qualifyType(state, mutableFix, mutableReplacement);
 
     return buildDescription(tree)
@@ -106,12 +113,13 @@ private Description suggestToMapAlternatives(MethodInvocationTree tree, VisitorS
     }
 
     SuggestedFix.Builder mutableFix = SuggestedFix.builder();
-    String hashMap = SuggestedFixes.qualifyType(state, mutableFix, "java.util.HashMap");
+    String hashMap =
+        SuggestedFixes.qualifyType(state, mutableFix, HashMap.class.getCanonicalName());
 
     return buildDescription(tree)
         .addFix(
             replaceMethodInvocation(
-                tree, "com.google.common.collect.ImmutableMap.toImmutableMap", state))
+                tree, ImmutableMap.class.getCanonicalName() + ".toImmutableMap", state))
         .addFix(
             mutableFix
                 .postfixWith(

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

@@ -36,7 +36,9 @@ public final class EmptyMethod extends BugChecker implements MethodTreeMatcher {
   private static final Matcher<Tree> PERMITTED_ANNOTATION =
       annotations(
           AT_LEAST_ONE,
-          anyOf(isType("java.lang.Override"), isType("org.aspectj.lang.annotation.Pointcut")));
+          anyOf(
+              isType(Override.class.getCanonicalName()),
+              isType("org.aspectj.lang.annotation.Pointcut")));
 
   /** Instantiates a new {@link EmptyMethod} instance. */
   public EmptyMethod() {}

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

@@ -0,0 +1,165 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
+import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
+import static com.google.errorprone.matchers.Matchers.instanceMethod;
+import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
+
+import com.google.auto.service.AutoService;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.primitives.Primitives;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFixes;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.AnnotationTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.LiteralTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.ClassSymbol;
+import com.sun.tools.javac.util.Constants;
+import java.util.regex.Pattern;
+import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
+
+/**
+ * A {@link BugChecker} that flags literal strings in Error Prone Support code that represent the
+ * fully qualified class name of a type that's on Error Prone's classpath.
+ *
+ * <p>Deriving such strings from the associated {@link Class} instance makes for more maintainable
+ * code.
+ */
+// XXX: Consider generalizing this to a `RuntimeClasspath` check with a configurable
+// `CLASSPATH_TYPES` regex. Such a check would likely be a no-op by default, with Error Prone
+// Support's parent specifying the regex that is currently hard-coded.
+// XXX: As-is this check may suggest usage of a string literal even if the relevant type will be on
+// the runtime classpath. Review how we can further reduce false-positives.
+// XXX: Slightly "out there", but we could try to derive the subset of classpath types likely to be
+// available at runtime from the `compile`-scoped Maven dependencies in the `pom.xml` file of
+// the module being compiled.
+// XXX: Right now this check does not rewrite primitive string references such as `"int"` to
+// `int.class.getCanonicalName()`. Review whether that's worth it.
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary =
+        "Prefer `Class#getCanonicalName()` over an equivalent string literal if and only if the "
+            + "type will be on the runtime classpath",
+    link = BUG_PATTERNS_BASE_URL + "ErrorProneRuntimeClasspath",
+    linkType = CUSTOM,
+    severity = SUGGESTION,
+    tags = FRAGILE_CODE)
+public final class ErrorProneRuntimeClasspath extends BugChecker
+    implements LiteralTreeMatcher, MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<ExpressionTree> GET_CANONICAL_NAME_INVOCATION =
+      instanceMethod().onExactClass(Class.class.getCanonicalName()).named("getCanonicalName");
+  private static final ImmutableSet<String> PRIMITIVE_TYPES =
+      Primitives.allPrimitiveTypes().stream()
+          .map(Class::getCanonicalName)
+          .collect(toImmutableSet());
+
+  /**
+   * A pattern that matches fully qualified type names that are expected to be runtime dependencies
+   * of Error Prone, and that are thus presumed to be unconditionally present on a bug checker's
+   * runtime classpath.
+   */
+  private static final Pattern CLASSPATH_TYPES =
+      Pattern.compile(
+          "com\\.google\\.common\\..*|com\\.google\\.errorprone\\.([^.]+(?<!TestHelper)(\\..*)?)|java\\..*");
+
+  /** Instantiates a new {@link ErrorProneRuntimeClasspath} instance. */
+  public ErrorProneRuntimeClasspath() {}
+
+  @Override
+  public Description matchLiteral(LiteralTree tree, VisitorState state) {
+    String value = ASTHelpers.constValue(tree, String.class);
+    if (value == null
+        || !CLASSPATH_TYPES.matcher(value).matches()
+        || state.findEnclosing(AnnotationTree.class) != null) {
+      return Description.NO_MATCH;
+    }
+
+    SuggestedFix fix = trySuggestClassReference(tree, value, state);
+    if (fix.isEmpty()) {
+      return Description.NO_MATCH;
+    }
+
+    return buildDescription(tree)
+        .setMessage(
+            "This type will be on the runtime classpath; use `Class#getCanonicalName()` instead")
+        .addFix(fix)
+        .build();
+  }
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (!GET_CANONICAL_NAME_INVOCATION.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    Symbol receiver = ASTHelpers.getSymbol(ASTHelpers.getReceiver(tree));
+    if (receiver == null
+        || !(receiver.owner instanceof ClassSymbol)
+        || PRIMITIVE_TYPES.contains(receiver.owner.getQualifiedName().toString())
+        || CLASSPATH_TYPES.matcher(receiver.owner.getQualifiedName()).matches()) {
+      return Description.NO_MATCH;
+    }
+
+    /*
+     * This class reference may not be safe; suggest using a string literal instead. (Note that
+     * dropping the type reference may make the associated import statement (if any) obsolete.
+     * Dropping such imports is left to Error Prone's `RemoveUnusedImports` check.)
+     */
+    return buildDescription(tree)
+        .setMessage("This type may not be on the runtime classpath; use a string literal instead")
+        .addFix(
+            SuggestedFix.replace(
+                tree, Constants.format(receiver.owner.getQualifiedName().toString())))
+        .build();
+  }
+
+  private static SuggestedFix trySuggestClassReference(
+      LiteralTree tree, String value, VisitorState state) {
+    if (isTypeOnClasspath(value, state)) {
+      return suggestClassReference(tree, value, "", state);
+    }
+
+    int lastDot = value.lastIndexOf('.');
+    String type = value.substring(0, lastDot);
+    if (isTypeOnClasspath(type, state)) {
+      return suggestClassReference(tree, type, value.substring(lastDot), state);
+    }
+
+    return SuggestedFix.emptyFix();
+  }
+
+  private static SuggestedFix suggestClassReference(
+      LiteralTree original, String type, String suffix, VisitorState state) {
+    SuggestedFix.Builder fix = SuggestedFix.builder();
+    String identifier = SuggestedFixes.qualifyType(state, fix, type);
+    return fix.replace(
+            original,
+            identifier
+                + ".class.getCanonicalName()"
+                + (suffix.isEmpty() ? "" : (" + " + Constants.format(suffix))))
+        .build();
+  }
+
+  private static boolean isTypeOnClasspath(String type, VisitorState state) {
+    try {
+      return ThirdPartyLibrary.canIntroduceUsage(type, state);
+    } catch (
+        @SuppressWarnings("java:S1166" /* Not exceptional. */)
+        IllegalArgumentException e) {
+      return false;
+    }
+  }
+}

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

@@ -9,6 +9,7 @@
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
 
 import com.google.auto.service.AutoService;
+import com.google.common.collect.ImmutableList;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
@@ -24,6 +25,7 @@
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.util.Position;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary;
 
@@ -65,10 +67,11 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
     if (ThirdPartyLibrary.GUAVA.isIntroductionAllowed(state)) {
       description.addFix(
           suggestBlockingElementCollection(
-              tree, "com.google.common.collect.ImmutableList.toImmutableList", state));
+              tree, ImmutableList.class.getCanonicalName() + ".toImmutableList", state));
     }
     description.addFix(
-        suggestBlockingElementCollection(tree, "java.util.stream.Collectors.toList", state));
+        suggestBlockingElementCollection(
+            tree, Collectors.class.getCanonicalName() + ".toList", state));
 
     return description.build();
   }

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

@@ -13,6 +13,8 @@
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
 
 import com.google.auto.service.AutoService;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
@@ -30,6 +32,7 @@
 import com.sun.source.tree.Tree.Kind;
 import com.sun.source.util.SimpleTreeVisitor;
 import java.util.ArrayList;
+import java.util.Formatter;
 import java.util.List;
 import java.util.Optional;
 import org.jspecify.annotations.Nullable;
@@ -118,14 +121,14 @@ public final class FormatStringConcatenation extends BugChecker
   private static final Matcher<ExpressionTree> GUAVA_FORMAT_METHOD =
       anyOf(
           staticMethod()
-              .onClass("com.google.common.base.Preconditions")
+              .onClass(Preconditions.class.getCanonicalName())
               .namedAnyOf("checkArgument", "checkNotNull", "checkState"),
-          staticMethod().onClass("com.google.common.base.Verify").named("verify"));
+          staticMethod().onClass(Verify.class.getCanonicalName()).named("verify"));
   // XXX: Add `PrintWriter`, maybe others.
   private static final Matcher<ExpressionTree> JDK_FORMAT_METHOD =
       anyOf(
-          staticMethod().onClass("java.lang.String").named("format"),
-          instanceMethod().onExactClass("java.util.Formatter").named("format"));
+          staticMethod().onClass(String.class.getCanonicalName()).named("format"),
+          instanceMethod().onExactClass(Formatter.class.getCanonicalName()).named("format"));
   private static final Matcher<ExpressionTree> SLF4J_FORMAT_METHOD =
       instanceMethod()
           .onDescendantOf("org.slf4j.Logger")

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

@@ -10,6 +10,17 @@
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
 
 import com.google.auto.service.AutoService;
+import com.google.common.collect.ImmutableBiMap;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableMultiset;
+import com.google.common.collect.ImmutableRangeMap;
+import com.google.common.collect.ImmutableRangeSet;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.collect.ImmutableTable;
 import com.google.common.primitives.Primitives;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
@@ -20,6 +31,7 @@
 import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.Matchers;
 import com.google.errorprone.util.ASTHelpers;
 import com.google.errorprone.util.ASTHelpers.TargetType;
 import com.sun.source.tree.ExpressionTree;
@@ -59,21 +71,19 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca
           staticMethod().onClass(String.class.getCanonicalName()).named("valueOf"),
           staticMethod()
               .onClassAny(
-                  "com.google.common.collect.ImmutableBiMap",
-                  "com.google.common.collect.ImmutableList",
-                  "com.google.common.collect.ImmutableListMultimap",
-                  "com.google.common.collect.ImmutableMap",
-                  "com.google.common.collect.ImmutableMultimap",
-                  "com.google.common.collect.ImmutableMultiset",
-                  "com.google.common.collect.ImmutableRangeMap",
-                  "com.google.common.collect.ImmutableRangeSet",
-                  "com.google.common.collect.ImmutableSet",
-                  "com.google.common.collect.ImmutableSetMultimap",
-                  "com.google.common.collect.ImmutableTable")
+                  ImmutableBiMap.class.getCanonicalName(),
+                  ImmutableList.class.getCanonicalName(),
+                  ImmutableListMultimap.class.getCanonicalName(),
+                  ImmutableMap.class.getCanonicalName(),
+                  ImmutableMultimap.class.getCanonicalName(),
+                  ImmutableMultiset.class.getCanonicalName(),
+                  ImmutableRangeMap.class.getCanonicalName(),
+                  ImmutableRangeSet.class.getCanonicalName(),
+                  ImmutableSet.class.getCanonicalName(),
+                  ImmutableSetMultimap.class.getCanonicalName(),
+                  ImmutableTable.class.getCanonicalName())
               .named("copyOf"),
-          staticMethod()
-              .onClass("com.google.errorprone.matchers.Matchers")
-              .namedAnyOf("allOf", "anyOf"),
+          staticMethod().onClass(Matchers.class.getCanonicalName()).namedAnyOf("allOf", "anyOf"),
           staticMethod().onClass("reactor.adapter.rxjava.RxJava2Adapter"),
           staticMethod()
               .onClass("reactor.core.publisher.Flux")