Enable applying refaster rules for repos with -Xlint:deprecation (#742)

palantir · Aug 7, 2019 · cfc9c10 · cfc9c10
1 parent f6ad5c8
commit cfc9c10
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 17 deletions.
diff --git a/changelog/@unreleased/pr-742.v2.yml b/changelog/@unreleased/pr-742.v2.yml
@@ -0,0 +1,5 @@
+type: fix
+fix:
+  description: Enable applying refaster rules for repos with -Xlint:deprecation
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/742
diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineErrorProne.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineErrorProne.java
@@ -108,29 +108,49 @@ public void apply(Project project) {
                                 return;
                             }
 
-                            if (project.hasProperty(PROP_REFASTER_APPLY)) {
-                                javaCompile.dependsOn(compileRefaster);
-                                javaCompile.getOptions().setWarnings(false);
-                                errorProneOptions.getErrorproneArgumentProviders().add(() -> ImmutableList.of(
-                                        "-XepPatchChecks:refaster:" + refasterRulesFile.get().getAbsolutePath(),
-                                        "-XepPatchLocation:IN_PLACE"));
-                                // Don't attempt to cache since it won't capture the source files that might be modified
-                                javaCompile.getOutputs().cacheIf(t -> false);
-                            } else if (project.hasProperty(PROP_ERROR_PRONE_APPLY)) {
-                                javaCompile.getOptions().setWarnings(false);
-                                // 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"));
+                            if (isRefactoring(project)) {
                                 // Don't attempt to cache since it won't capture the source files that might be modified
                                 javaCompile.getOutputs().cacheIf(t -> false);
+
+                                if (isRefasterRefactoring(project)) {
+                                    javaCompile.dependsOn(compileRefaster);
+                                    errorProneOptions.getErrorproneArgumentProviders().add(() -> ImmutableList.of(
+                                            "-XepPatchChecks:refaster:" + refasterRulesFile.get().getAbsolutePath(),
+                                            "-XepPatchLocation:IN_PLACE"));
+                                }
+
+                                if (isErrorProneRefactoring(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"));
+                                }
                             }
                         });
             });
 
+            // To allow refactoring of deprecated methods, even when -Xlint:deprecation is specified, we need to remove
+            // these compiler flags after all configuration has happened.
+            project.afterEvaluate(unused -> project.getTasks().withType(JavaCompile.class)
+                    .configureEach(javaCompile -> {
+                        if (javaCompile.equals(compileRefaster)) {
+                            return;
+                        }
+                        if (isRefactoring(project)) {
+                            javaCompile.getOptions().setWarnings(false);
+                            javaCompile.getOptions().setDeprecation(false);
+                            javaCompile.getOptions().setCompilerArgs(javaCompile.getOptions().getCompilerArgs()
+                                    .stream()
+                                    .filter(arg -> !arg.equals("-Werror"))
+                                    .filter(arg -> !arg.equals("-deprecation"))
+                                    .filter(arg -> !arg.equals("-Xlint:deprecation"))
+                                    .collect(Collectors.toList()));
+                        }
+                    }));
+
             project.getPluginManager().withPlugin("java-gradle-plugin", appliedPlugin -> {
                 project.getTasks().withType(JavaCompile.class).configureEach(javaCompile ->
                         ((ExtensionAware) javaCompile.getOptions()).getExtensions()
@@ -166,6 +186,18 @@ public void apply(Project project) {
         });
     }
 
+    private boolean isRefactoring(Project project) {
+        return isRefasterRefactoring(project) || isErrorProneRefactoring(project);
+    }
+
+    private boolean isRefasterRefactoring(Project project) {
+        return project.hasProperty(PROP_REFASTER_APPLY);
+    }
+
+    private boolean isErrorProneRefactoring(Project project) {
+        return project.hasProperty(PROP_ERROR_PRONE_APPLY);
+    }
+
     private static final class LazyConfigurationList extends AbstractList<File> {
         private final FileCollection files;
         private List<File> fileList;

diff --git a/...va/src/test/groovy/com/palantir/baseline/BaselineErrorProneRefasterIntegrationTest.groovy b/...va/src/test/groovy/com/palantir/baseline/BaselineErrorProneRefasterIntegrationTest.groovy
@@ -35,6 +35,9 @@ class BaselineErrorProneRefasterIntegrationTest extends AbstractPluginTest {
             mavenLocal()
             jcenter()
         }
+        tasks.withType(JavaCompile) {
+            options.compilerArgs += ['-Werror', '-Xlint:deprecation']
+        }
     '''.stripIndent()
 
     def 'compileJava with refaster fixes CollectionsIsEmpty'() {
@@ -110,4 +113,32 @@ class BaselineErrorProneRefasterIntegrationTest extends AbstractPluginTest {
         }
         '''.stripIndent()
     }
+
+    def 'compileJava with refaster fixes Utf8Length with deprecated method'() {
+        when:
+        buildFile << standardBuildFile
+        file('src/main/java/test/Test.java') << '''
+        package test;
+        import com.google.common.base.CharMatcher;
+        import java.nio.charset.StandardCharsets;
+        public class Test {
+            CharMatcher matcher = CharMatcher.digit();  // Would normally fail with -Xlint:deprecation
+            int i = "hello world".getBytes(StandardCharsets.UTF_8).length;
+        }
+        '''.stripIndent()
+
+        then:
+        BuildResult result = with('compileJava', '-i', '-PrefasterApply').build()
+        result.task(":compileJava").outcome == TaskOutcome.SUCCESS
+        file('src/main/java/test/Test.java').text == '''
+        package test;
+        import com.google.common.base.CharMatcher;
+        import com.google.common.base.Utf8;
+        import java.nio.charset.StandardCharsets;
+        public class Test {
+            CharMatcher matcher = CharMatcher.digit();  // Would normally fail with -Xlint:deprecation
+            int i = Utf8.encodedLength("hello world");
+        }
+        '''.stripIndent()
+    }
 }