From 020d45e7fc691fef08cd508e106b175ef8a0284c Mon Sep 17 00:00:00 2001
From: Stephan Schroevers <stephan.schroevers@teampicnic.com>
Date: Wed, 21 Feb 2018 15:30:19 +0100
Subject: [PATCH] Support running all available patch checks

The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.
---
 .../google/errorprone/ErrorProneOptions.java  | 28 ++++++++-----------
 .../errorprone/ErrorProneOptionsTest.java     | 21 ++++++++++++--
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java b/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java
index 768f556cc6ce..32686d35422d 100644
--- a/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java
+++ b/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java
@@ -132,21 +132,7 @@ abstract static class Builder {
 
       abstract Builder importOrganizer(ImportOrganizer importOrganizer);
 
-      abstract PatchingOptions autoBuild();
-
-      final PatchingOptions build() {
-
-        PatchingOptions patchingOptions = autoBuild();
-
-        // If anything is specified, then (checkers or refaster) and output must be set.
-        if ((!patchingOptions.namedCheckers().isEmpty()
-                || patchingOptions.customRefactorer().isPresent())
-            ^ patchingOptions.doRefactor()) {
-          throw new InvalidCommandLineOptionException(
-              "-XepPatchChecks and -XepPatchLocation must be specified together");
-        }
-        return patchingOptions;
-      }
+      abstract PatchingOptions build();
     }
   }
 
@@ -347,6 +333,8 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
      * You can pass the IGNORE_UNKNOWN_CHECKS_FLAG to opt-out of that checking.  This allows you to
      * use command lines from different versions of error-prone interchangeably.
      */
+    boolean patchLocationSet = false;
+    boolean patchCheckSet = false;
     Builder builder = new Builder();
     for (String arg : args) {
       switch (arg) {
@@ -374,6 +362,7 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
           } else if (arg.startsWith(ErrorProneFlags.PREFIX)) {
             builder.parseFlag(arg);
           } else if (arg.startsWith(PATCH_OUTPUT_LOCATION)) {
+            patchLocationSet = true;
             String remaining = arg.substring(PATCH_OUTPUT_LOCATION.length());
             if (remaining.equals("IN_PLACE")) {
               builder.patchingOptionsBuilder().inPlace(true);
@@ -384,6 +373,7 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
               builder.patchingOptionsBuilder().baseDirectory(remaining);
             }
           } else if (arg.startsWith(PATCH_CHECKS_PREFIX)) {
+            patchCheckSet = true;
             String remaining = arg.substring(PATCH_CHECKS_PREFIX.length());
             if (remaining.startsWith("refaster:")) {
               // Refaster rule, load from InputStream at file
@@ -401,7 +391,8 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
                         }
                       });
             } else {
-              Iterable<String> checks = Splitter.on(',').trimResults().split(remaining);
+              Iterable<String> checks =
+                  Splitter.on(',').trimResults().omitEmptyStrings().split(remaining);
               builder.patchingOptionsBuilder().namedCheckers(ImmutableSet.copyOf(checks));
             }
           } else if (arg.startsWith(PATCH_IMPORT_ORDER_PREFIX)) {
@@ -417,6 +408,11 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
       }
     }
 
+    if (patchCheckSet && !patchLocationSet) {
+      throw new InvalidCommandLineOptionException(
+          "-XepPatchLocation must be specified when -XepPatchChecks is");
+    }
+
     return builder.build(remainingArgs.build());
   }
 
diff --git a/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java b/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java
index ab7afefef563..8b2cc8a59369 100644
--- a/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java
+++ b/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java
@@ -207,9 +207,6 @@ public void recognizesPatch() {
 
   @Test
   public void throwsExceptionWithBadPatchArgs() {
-    assertThrows(
-        InvalidCommandLineOptionException.class,
-        () -> ErrorProneOptions.processArgs(new String[] {"-XepPatchLocation:IN_PLACE"}));
     assertThrows(
         InvalidCommandLineOptionException.class,
         () ->
@@ -226,6 +223,24 @@ public void recognizesRefaster() {
     assertThat(options.patchingOptions().customRefactorer()).isPresent();
   }
 
+  @Test
+  public void understandsEmptySetOfNamedCheckers() {
+    ErrorProneOptions options =
+        ErrorProneOptions.processArgs(new String[] {"-XepPatchLocation:IN_PLACE"});
+    assertThat(options.patchingOptions().doRefactor()).isTrue();
+    assertThat(options.patchingOptions().inPlace()).isTrue();
+    assertThat(options.patchingOptions().namedCheckers()).isEmpty();
+    assertThat(options.patchingOptions().customRefactorer()).isAbsent();
+
+    options =
+        ErrorProneOptions.processArgs(
+            new String[] {"-XepPatchLocation:IN_PLACE", "-XepPatchChecks:"});
+    assertThat(options.patchingOptions().doRefactor()).isTrue();
+    assertThat(options.patchingOptions().inPlace()).isTrue();
+    assertThat(options.patchingOptions().namedCheckers()).isEmpty();
+    assertThat(options.patchingOptions().customRefactorer()).isAbsent();
+  }
+
   @Test
   public void importOrder_staticFirst() {
     ErrorProneOptions options =