Fix a bug in the logic that prevents Error Prone from running on comp…

…ilations with underlying errors

PiperOrigin-RevId: 703169425

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

@@ -19,9 +19,8 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 
-import com.google.common.base.Supplier;
-import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.fixes.AppliedFix;
 import com.google.errorprone.fixes.Fix;
 import com.google.errorprone.matchers.Description;
@@ -33,7 +32,6 @@
 import com.sun.tools.javac.util.Log;
 import java.io.IOException;
 import java.io.UncheckedIOException;
-import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -60,17 +58,12 @@ public class JavacErrorDescriptionListener implements DescriptionListener {
   // The suffix for properties in src/main/resources/com/google/errorprone/errors.properties
   private static final String MESSAGE_BUNDLE_KEY = "error.prone";
 
-  // DiagnosticFlag.MULTIPLE went away in JDK13, so we want to load it if it's available.
-  private static final Supplier<EnumSet<JCDiagnostic.DiagnosticFlag>> diagnosticFlags =
-      Suppliers.memoize(
-          () -> {
-            try {
-              return EnumSet.of(JCDiagnostic.DiagnosticFlag.valueOf("MULTIPLE"));
-            } catch (IllegalArgumentException iae) {
-              // JDK 13 and above
-              return EnumSet.noneOf(JCDiagnostic.DiagnosticFlag.class);
-            }
-          });
+  // DiagnosticFlag.API ensures that errors are always reported, bypassing 'shouldReport' logic
+  // that filters out duplicate diagnostics at the same position, and ensures that
+  // ErrorProneAnalyzer can compare the counts of errors reported by Error Prone with the total
+  // number of errors reported.
+  private static final ImmutableSet<JCDiagnostic.DiagnosticFlag> DIAGNOSTIC_FLAGS =
+      ImmutableSet.of(JCDiagnostic.DiagnosticFlag.API);
 
   private JavacErrorDescriptionListener(
       Log log,
@@ -125,7 +118,7 @@ public void onDescribed(Description description) {
           factory.create(
               type,
               /* lintCategory */ null,
-              diagnosticFlags.get(),
+              DIAGNOSTIC_FLAGS,
               log.currentSource(),
               pos,
               MESSAGE_BUNDLE_KEY,

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

@@ -16,6 +16,7 @@
 
 package com.google.errorprone;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
@@ -25,6 +26,7 @@
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.util.ASTHelpers.constValue;
 import static java.util.Locale.ENGLISH;
+import static java.util.Objects.requireNonNull;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assume.assumeTrue;
 
@@ -760,4 +762,65 @@ public void stopPolicy_init_xD() {
                 compiler.compile(new String[] {"-XDshould-stop.ifError=INIT"}, ImmutableList.of()));
     assertThat(e).hasMessageThat().contains("--should-stop=ifError=INIT is not supported");
   }
+
+  @BugPattern(summary = "Checks that symbols are non-null", severity = ERROR)
+  public static class GetSymbolChecker extends BugChecker implements MethodInvocationTreeMatcher {
+    @Override
+    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+      requireNonNull(ASTHelpers.getSymbol(tree));
+      return NO_MATCH;
+    }
+  }
+
+  @BugPattern(summary = "Duplicates CPSChecker", severity = ERROR)
+  public static class CPSChecker2 extends CPSChecker {}
+
+  // This is a regression test for a crash if two Error Prone diagnostics are reported at the same
+  // position. javac has logic to filter duplicate diagnostics, which can result in counts of
+  // Error Prone and other diagnostics getting out of sync with the counters kept in
+  // ErrorProneAnalyzer, which are used to skip Error Prone checks on compilation units with
+  // underlying errors. If the checks run on compilations with underlying errors they may see
+  // null symbols.
+  @Test
+  public void processingError() {
+    compilerBuilder.report(
+        ScannerSupplier.fromBugCheckerClasses(
+            CPSChecker.class, CPSChecker2.class, GetSymbolChecker.class));
+    compiler = compilerBuilder.build();
+    Result exitCode =
+        compiler.compile(
+            new String[] {"--should-stop=ifError=FLOW", "-XDcompilePolicy=byfile"},
+            ImmutableList.of(
+                forSourceLines(
+                    "A.java",
+                    """
+                    class A {
+                      int f(int x) {
+                        return x;
+                      }
+                    }
+                    """),
+                forSourceLines(
+                    "B.java",
+                    """
+                    package test;
+                    import java.util.HashSet;
+                    import java.util.Set;
+                    class B {
+                      enum E { ONE }
+                      E f(int s) {
+                        return E.valueOf(s);
+                      }
+                    }
+                    """)),
+            ImmutableList.of());
+    assertWithMessage(outputStream.toString()).that(exitCode).isEqualTo(Result.ERROR);
+    ImmutableList<String> diagnostics =
+        diagnosticHelper.getDiagnostics().stream()
+            .filter(d -> d.getKind().equals(Diagnostic.Kind.ERROR))
+            .map(d -> d.getCode())
+            .collect(toImmutableList());
+    assertThat(diagnostics).doesNotContain("compiler.err.error.prone.crash");
+    assertThat(diagnostics).hasSize(3);
+  }
 }