Generalize -XepPatchChecks command line parsing

- Support arguments combining built-in and Refaster rules. - Support arguments listing multiple Refaster rules. Fixes #595
google · Oct 23, 2017 · 3caecde · 3caecde
1 parent 319c505
commit 3caecde
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 35 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/BaseErrorProneJavaCompiler.java b/check_api/src/main/java/com/google/errorprone/BaseErrorProneJavaCompiler.java
@@ -18,6 +18,8 @@
 
 import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
 
+import com.google.common.base.Optional;
+import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.RefactoringCollection.RefactoringResult;
 import com.google.errorprone.scanner.ErrorProneScannerTransformer;
@@ -199,24 +201,31 @@ static ErrorProneAnalyzer createAnalyzer(
     }
     refactoringCollection[0] = RefactoringCollection.refactor(epOptions.patchingOptions());
 
-    // Refaster refactorer or using builtin checks
-    CodeTransformer codeTransformer =
-        epOptions
-            .patchingOptions()
-            .customRefactorer()
-            .or(
-                () -> {
-                  ScannerSupplier toUse = ErrorPronePlugins.loadPlugins(scannerSupplier, context);
-                  Set<String> namedCheckers = epOptions.patchingOptions().namedCheckers();
-                  if (!namedCheckers.isEmpty()) {
-                    toUse = toUse.filter(bci -> namedCheckers.contains(bci.canonicalName()));
-                  }
-                  return ErrorProneScannerTransformer.create(toUse.applyOverrides(epOptions).get());
-                })
-            .get();
+    // Use Refaster refactorer if configured
+    Optional<Supplier<CodeTransformer>> customRefactorer =
+        epOptions.patchingOptions().customRefactorer();
+    CodeTransformer builtinTransformer =
+        createBuiltinRefactorer(scannerSupplier, epOptions, context, customRefactorer.isPresent());
+    CodeTransformer compoundTransformer =
+        customRefactorer
+            .transform(s -> CompositeCodeTransformer.compose(s.get(), builtinTransformer))
+            .or(builtinTransformer);
 
     return ErrorProneAnalyzer.createWithCustomDescriptionListener(
-        codeTransformer, epOptions, context, refactoringCollection[0]);
+        compoundTransformer, epOptions, context, refactoringCollection[0]);
+  }
+
+  private static CodeTransformer createBuiltinRefactorer(
+      ScannerSupplier scannerSupplier,
+      ErrorProneOptions epOptions,
+      Context context,
+      boolean forceFilter) {
+    ScannerSupplier toUse = ErrorPronePlugins.loadPlugins(scannerSupplier, context);
+    Set<String> namedCheckers = epOptions.patchingOptions().namedCheckers();
+    if (forceFilter || !namedCheckers.isEmpty()) {
+      toUse = toUse.filter(bci -> namedCheckers.contains(bci.canonicalName()));
+    }
+    return ErrorProneScannerTransformer.create(toUse.applyOverrides(epOptions).get());
   }
 
   static class RefactoringTask implements TaskListener {

diff --git a/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java b/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java
@@ -24,15 +24,18 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.errorprone.apply.ImportOrganizer;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.ObjectInputStream;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -368,25 +371,7 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
             }
           } else if (arg.startsWith(PATCH_CHECKS_PREFIX)) {
             String remaining = arg.substring(PATCH_CHECKS_PREFIX.length());
-            if (remaining.startsWith("refaster:")) {
-              // Refaster rule, load from InputStream at file
-              builder
-                  .patchingOptionsBuilder()
-                  .customRefactorer(
-                      () -> {
-                        String path = remaining.substring("refaster:".length());
-                        try (InputStream in =
-                                Files.newInputStream(FileSystems.getDefault().getPath(path));
-                            ObjectInputStream ois = new ObjectInputStream(in)) {
-                          return (CodeTransformer) ois.readObject();
-                        } catch (IOException | ClassNotFoundException e) {
-                          throw new RuntimeException("Can't load Refaster rule from " + path, e);
-                        }
-                      });
-            } else {
-              Iterable<String> checks = Splitter.on(',').trimResults().split(remaining);
-              builder.patchingOptionsBuilder().namedCheckers(ImmutableSet.copyOf(checks));
-            }
+            configurePatchChecks(builder, remaining);
           } else if (arg.startsWith(PATCH_IMPORT_ORDER_PREFIX)) {
             String remaining = arg.substring(PATCH_IMPORT_ORDER_PREFIX.length());
             ImportOrganizer importOrganizer = ImportOrderParser.getImportOrganizer(remaining);
@@ -400,6 +385,38 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
     return builder.build(remainingArgs.build());
   }
 
+  private static void configurePatchChecks(Builder builder, String arg) {
+    List<Supplier<CodeTransformer>> customCodeTransformers = new ArrayList<>();
+    ImmutableSet.Builder<String> namedCheckers = ImmutableSet.builder();
+    for (String check : Splitter.on(',').trimResults().split(arg)) {
+      if (check.startsWith("refaster:")) {
+        customCodeTransformers.add(
+            createRefasterCodeTransformerLoader(check.substring("refaster:".length())));
+      } else {
+        namedCheckers.add(check);
+      }
+    }
+    builder.patchingOptionsBuilder().namedCheckers(namedCheckers.build());
+    if (!customCodeTransformers.isEmpty())
+      builder
+          .patchingOptionsBuilder()
+          .customRefactorer(
+              () ->
+                  CompositeCodeTransformer.compose(
+                      Iterables.transform(customCodeTransformers, Supplier::get)));
+  }
+
+  private static Supplier<CodeTransformer> createRefasterCodeTransformerLoader(String path) {
+    return () -> {
+      try (InputStream in = Files.newInputStream(FileSystems.getDefault().getPath(path));
+          ObjectInputStream ois = new ObjectInputStream(in)) {
+        return (CodeTransformer) ois.readObject();
+      } catch (IOException | ClassNotFoundException e) {
+        throw new RuntimeException("Can't load Refaster rule from " + path, e);
+      }
+    };
+  }
+
   /**
    * Given a list of command-line arguments, produce the corresponding {@link ErrorProneOptions}
    * instance.