Fix removeUnused mode not loading plugin checks and alt names

gradle-suppressible-error-prone/src/test/groovy/com/palantir/gradle/suppressibleerrorprone/SuppressibleErrorPronePluginIntegrationTest.groovy

@@ -76,14 +76,16 @@ class SuppressibleErrorPronePluginIntegrationTest extends ConfigurationCacheSpec
                 // This should guarantee that we're using the same version, both of which should be in maven local
                 //   and be the current version
                 errorprone 'com.palantir.suppressible-error-prone:test-error-prone-checks:' + project.findProperty("suppressibleErrorProneVersion")
+                errorprone "com.uber.nullaway:nullaway:0.12.12"
+                // NullAway is turned off by default, but is used in some tests
             }
 
             suppressibleErrorProne {
                 configureEachErrorProneOptions {
                     // These interfere with some tests, so disable them
                     // TODO(callumr): Rewrite the tests to use custom testing error-prones rather than built in checks
                     //                to make upgrading error-prone easier.
-                    disable('Varifier', 'ReturnValueIgnored', 'UnusedVariable', 'IdentifierName', 'UnusedMethod')
+                    disable('Varifier', 'ReturnValueIgnored', 'UnusedVariable', 'IdentifierName', 'UnusedMethod', 'NullAway')
                     ignoreUnknownCheckNames = true
                 }
             }
@@ -1596,7 +1598,7 @@ class SuppressibleErrorPronePluginIntegrationTest extends ConfigurationCacheSpec
         writeJavaSourceFileToSourceSets '''
             package app;
 
-            @SuppressWarnings({"ArrayToString", "UnnecessaryFinal", "InlineTrivialConstant", "NotAnErrorProne", "checkstyle:Bla",})
+            @SuppressWarnings({"ArrayToString", "UnnecessaryFinal", "InlineTrivialConstant", "NotAnErrorProne", "ShouldBePrivate", "checkstyle:Bla",})
             public final class App {
                 private static final String EMPTY_STRING = "";
 
@@ -1607,7 +1609,7 @@ class SuppressibleErrorPronePluginIntegrationTest extends ConfigurationCacheSpec
         '''.stripIndent(true)
 
         when:
-        runTasksSuccessfully('compileAllErrorProne', '-PerrorProneRemoveUnused=ArrayToString')
+        runTasksSuccessfully('compileAllErrorProne', '-PerrorProneRemoveUnused')
 
         then:
         // language=Java
@@ -1625,6 +1627,48 @@ class SuppressibleErrorPronePluginIntegrationTest extends ConfigurationCacheSpec
         '''.stripIndent(true)
     }
 
+    def 'errorProneRemoveUnused understands alt-names'() {
+        // language=Java
+        writeJavaSourceFileToSourceSets '''
+            package app;
+
+            public final class App {
+                @SuppressWarnings("ShouldBePrivate")
+                void fixme(String s) {}
+
+                @SuppressWarnings("MustBePrivate")
+                void fixme(Integer s) {}
+
+                @SuppressWarnings("ShouldBePrivate")
+                private void fixme(Float s) {}
+
+                @SuppressWarnings("MustBePrivate")
+                private void fixme(Character s) {}
+            }
+        '''.stripIndent(true)
+
+        when:
+        runTasksSuccessfully('compileAllErrorProne', '-PerrorProneRemoveUnused')
+
+        then:
+        // language=Java
+        javaSourceIsSyntacticallyEqualTo '''
+            package app;
+
+            public final class App {
+                @SuppressWarnings("ShouldBePrivate")
+                void fixme(String s) {}
+
+                @SuppressWarnings("MustBePrivate")
+                void fixme(Integer s) {}
+
+                private void fixme(Float s) {}
+
+                private void fixme(Character s) {}
+            }
+        '''.stripIndent(true)
+    }
+
     def 'errorProneRemoveUnused does not apply fixes'() {
         given:
         // language=Java

suppressible-error-prone/build.gradle

@@ -21,7 +21,9 @@ moduleJvmArgs {
             'jdk.compiler/com.sun.tools.javac.comp',
             'jdk.compiler/com.sun.tools.javac.file',
             'jdk.compiler/com.sun.tools.javac.main',
+            'jdk.compiler/com.sun.tools.javac.model',
             'jdk.compiler/com.sun.tools.javac.parser',
+            'jdk.compiler/com.sun.tools.javac.processing',
             'jdk.compiler/com.sun.tools.javac.tree',
             'jdk.compiler/com.sun.tools.javac.util')
 }

suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AllErrorprones.java

@@ -16,33 +16,63 @@
 
 package com.palantir.suppressibleerrorprone;
 
-import com.google.common.base.Suppliers;
 import com.google.errorprone.BugCheckerInfo;
+import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.scanner.BuiltInCheckerSuppliers;
+import com.google.errorprone.suppliers.Supplier;
+import com.sun.tools.javac.processing.JavacProcessingEnvironment;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import java.util.ServiceLoader;
+import java.util.ServiceLoader.Provider;
 import java.util.Set;
-import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import one.util.streamex.EntryStream;
 import one.util.streamex.StreamEx;
 
 public final class AllErrorprones {
-    private static Supplier<Set<String>> cachedAllBugcheckerNames = Suppliers.memoize(() -> {
-        Stream<BugChecker> pluginChecks =
-                ServiceLoader.load(BugChecker.class).stream().map(ServiceLoader.Provider::get);
-        Stream<String> pluginCheckNames =
-                StreamEx.of(pluginChecks).flatMap(bugchecker -> bugchecker.allNames().stream());
+    private static Supplier<Map<String, Set<String>>> canonicalToAllNames = VisitorState.memoize(state -> {
+        // Use the same classloader that Error Prone was loaded from to avoid classloader skew
+        // when using Error Prone plugins together with the Error Prone javac plugin.
+        JavacProcessingEnvironment processingEnvironment = JavacProcessingEnvironment.instance(state.context);
+        ClassLoader loader = processingEnvironment.getProcessorClassLoader();
+        List<BugChecker> pluginChecks = ServiceLoader.load(BugChecker.class, loader).stream()
+                .map(Provider::get)
+                .collect(Collectors.toList());
+        EntryStream<String, Set<String>> pluginCheckNames =
+                StreamEx.of(pluginChecks).mapToEntry(BugChecker::canonicalName, BugChecker::allNames);
 
         Stream<BugCheckerInfo> builtInChecks = BuiltInCheckerSuppliers.allChecks().getAllChecks().values().stream();
-        Stream<String> builtInCheckNames =
-                StreamEx.of(builtInChecks).flatMap(bugchecker -> bugchecker.allNames().stream());
+        EntryStream<String, Set<String>> builtInCheckNames =
+                StreamEx.of(builtInChecks).mapToEntry(BugCheckerInfo::canonicalName, BugCheckerInfo::allNames);
 
-        return Stream.concat(pluginCheckNames, builtInCheckNames).collect(Collectors.toSet());
+        return pluginCheckNames.append(builtInCheckNames).toMap();
     });
 
-    public static Set<String> allBugcheckerNames() {
-        return cachedAllBugcheckerNames.get();
+    private static Supplier<Set<String>> allNames =
+            VisitorState.memoize(state -> EntryStream.of(canonicalToAllNames.get(state))
+                    .flatMap(entry -> entry.getValue().stream())
+                    .collect(Collectors.toSet()));
+
+    private static Supplier<Map<String, Set<String>>> nameToPossibleCanonicalNames =
+            VisitorState.memoize(state -> EntryStream.of(canonicalToAllNames.get(state))
+                    .flatMapValues(Set::stream)
+                    .invert()
+                    .grouping(Collectors.toSet()));
+
+    public static Set<String> allBugcheckerNames(VisitorState state) {
+        return allNames.get(state);
+    }
+
+    public static Optional<Set<String>> allNames(VisitorState state, String canonicalName) {
+        return Optional.ofNullable(canonicalToAllNames.get(state).get(canonicalName));
+    }
+
+    public static Set<String> possibleCanonicalNames(VisitorState state, String name) {
+        return nameToPossibleCanonicalNames.get(state).getOrDefault(name, Set.of());
     }
 
     private AllErrorprones() {}

suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/ReportedFixCache.java

@@ -38,15 +38,6 @@ public final class ReportedFixCache {
     // garbage collected.
     private final WeakHashMap<Tree, LazySuppressionFix> cache = new WeakHashMap<>();
 
-    public static final Predicate<String> REMOVE_EVERYTHING = bc -> false;
-    public static final Predicate<String> KEEP_EVERYTHING = bc -> true;
-    public static final Predicate<String> NOT_AN_ERRORPRONE = suppression -> {
-        String checkerName = suppression.startsWith(CommonConstants.AUTOMATICALLY_ADDED_PREFIX)
-                ? suppression.substring(CommonConstants.AUTOMATICALLY_ADDED_PREFIX.length())
-                : suppression;
-        return !AllErrorprones.allBugcheckerNames().contains(checkerName);
-    };
-
     ReportedFixCache() {}
 
     /**

suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressWarningsUtils.java

@@ -142,5 +142,12 @@ public static Optional<? extends AnnotationTree> getSuppressWarnings(TreePath su
                 .findFirst();
     }
 
+    public static String stripForRollout(String suppression) {
+        if (suppression.startsWith(CommonConstants.AUTOMATICALLY_ADDED_PREFIX)) {
+            return suppression.substring(12);
+        }
+        return suppression;
+    }
+
     private SuppressWarningsUtils() {}
 }

suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressionRemover.java

@@ -40,7 +40,9 @@ public Void visitAnnotation(AnnotationTree node, Void unused) {
                     if (AnnotationUtils.isSuppressWarningsAnnotation(node)) {
                         TreePath declaration = getCurrentPath().getParentPath().getParentPath();
 
-                        reportedFixes.getOrReportNew(declaration, state, ReportedFixCache.NOT_AN_ERRORPRONE);
+                        reportedFixes.getOrReportNew(
+                                declaration, state, suppression -> !AllErrorprones.allBugcheckerNames(state)
+                                        .contains(suppression));
                     }
 
                     return super.visitAnnotation(node, unused);

suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/VisitorStateModifications.java

@@ -24,10 +24,12 @@
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.CompilationUnitTree;
 import com.sun.source.util.TreePath;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Logger;
 import java.util.stream.Stream;
+import one.util.streamex.MoreCollectors;
 
 // CHECKSTYLE:ON
 
@@ -83,13 +85,23 @@ public static Description interceptDescription(VisitorState visitorState, Descri
 
         Optional<TreePath> firstSuppressibleWhichSuppressesDescription = Stream.iterate(
                         pathToActualError, treePath -> treePath.getParentPath() != null, TreePath::getParentPath)
-                .dropWhile(path -> !suppresses(path, description))
+                .dropWhile(path -> !suppresses(path, description, visitorState))
                 .findFirst();
 
         if (firstSuppressibleWhichSuppressesDescription.isPresent() && modes.contains("RemoveUnused")) {
             // In RemoveUnused mode, removeAllSuppressionsOnErrorprones guarantees that a fix must already exist on
             // this suppressible.
-            FIXES.getExisting(firstSuppressibleWhichSuppressesDescription.get()).addSuppression(description.checkName);
+            TreePath declaration = firstSuppressibleWhichSuppressesDescription.get();
+            Set<String> allNames =
+                    AllErrorprones.allNames(visitorState, description.checkName).get();
+            // Use the existing suppression, rather than changing it to the canonical suppression
+            String existingSuppression = AnnotationUtils.annotationStringValues(
+                            SuppressWarningsUtils.getSuppressWarnings(declaration)
+                                    .get())
+                    .filter(suppression -> allNames.contains(SuppressWarningsUtils.stripForRollout(suppression)))
+                    .collect(MoreCollectors.first())
+                    .get();
+            FIXES.getExisting(declaration).addSuppression(existingSuppression);
             return Description.NO_MATCH;
         }
 
@@ -124,26 +136,31 @@ public static Description interceptDescription(VisitorState visitorState, Descri
         // this, we make sure we only make one fix per source element we put the suppression on by using a Map. This
         // way we have our own mutable Fix that we can add errors to, and only once the file has been visited by all
         // the error-prone checks it will then produce a replacement with all the checks suppressed.
-        LazySuppressionFix suppressingFix =
-                FIXES.getOrReportNew(firstSuppressible.get(), visitorState, ReportedFixCache.KEEP_EVERYTHING);
+        LazySuppressionFix suppressingFix = FIXES.getOrReportNew(firstSuppressible.get(), visitorState, bc -> true);
         suppressingFix.addSuppression(CommonConstants.AUTOMATICALLY_ADDED_PREFIX + description.checkName);
         return Description.NO_MATCH;
     }
 
-    private static boolean suppresses(TreePath declaration, Description description) {
+    private static boolean suppresses(TreePath declaration, Description description, VisitorState state) {
+        return !suppressionsOn(declaration, description, state).isEmpty();
+    }
+
+    private static List<String> suppressionsOn(TreePath declaration, Description description, VisitorState state) {
         if (!SuppressWarningsUtils.suppressibleTreePath(declaration)) {
-            return false;
+            return List.of();
         }
 
         Optional<? extends AnnotationTree> suppressWarningsMaybe =
                 SuppressWarningsUtils.getSuppressWarnings(declaration);
         if (suppressWarningsMaybe.isEmpty()) {
-            return false;
+            return List.of();
         }
 
-        String checkName = description.checkName;
         return AnnotationUtils.annotationStringValues(suppressWarningsMaybe.get())
-                .anyMatch(checkName::equals);
+                .filter(suppression -> AllErrorprones.possibleCanonicalNames(
+                                state, SuppressWarningsUtils.stripForRollout(suppression))
+                        .contains(description.checkName))
+                .toList();
     }
 
     private static Set<String> getModes(VisitorState state) {

test-error-prone-checks/src/main/java/com/palantir/gradle/suppressibleerrorprone/ShouldBePrivate.java

@@ -29,7 +29,8 @@
 @AutoService(BugChecker.class)
 @BugPattern(
         severity = BugPattern.SeverityLevel.ERROR,
-        summary = "This check is meant only for testing suppressible-error-prone functionality")
+        summary = "This check is meant only for testing suppressible-error-prone functionality",
+        altNames = "MustBePrivate")
 public final class ShouldBePrivate extends BugChecker implements BugChecker.MethodTreeMatcher {
 
     @Override

test-error-prone-checks/src/main/java/com/palantir/gradle/suppressibleerrorprone/ShouldNotReturn.java

@@ -0,0 +1,45 @@
+/*
+ * (c) Copyright 2025 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.palantir.gradle.suppressibleerrorprone;
+
+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.MethodTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.MethodTree;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+        severity = BugPattern.SeverityLevel.ERROR,
+        summary = "This check is meant only for testing suppressible-error-prone functionality")
+public final class ShouldNotReturn extends BugChecker implements MethodTreeMatcher {
+    @Override
+    public Description matchMethod(MethodTree tree, VisitorState _state) {
+        if (!(tree.getReturnType() instanceof IdentifierTree id)) {
+            return Description.NO_MATCH;
+        }
+
+        if (!id.getName().contentEquals("DontReturnMe")) {
+            return Description.NO_MATCH;
+        }
+
+        return buildDescription(id).setMessage("Don't return me").build();
+    }
+}