Check that -XDshould-stop.ifError=FLOW is set when running Error Prone

This is necessary to ensure javac compiles the 'flow' phase for subsequent compilation units after Error Prone reports diagnostics. The 'flow' phase is necessary among other things to set `EFFECTIVELY_FINAL`. Configuring a later stop policy also causes more diagnostics to be shown at once for failing builds when using `-XDcompilePolicy=simple`, which is helpful.

#4595

RELNOTES=The javac flag `-XDshould-stop.ifError=FLOW` is now required when running Error Prone, see #4595
PiperOrigin-RevId: 686102738

check_api/src/main/java/com/google/errorprone/BaseErrorProneJavaCompiler.java

@@ -23,6 +23,7 @@
 import com.sun.source.util.JavacTask;
 import com.sun.tools.javac.api.BasicJavacTask;
 import com.sun.tools.javac.api.JavacTool;
+import com.sun.tools.javac.comp.CompileStates.CompileState;
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.JavacMessages;
 import com.sun.tools.javac.util.Options;
@@ -70,6 +71,7 @@ public CompilationTask getTask(
     ImmutableList<String> javacOpts = errorProneOptions.getRemainingArgs();
     javacOpts = defaultToLatestSupportedLanguageLevel(javacOpts);
     javacOpts = setCompilePolicyToByFile(javacOpts);
+    javacOpts = setShouldStopIfErrorPolicyToFlow(javacOpts);
     JavacTask task =
         (JavacTask)
             javacTool.getTask(
@@ -194,6 +196,29 @@ private static ImmutableList<String> setCompilePolicyToByFile(ImmutableList<Stri
     return ImmutableList.<String>builder().addAll(args).add("-XDcompilePolicy=simple").build();
   }
 
+  private static void checkShouldStopIfErrorPolicy(String value) {
+    CompileState state = CompileState.valueOf(value);
+    if (CompileState.FLOW.isAfter(state)) {
+      throw new InvalidCommandLineOptionException(
+          String.format(
+              "-XDshould-stop.ifError=%s is not supported by Error Prone, pass"
+                  + " -XDshould-stop.ifError=FLOW instead",
+              state));
+    }
+  }
+
+  private static ImmutableList<String> setShouldStopIfErrorPolicyToFlow(
+      ImmutableList<String> args) {
+    for (String arg : args) {
+      if (arg.startsWith("-XDshould-stop.ifError")) {
+        String value = arg.substring(arg.indexOf('=') + 1);
+        checkShouldStopIfErrorPolicy(value);
+        return args; // don't do anything if a valid policy is already set
+      }
+    }
+    return ImmutableList.<String>builder().addAll(args).add("-XDshould-stop.ifError=FLOW").build();
+  }
+
   /** Registers our message bundle. */
   public static void setupMessageBundle(Context context) {
     ResourceBundle bundle = ResourceBundle.getBundle("com.google.errorprone.errors");

core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java

@@ -39,17 +39,20 @@
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
 import com.google.errorprone.bugpatterns.NonAtomicVolatileUpdate;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.scanner.BuiltInCheckerSuppliers;
 import com.google.errorprone.scanner.ScannerSupplier;
+import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ExpressionStatementTree;
 import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.MemberSelectTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.ReturnTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.VariableTree;
 import com.sun.tools.javac.main.Main.Result;
 import java.io.PrintWriter;
 import java.io.StringWriter;
@@ -669,4 +672,82 @@ public class Test {
     outputStream.flush();
     assertWithMessage(outputStream.toString()).that(exitCode).isEqualTo(Result.OK);
   }
+
+  @BugPattern(summary = "All variables should be effectively final", severity = ERROR)
+  public static class EffectivelyFinalChecker extends BugChecker implements VariableTreeMatcher {
+    @Override
+    public Description matchVariable(VariableTree tree, VisitorState state) {
+      if (ASTHelpers.isConsideredFinal(ASTHelpers.getSymbol(tree))) {
+        return NO_MATCH;
+      }
+      return describeMatch(tree);
+    }
+  }
+
+  @Test
+  public void stopPolicy_effectivelyFinal() {
+    compilerBuilder.report(
+        ScannerSupplier.fromBugCheckerClasses(EffectivelyFinalChecker.class, CPSChecker.class));
+    compiler = compilerBuilder.build();
+    // Without -XDshould-stop.ifError=FLOW, the errors reported by CPSChecker will cause javac to
+    // stop processing B after an error is reported in A. Error Prone will still analyze B without
+    // it having gone through 'flow', and the EFFECTIVELY_FINAL analysis will not have happened.
+    // see https://github.com/google/error-prone/issues/4595
+    Result exitCode =
+        compiler.compile(
+            ImmutableList.of(
+                forSourceLines(
+                    "A.java",
+                    """
+                    class A {
+                      int f(int x) {
+                        return x;
+                      }
+                    }
+                    """),
+                forSourceLines(
+                    "B.java",
+                    """
+                    class B {
+                      int f(int x) {
+                        return x;
+                      }
+                    }
+                    """)));
+
+    outputStream.flush();
+    assertWithMessage(outputStream.toString()).that(exitCode).isEqualTo(Result.ERROR);
+
+    assertThat(diagnosticHelper.getDiagnostics()).hasSize(2);
+    assertWithMessage("Error should be found. " + diagnosticHelper.describe())
+        .that(diagnosticHelper.getDiagnostics())
+        .comparingElementsUsing(DIAGNOSTIC_CONTAINING)
+        .containsExactly("[CPSChecker]", "[CPSChecker]");
+  }
+
+  @Test
+  public void stopPolicy_flow() {
+    Result exitCode =
+        compiler.compile(
+            new String[] {"-XDshould-stop.ifError=FLOW"},
+            ImmutableList.of(
+                forSourceLines(
+                    "Test.java",
+                    """
+                    package test;
+                    class Test {}
+                    """)));
+    outputStream.flush();
+    assertWithMessage(outputStream.toString()).that(exitCode).isEqualTo(Result.OK);
+  }
+
+  @Test
+  public void stopPolicy_init() {
+    InvalidCommandLineOptionException e =
+        assertThrows(
+            InvalidCommandLineOptionException.class,
+            () ->
+                compiler.compile(new String[] {"-XDshould-stop.ifError=INIT"}, ImmutableList.of()));
+    assertThat(e).hasMessageThat().contains("-XDshould-stop.ifError=INIT is not supported");
+  }
 }