Fix: don't apply error prone patches for disabled checks (#793)

Stop applying error prone patches for checks that have been turned off.

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferSafeLoggableExceptions.java

@@ -41,6 +41,7 @@
         linkType = BugPattern.LinkType.CUSTOM,
         severity = BugPattern.SeverityLevel.WARNING,
         summary = "Throw SafeLoggable exceptions to ensure the exception message will not be redacted")
+@SuppressWarnings("PreferSafeLoggableExceptions")
 public final class PreferSafeLoggableExceptions extends BugChecker implements BugChecker.NewClassTreeMatcher {
 
     private static final long serialVersionUID = 1L;

changelog/@unreleased/pr-793.v2.yml

@@ -0,0 +1,6 @@
+type: fix
+fix:
+  description: Stop applying error prone patches for checks that have been turned
+    off.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/793

gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineErrorProne.java

@@ -24,8 +24,10 @@
 import java.io.File;
 import java.util.AbstractList;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import net.ltgt.gradle.errorprone.CheckSeverity;
 import net.ltgt.gradle.errorprone.ErrorProneOptions;
 import net.ltgt.gradle.errorprone.ErrorPronePlugin;
@@ -36,13 +38,16 @@
 import org.gradle.api.artifacts.Configuration;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.file.RegularFile;
+import org.gradle.api.logging.Logger;
+import org.gradle.api.logging.Logging;
 import org.gradle.api.plugins.ExtensionAware;
 import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.compile.JavaCompile;
 import org.gradle.api.tasks.javadoc.Javadoc;
 import org.gradle.api.tasks.testing.Test;
 
 public final class BaselineErrorProne implements Plugin<Project> {
+    private static final Logger log = Logging.getLogger(BaselineErrorProne.class);
 
     public static final String REFASTER_CONFIGURATION = "refaster";
     public static final String EXTENSION_NAME = "baselineErrorProne";
@@ -51,6 +56,7 @@ public final class BaselineErrorProne implements Plugin<Project> {
     private static final String PROP_ERROR_PRONE_APPLY = "errorProneApply";
     private static final String PROP_REFASTER_APPLY = "refasterApply";
 
+    @SuppressWarnings("UnstableApiUsage")
     @Override
     public void apply(Project project) {
         project.getPluginManager().withPlugin("java", plugin -> {
@@ -132,10 +138,14 @@ public void apply(Project project) {
                                     // TODO(gatesn): Is there a way to discover error-prone checks?
                                     // Maybe service-load from a ClassLoader configured with annotation processor path?
                                     // https://github.com/google/error-prone/pull/947
-                                    errorProneOptions.getErrorproneArgumentProviders().add(() -> ImmutableList.of(
-                                            "-XepPatchChecks:" + Joiner.on(',')
-                                                    .join(errorProneExtension.getPatchChecks().get()),
-                                            "-XepPatchLocation:IN_PLACE"));
+                                    errorProneOptions.getErrorproneArgumentProviders().add(() -> {
+                                        // Don't apply checks that have been explicitly disabled
+                                        Stream<String> errorProneChecks = getNotDisabledErrorproneChecks(
+                                                errorProneExtension, javaCompile, errorProneOptions);
+                                        return ImmutableList.of(
+                                                "-XepPatchChecks:" + Joiner.on(',').join(errorProneChecks.iterator()),
+                                                "-XepPatchLocation:IN_PLACE");
+                                    });
                                 }
                             }
                         });
@@ -196,6 +206,22 @@ public void apply(Project project) {
         });
     }
 
+    private static Stream<String> getNotDisabledErrorproneChecks(
+            BaselineErrorProneExtension errorProneExtension,
+            JavaCompile javaCompile,
+            ErrorProneOptions errorProneOptions) {
+        return errorProneExtension.getPatchChecks().get().stream().filter(check -> {
+            if (checkExplicitlyDisabled(errorProneOptions, check)) {
+                log.info(
+                        "Task {}: not applying errorprone check {} because it has severity OFF in errorProneOptions",
+                        javaCompile.getPath(),
+                        check);
+                return false;
+            }
+            return true;
+        });
+    }
+
     private boolean isRefactoring(Project project) {
         return isRefasterRefactoring(project) || isErrorProneRefactoring(project);
     }
@@ -208,6 +234,12 @@ private boolean isErrorProneRefactoring(Project project) {
         return project.hasProperty(PROP_ERROR_PRONE_APPLY);
     }
 
+    private static boolean checkExplicitlyDisabled(ErrorProneOptions errorProneOptions, String check) {
+        Map<String, CheckSeverity> checks = errorProneOptions.getChecks();
+        return checks.get(check) == CheckSeverity.OFF
+                || errorProneOptions.getErrorproneArgs().contains(String.format("-Xep:%s:OFF", check));
+    }
+
     private static final class LazyConfigurationList extends AbstractList<File> {
         private final FileCollection files;
         private List<File> fileList;

gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineErrorProneIntegrationTest.groovy

@@ -19,6 +19,7 @@ package com.palantir.baseline
 
 import org.gradle.testkit.runner.BuildResult
 import org.gradle.testkit.runner.TaskOutcome
+import spock.lang.Unroll
 
 /**
  * This test depends on ./gradlew :baseline-error-prone:publishToMavenLocal
@@ -128,4 +129,50 @@ class BaselineErrorProneIntegrationTest extends AbstractPluginTest {
         }
         '''.stripIndent()
     }
+
+    enum CheckConfigurationMethod { ARG, DSL }
+
+    @Unroll
+    def 'compileJava does not apply patches for error-prone checks that were turned OFF via #checkConfigurationMethod'() {
+        def checkName = "Slf4jLogsafeArgs"
+        def turnOffCheck = [
+                (CheckConfigurationMethod.ARG): "options.errorprone.errorproneArgs += ['-Xep:$checkName:OFF']",
+                (CheckConfigurationMethod.DSL): """
+                    options.errorprone {
+                        check '$checkName', net.ltgt.gradle.errorprone.CheckSeverity.OFF
+                    }                    
+                """.stripIndent(),
+        ]
+
+        buildFile << standardBuildFile
+        buildFile << """
+            tasks.withType(JavaCompile) {
+                ${turnOffCheck[checkConfigurationMethod]}
+            }
+            dependencies {
+                implementation 'org.slf4j:slf4j-api:1.7.25'
+            }
+        """.stripIndent()
+
+        def correctJavaFile = '''
+        package test;
+        import org.slf4j.LoggerFactory;
+        import org.slf4j.Logger;
+        public class Test {
+            void test() {
+                Logger log = LoggerFactory.getLogger("foo");
+                log.info("Hi there {}", "non safe arg");
+            }
+        }
+        '''.stripIndent()
+        file('src/main/java/test/Test.java') << correctJavaFile
+
+        expect:
+        BuildResult result = with('compileJava', '-PerrorProneApply').build()
+        result.task(":compileJava").outcome == TaskOutcome.SUCCESS
+        file('src/main/java/test/Test.java').text == correctJavaFile
+
+        where:
+        checkConfigurationMethod << CheckConfigurationMethod.values()
+    }
 }