Add the ability to ignore suppressions in SuppressibleTreePathScanner

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

@@ -19,9 +19,11 @@
 import com.palantir.gradle.suppressibleerrorprone.modes.Modes;
 import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions;
 import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption;
+import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.ModifiedFile;
 import com.palantir.gradle.suppressibleerrorprone.transform.ModifyErrorProneCheckApi;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 import javax.inject.Inject;
 import net.ltgt.gradle.errorprone.ErrorProneOptions;
@@ -75,7 +77,7 @@ private void applyToJavaProject(Project project) {
         if (getModes().modifyCheckApi() instanceof ModifyCheckApiOption.MustModify mustModify) {
             // When auto-suppressing, the logic will run a bytecode patched version of errorprone
             // (via an artifact transform) that intercepts every error from every check and adds a custom fix
-            setupErrorProneArtifactTransform(project, mustModify.modifyVisitorState());
+            setupErrorProneArtifactTransform(project, mustModify.modifiedFiles());
         }
 
         project.getConfigurations().named(ErrorPronePlugin.CONFIGURATION_NAME).configure(errorProneConfiguration -> {
@@ -108,7 +110,7 @@ private void applyToJavaProject(Project project) {
         });
     }
 
-    private static void setupErrorProneArtifactTransform(Project project, boolean modifyVisitorState) {
+    private static void setupErrorProneArtifactTransform(Project project, Set<ModifiedFile> modifiedFiles) {
         Attribute<Boolean> suppressible =
                 Attribute.of("com.palantir.suppressible-error-prone.suppressible", Boolean.class);
         project.getDependencies().getAttributesSchema().attribute(suppressible);
@@ -120,7 +122,7 @@ private static void setupErrorProneArtifactTransform(Project project, boolean mo
                 .attribute(suppressible, false);
 
         project.getDependencies().registerTransform(ModifyErrorProneCheckApi.class, spec -> {
-            spec.getParameters().getModifyVisitorState().set(modifyVisitorState);
+            spec.getParameters().getFilesToModify().set(modifiedFiles);
 
             Attribute<String> artifactType = Attribute.of("artifactType", String.class);
             spec.getFrom().attribute(suppressible, false).attribute(artifactType, "jar");
@@ -205,6 +207,10 @@ private void setupErrorProneOptions(CommonOptions commonOptions, ErrorProneOptio
 
         errorProneOptions.getErrorproneArgumentProviders().add(patchChecksCommandLineArgumentProvider);
 
+        errorProneOptions
+                .getIgnoreSuppressionAnnotations()
+                .set(getProviderFactory().provider(commonOptions::ignoreSuppressionAnnotations));
+
         errorProneOptions
                 .getCheckOptions()
                 .putAll(getProviderFactory().provider(commonOptions::extraErrorProneCheckOptions));

gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/CommonOptions.java

@@ -41,6 +41,10 @@ default RemoveRolloutCheck removeRolloutCheck() {
         return RemoveRolloutCheck.DISABLE;
     }
 
+    default boolean ignoreSuppressionAnnotations() {
+        return false;
+    }
+
     default CommonOptions naivelyCombinedWith(CommonOptions other) {
         return new CommonOptions() {
             @Override
@@ -59,6 +63,11 @@ public Map<String, String> extraErrorProneCheckOptions() {
             public RemoveRolloutCheck removeRolloutCheck() {
                 return CommonOptions.this.removeRolloutCheck().or(other.removeRolloutCheck());
             }
+
+            @Override
+            public boolean ignoreSuppressionAnnotations() {
+                return CommonOptions.this.ignoreSuppressionAnnotations() || other.ignoreSuppressionAnnotations();
+            }
         };
     }
 
@@ -68,16 +77,26 @@ static CommonOptions naivelyCombine(Collection<CommonOptions> commonOptions) {
 
     default CommonOptions withExtraErrorProneCheckFlag(String key, Supplier<String> value) {
         return new CommonOptions() {
+            @Override
+            public Map<String, String> extraErrorProneCheckOptions() {
+                Map<String, String> map = new HashMap<>(CommonOptions.this.extraErrorProneCheckOptions());
+                map.put(key, value.get());
+                return Collections.unmodifiableMap(map);
+            }
+
             @Override
             public PatchChecksOption patchChecks() {
                 return CommonOptions.this.patchChecks();
             }
 
             @Override
-            public Map<String, String> extraErrorProneCheckOptions() {
-                Map<String, String> map = new HashMap<>(CommonOptions.this.extraErrorProneCheckOptions());
-                map.put(key, value.get());
-                return Collections.unmodifiableMap(map);
+            public RemoveRolloutCheck removeRolloutCheck() {
+                return CommonOptions.this.removeRolloutCheck();
+            }
+
+            @Override
+            public boolean ignoreSuppressionAnnotations() {
+                return CommonOptions.this.ignoreSuppressionAnnotations();
             }
         };
     }

gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/ModifyCheckApiOption.java

@@ -47,19 +47,16 @@ static ModifyCheckApiOption noEffect() {
     }
 
     /**
-     * The {@code error_prone_check_api} jar must be modified to allow {@code `for-rollout:`} suppressions to work.
+     * Modify these files in the error-prone API.
      */
-    static ModifyCheckApiOption mustModify() {
-        return new MustModify(false);
+    static MustModify mustModify(ModifiedFile... modifiedFile) {
+        return new MustModify(Set.of(modifiedFile));
     }
 
-    /**
-     * The {@code error_prone_check_api} jar must be modified to allow {@code `for-rollout:`} suppressions to work,
-     * and {@code VisitorState} must be modified to intercept reportMatch.
-     */
-    static ModifyCheckApiOption mustModifyIncludingVisitorState() {
-        return new MustModify(true);
+    default Set<ModifiedFile> getModifiedFiles() {
+        return Set.of();
     }
+    ;
 
     enum DoNotModify implements ModifyCheckApiOption, CombinedValue {
         INSTANCE
@@ -69,9 +66,11 @@ enum NoEffect implements ModifyCheckApiOption {
         INSTANCE
     }
 
-    record MustModify(boolean modifyVisitorState) implements ModifyCheckApiOption, CombinedValue {
+    record MustModify(Set<ModifiedFile> modifiedFiles) implements ModifyCheckApiOption, CombinedValue {
         public MustModify combine(MustModify other) {
-            return new MustModify(modifyVisitorState || other.modifyVisitorState);
+            Set<ModifiedFile> union =
+                    StreamEx.of(modifiedFiles).append(other.modifiedFiles).toSet();
+            return new MustModify(union);
         }
     }
 
@@ -84,7 +83,7 @@ static CombinedValue combine(Collection<ModifyCheckApiOption> options) {
 
         if (withoutNoEffects.isEmpty()) {
             // By default, we need to modify the check API to support for-rollout suppressions
-            return new MustModify(false);
+            return mustModify(ModifiedFile.BUG_CHECKER_INFO);
         }
 
         boolean doNotModify = withoutNoEffects.contains(DoNotModify.INSTANCE);
@@ -103,4 +102,16 @@ static CombinedValue combine(Collection<ModifyCheckApiOption> options) {
                 .reduce(MustModify::combine)
                 .get();
     }
+
+    enum ModifiedFile {
+        BUG_CHECKER_INFO("BugCheckerInfo"),
+        VISITOR_STATE("VisitorState"),
+        SUPPRESSIBLE_TREE_PATH_SCANNER("SuppressibleTreePathScanner");
+
+        private final String className;
+
+        ModifiedFile(String className) {
+            this.className = className;
+        }
+    }
 }

gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/SuppressMode.java

@@ -19,12 +19,13 @@
 import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions;
 import com.palantir.gradle.suppressibleerrorprone.modes.common.Mode;
 import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption;
+import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.ModifiedFile;
 import com.palantir.gradle.suppressibleerrorprone.modes.common.PatchChecksOption;
 
 public final class SuppressMode implements Mode {
     @Override
     public ModifyCheckApiOption modifyCheckApi() {
-        return ModifyCheckApiOption.mustModifyIncludingVisitorState();
+        return ModifyCheckApiOption.mustModify(ModifiedFile.BUG_CHECKER_INFO, ModifiedFile.VISITOR_STATE);
     }
 
     @Override

gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/transform/ModifyErrorProneCheckApi.java

@@ -16,6 +16,7 @@
 
 package com.palantir.gradle.suppressibleerrorprone.transform;
 
+import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.ModifiedFile;
 import com.palantir.gradle.suppressibleerrorprone.transform.ModifyErrorProneCheckApi.Params;
 import com.palantir.gradle.utils.environmentvariables.EnvironmentVariables;
 import java.io.BufferedOutputStream;
@@ -25,6 +26,7 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.util.Optional;
+import java.util.Set;
 import java.util.UUID;
 import java.util.function.UnaryOperator;
 import java.util.jar.JarFile;
@@ -36,6 +38,7 @@
 import org.gradle.api.file.FileSystemLocation;
 import org.gradle.api.provider.Property;
 import org.gradle.api.provider.Provider;
+import org.gradle.api.provider.SetProperty;
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.Nested;
 import org.objectweb.asm.ClassReader;
@@ -67,7 +70,7 @@ private void suppressCheckApi(File output) {
         visitJar(output, (classJarPath, inputStream) -> classVisitorFor(classJarPath)
                 .map(classVisitorFactory -> {
                     ClassReader classReader = newClassReader(inputStream);
-                    ClassWriter classWriter = new ClassWriter(classReader, 0);
+                    ClassWriter classWriter = new ClassWriter(classReader, ClassWriter.COMPUTE_FRAMES);
                     ClassVisitor classVisitor = classVisitorFactory.apply(classWriter);
 
                     classReader.accept(classVisitor, 0);
@@ -79,18 +82,26 @@ private Optional<UnaryOperator<ClassVisitor>> classVisitorFor(String classJarPat
         // We always want to modify the BugCheckerInfo class, as this is where we help errorprone understand
         // what the `for-rollout:` prefix for suppressions mean (which means this class is always modified,
         // even during normal compilation).
-        if (classJarPath.equals("com/google/errorprone/BugCheckerInfo.class")) {
+
+        Set<ModifiedFile> filesToModify = getParameters().getFilesToModify().get();
+        if (classJarPath.equals("com/google/errorprone/BugCheckerInfo.class")
+                && filesToModify.contains(ModifiedFile.BUG_CHECKER_INFO)) {
             return Optional.of(BugCheckerInfoVisitor::new);
         }
 
         // We only want to modify the VisitorState class if we're in stage 1 of the suppression process, as
         // it intercepts all errors and changes them. So when we're just running normal compilation, we want
         // to avoid running our modifications at all, and let the errors continue on their way unchanged.
         if (classJarPath.equals("com/google/errorprone/VisitorState.class")
-                && getParameters().getModifyVisitorState().get()) {
+                && filesToModify.contains(ModifiedFile.VISITOR_STATE)) {
             return Optional.of(VisitorStateClassVisitor::new);
         }
 
+        if (classJarPath.equals("com/google/errorprone/bugpatterns/BugChecker$SuppressibleTreePathScanner.class")
+                && filesToModify.contains(ModifiedFile.SUPPRESSIBLE_TREE_PATH_SCANNER)) {
+            return Optional.of(SuppressibleTreePathScannerClassVisitor::new);
+        }
+
         return Optional.empty();
     }
 
@@ -132,7 +143,7 @@ private void visitJar(File output, ClassTransformer classTransformer) {
 
     public abstract static class Params implements TransformParameters {
         @Input
-        public abstract Property<Boolean> getModifyVisitorState();
+        public abstract SetProperty<ModifiedFile> getFilesToModify();
 
         @Input
         public abstract Property<String> getCacheBust();

gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/transform/SuppressibleTreePathScannerClassVisitor.java

@@ -0,0 +1,86 @@
+/*
+ * (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.transform;
+
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+/**
+ * Forces {@code SuppressibleTreePathScanner} to respect the ignore suppressions option.
+ *
+ * <p>{@code RemoveUnusedSuppressions} must see violations that are normally hidden by suppressions to
+ * determine which suppressions are actually needed. While ErrorProneOptions has an
+ * {@code ignoreSuppressionAnnotations} flag for this purpose, many BugCheckers use
+ * {@code SuppressibleTreePathScanner}, which doesn't respect this flag.
+ *
+ * <p>This class uses bytecode manipulation to patch {@code SuppressibleTreePathScanner::isSuppressed}
+ * to respect the ErrorProneOptions setting.
+ */
+final class SuppressibleTreePathScannerClassVisitor extends ClassVisitor {
+    SuppressibleTreePathScannerClassVisitor(ClassVisitor classVisitor) {
+        super(Opcodes.ASM9, classVisitor);
+    }
+
+    @Override
+    public MethodVisitor visitMethod(
+            int access, String name, String descriptor, String signature, String[] exceptions) {
+        MethodVisitor methodVisitor = super.visitMethod(access, name, descriptor, signature, exceptions);
+
+        if (name.equals("suppressed") && descriptor.equals("(Lcom/sun/source/tree/Tree;)Z")) {
+            return new SuppressedMethodVisitor(methodVisitor);
+        }
+
+        return methodVisitor;
+    }
+
+    private static final class SuppressedMethodVisitor extends MethodVisitor {
+        SuppressedMethodVisitor(MethodVisitor methodVisitor) {
+            super(Opcodes.ASM9, methodVisitor);
+        }
+
+        @Override
+        public void visitCode() {
+            super.visitCode();
+            // Load this (SuppressibleTreePathScanner instance)
+            mv.visitVarInsn(Opcodes.ALOAD, 0);
+            // Get the state field
+            mv.visitFieldInsn(
+                    Opcodes.GETFIELD,
+                    "com/google/errorprone/bugpatterns/BugChecker$SuppressibleTreePathScanner",
+                    "state",
+                    "Lcom/google/errorprone/VisitorState;");
+            // Check condition and potentially return false early
+            mv.visitMethodInsn(
+                    Opcodes.INVOKESTATIC,
+                    "com/palantir/suppressibleerrorprone/SuppressibleTreePathScannerModifications",
+                    "shouldIgnoreSuppressions",
+                    "(Lcom/google/errorprone/VisitorState;)Z",
+                    false);
+
+            // If condition is true, return false immediately
+            Label continueLabel = new Label();
+            mv.visitJumpInsn(Opcodes.IFEQ, continueLabel);
+            mv.visitInsn(Opcodes.ICONST_0); // push false
+            mv.visitInsn(Opcodes.IRETURN);
+            mv.visitLabel(continueLabel);
+            // Add a frame here to fix verification
+            mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
+        }
+    }
+}