Check that --should-stop=ifError=FLOW is set when EP is set up usin…

…g the `-Xplugin` integration See #4595 (comment) PiperOrigin-RevId: 687659943
google · Oct 28, 2024 · 6a918bd · 6a918bd
1 parent d67bc15
commit 6a918bd
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 12 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
@@ -84,6 +84,7 @@ static void addTaskListener(
       JavacTask javacTask, ScannerSupplier scannerSupplier, ErrorProneOptions errorProneOptions) {
     Context context = ((BasicJavacTask) javacTask).getContext();
     checkCompilePolicy(Options.instance(context).get("compilePolicy"));
+    checkShouldStopIfErrorPolicy(Options.instance(context).get("should-stop.ifError"));
     setupMessageBundle(context);
     RefactoringCollection[] refactoringCollection = {null};
     javacTask.addTaskListener(
@@ -196,21 +197,28 @@ private static ImmutableList<String> setCompilePolicyToByFile(ImmutableList<Stri
     return ImmutableList.<String>builder().addAll(args).add("-XDcompilePolicy=simple").build();
   }
 
-  private static void checkShouldStopIfErrorPolicy(String arg) {
-    String value = arg.substring(arg.lastIndexOf('=') + 1);
+  private static void checkShouldStopIfErrorPolicy(String value) {
+    if (value == null) {
+      throw new InvalidCommandLineOptionException(
+          "The default --should-stop=ifError policy (INIT) is not supported by Error Prone,"
+              + " pass --should-stop=ifError=FLOW instead");
+    }
     CompileState state = CompileState.valueOf(value);
     if (CompileState.FLOW.isAfter(state)) {
       throw new InvalidCommandLineOptionException(
           String.format(
-              "%s is not supported by Error Prone, pass --should-stop=ifError=FLOW instead", arg));
+              "--should-stop=ifError=%s is not supported by Error Prone, pass"
+                  + " --should-stop=ifError=FLOW instead",
+              value));
     }
   }
 
   private static ImmutableList<String> setShouldStopIfErrorPolicyToFlow(
       ImmutableList<String> args) {
     for (String arg : args) {
       if (arg.startsWith("--should-stop=ifError") || arg.startsWith("-XDshould-stop.ifError")) {
-        checkShouldStopIfErrorPolicy(arg);
+        String value = arg.substring(arg.lastIndexOf('=') + 1);
+        checkShouldStopIfErrorPolicy(value);
         return args; // don't do anything if a valid policy is already set
       }
     }

diff --git a/core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java b/core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java
@@ -758,6 +758,6 @@ public void stopPolicy_init_xD() {
             InvalidCommandLineOptionException.class,
             () ->
                 compiler.compile(new String[] {"-XDshould-stop.ifError=INIT"}, ImmutableList.of()));
-    assertThat(e).hasMessageThat().contains("-XDshould-stop.ifError=INIT is not supported");
+    assertThat(e).hasMessageThat().contains("--should-stop=ifError=INIT is not supported");
   }
 }
diff --git a/core/src/test/java/com/google/errorprone/ErrorProneJavacPluginTest.java b/core/src/test/java/com/google/errorprone/ErrorProneJavacPluginTest.java
@@ -100,7 +100,8 @@ public void hello() throws IOException {
                 null,
                 fileManager,
                 diagnosticCollector,
-                ImmutableList.of("-Xplugin:ErrorProne", "-XDcompilePolicy=byfile"),
+                ImmutableList.of(
+                    "-Xplugin:ErrorProne", "-XDcompilePolicy=byfile", "--should-stop=ifError=FLOW"),
                 ImmutableList.of(),
                 fileManager.getJavaFileObjects(source));
     assertThat(task.call()).isFalse();
@@ -144,7 +145,8 @@ public void applyFixes() throws IOException {
                 ImmutableList.of(
                     "-Xplugin:ErrorProne"
                         + " -XepPatchChecks:MissingOverride -XepPatchLocation:IN_PLACE",
-                    "-XDcompilePolicy=byfile"),
+                    "-XDcompilePolicy=byfile",
+                    "--should-stop=ifError=FLOW"),
                 ImmutableList.of(),
                 fileManager.getJavaFileObjects(fileA, fileB));
     assertWithMessage(Joiner.on('\n').join(diagnosticCollector.getDiagnostics()))
@@ -203,7 +205,8 @@ public void applyToPatchFile() throws IOException {
                     "-Xplugin:ErrorProne"
                         + " -XepPatchChecks:MissingOverride -XepPatchLocation:"
                         + patchDir,
-                    "-XDcompilePolicy=byfile"),
+                    "-XDcompilePolicy=byfile",
+                    "--should-stop=ifError=FLOW"),
                 ImmutableList.of(),
                 fileManager.getJavaFileObjects(fileA, fileB));
     assertWithMessage(Joiner.on('\n').join(diagnosticCollector.getDiagnostics()))
@@ -254,7 +257,8 @@ public void explicitBadPolicyGiven() throws IOException {
                 new PrintWriter(sw, true),
                 fileManager,
                 diagnosticCollector,
-                ImmutableList.of("-XDcompilePolicy=bytodo", "-Xplugin:ErrorProne"),
+                ImmutableList.of(
+                    "-XDcompilePolicy=bytodo", "--should-stop=ifError=FLOW", "-Xplugin:ErrorProne"),
                 ImmutableList.of(),
                 fileManager.getJavaFileObjects(source));
     RuntimeException expected = assertThrows(RuntimeException.class, () -> task.call());
@@ -375,7 +379,8 @@ public void compilesWithFix() throws IOException {
                 diagnosticCollector,
                 ImmutableList.of(
                     "-Xplugin:ErrorProne -XepDisableAllChecks -Xep:TestCompilesWithFix:ERROR",
-                    "-XDcompilePolicy=byfile"),
+                    "-XDcompilePolicy=byfile",
+                    "--should-stop=ifError=FLOW"),
                 ImmutableList.of(),
                 fileManager.getJavaFileObjects(source));
     assertThat(task.call()).isFalse();
@@ -385,4 +390,49 @@ public void compilesWithFix() throws IOException {
             .collect(onlyElement());
     assertThat(diagnostic.getMessage(ENGLISH)).contains("[TestCompilesWithFix]");
   }
+
+  @Test
+  public void noShouldStopIfErrorPolicy() throws IOException {
+    FileSystem fileSystem = Jimfs.newFileSystem(Configuration.unix());
+    Path source = fileSystem.getPath("Test.java");
+    Files.writeString(source, "class Test {}");
+    JavacFileManager fileManager = new JavacFileManager(new Context(), false, UTF_8);
+    DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
+    StringWriter sw = new StringWriter();
+    JavacTask task =
+        JavacTool.create()
+            .getTask(
+                new PrintWriter(sw, true),
+                fileManager,
+                diagnosticCollector,
+                ImmutableList.of("-Xplugin:ErrorProne", "-XDcompilePolicy=byfile"),
+                ImmutableList.of(),
+                fileManager.getJavaFileObjects(source));
+    RuntimeException expected = assertThrows(RuntimeException.class, task::call);
+    assertThat(expected)
+        .hasMessageThat()
+        .contains("The default --should-stop=ifError policy (INIT) is not supported");
+  }
+
+  @Test
+  public void shouldStopIfErrorPolicyInit() throws IOException {
+    FileSystem fileSystem = Jimfs.newFileSystem(Configuration.unix());
+    Path source = fileSystem.getPath("Test.java");
+    Files.writeString(source, "class Test {}");
+    JavacFileManager fileManager = new JavacFileManager(new Context(), false, UTF_8);
+    DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
+    StringWriter sw = new StringWriter();
+    JavacTask task =
+        JavacTool.create()
+            .getTask(
+                new PrintWriter(sw, true),
+                fileManager,
+                diagnosticCollector,
+                ImmutableList.of(
+                    "-Xplugin:ErrorProne", "-XDcompilePolicy=byfile", "--should-stop=ifError=INIT"),
+                ImmutableList.of(),
+                fileManager.getJavaFileObjects(source));
+    RuntimeException expected = assertThrows(RuntimeException.class, task::call);
+    assertThat(expected).hasMessageThat().contains("--should-stop=ifError=INIT is not supported");
+  }
 }
diff --git a/core/src/test/java/com/google/errorprone/VisitorStateTest.java b/core/src/test/java/com/google/errorprone/VisitorStateTest.java
@@ -180,7 +180,8 @@ public void memoizeCannotAccessTreePath() throws IOException {
                 ImmutableList.of(
                     "-Xplugin:ErrorProne -XepDisableAllChecks"
                         + " -Xep:CheckThatTriesToMemoizeBasedOnTreePath:ERROR",
-                    "-XDcompilePolicy=byfile"),
+                    "-XDcompilePolicy=byfile",
+                    "--should-stop=ifError=FLOW"),
                 ImmutableList.of(),
                 fileManager.getJavaFileObjects(source));
     assertThat(task.call()).isFalse();

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnicodeInCodeTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnicodeInCodeTest.java
@@ -162,7 +162,8 @@ public void asciiSub() {
                 null,
                 fileManager,
                 diagnosticCollector,
-                ImmutableList.of("-Xplugin:ErrorProne", "-XDcompilePolicy=simple"),
+                ImmutableList.of(
+                    "-Xplugin:ErrorProne", "-XDcompilePolicy=simple", "--should-stop=ifError=FLOW"),
                 ImmutableList.of(),
                 ImmutableList.of(
                     new SimpleJavaFileObject(