fail if there any problems are detected, but report all problems found

[donalmurtagh]

I would like Error Prone to fail if it encounters any problems, but report all problems found. However it seems only the following are possible

(A) When problems are reported as warnings

1. Problems do not cause the build to fail
2. Problems in all files are reported by the build

(B) When problems are reported as errors

1. Any file found with problems causes the build to fail
2. Only the problems in this file are reported. A consequence of failing the build when the first problematic file is encountered is that it's impossible to report problems in any other file.

The behaviour I want is A2 and B1, but this does not seem to be supported. When the flag -Werror is present, Error Prone behaves in mode (B), otherwise it behaves as per (A).

I want all errors/warnings to fail the build in order to force people to fix them. However, if there are a large number of problems in the project (e.g. because Error Prone has just been introduced), it's tedious to discover them on a file-by-file basis.

I'm using Error Prone v2.30.0 with Gradle.

Relevant Issues

• https://github.com/google/error-prone/issues/436
• https://github.com/google/error-prone/issues/424

Example Application

I've create a sample application that reproduces the problem. Instructions are provided in this comment

[cpovirk]

I'm not on the Error Prone team proper, but as someone who claimed that this was probably fixed on the issues you cited, I should probably say something :)

Do you have an example project that demonstrates this? One thing that we've seen is that the exact behavior depends a lot on the specific build system. I have seen errors reported for multiple files in our internal Bazel integration and in Maven (like for this set of problems, for which I receive two errors in the same build), so it's possible that the build system you're using isn't covered by the fixes so far, or it's possible that upgrading some component (build system, compiler, Error Prone) could help.

[cpovirk]

Oh, sorry, 2.30.0 is at least not the problem, then :)

[cpovirk]

Actually, sorry again, I do have a trivial Gradle project lying around, too. I can introduce files in multiple problems there and see multiple errors in one build, too. (That's with Error Prone 2.20.0 because somehow I get a circular-dependencies error if I upgrade to 2.30.0? Ah, maybe it's because I'm building JSpecify itself, which is a dependency of Error Prone? https://github.com/jspecify/jspecify/issues/604)

[donalmurtagh]

@cpovirk I've created an example app that demonstrates the problem with Error Prone v2.30.0. If you check out the error-prone branch of this repo, then (using JDK v21) run

./gradlew compileJava

The build fails, but only reports an error in MissingNullableAnnotations.java. If you comment out the following line in build.gradle

options.compilerArgs << "-Werror"

then run ./gradlew compileJava again, the build doesn't fail, but it reports error in all (2) files. I want the build to fail, but report errors in all files.

[cpovirk]

Thanks!

It looks like you can get errors reported in multiple files if you tell Error Prone to produce errors:

--- a/build.gradle
+++ b/build.gradle
@@ -33,6 +33,6 @@ tasks.named('compileJava') {
        options.errorprone {
                // NullAway configuration options: https://github.com/uber/NullAway/wiki/Configuration
                option("NullAway:AnnotatedPackages", "com.example.demo")
+               errorproneArgs.add("-Xep:NullAway:ERROR")
        }
-       options.compilerArgs << "-Werror"
 }

> Task :compileJava FAILED
/tmp/tmp.7PN88j83Nf/spring-boot-demo/src/main/java/com/example/demo/DemoApplication.java:10: error: [NullAway] passing @Nullable parameter 'null' where @NonNull is required
                new MissingNullableAnnotations(null);
                                               ^
    (see http://t.uber.com/nullaway )
/tmp/tmp.7PN88j83Nf/spring-boot-demo/src/main/java/com/example/demo/MissingNullableAnnotations.java:9: error: [NullAway] returning @Nullable expression from method with @NonNull return type
        return null;
        ^
    (see http://t.uber.com/nullaway )
2 errors

That's interesting that -Werror produces different results. In particular, I notice that -Werror -Xlint:rawtypes can report warnings-turned-into-errors from multiple files. It's possible that Error Prone would need to do some additional work in order to make that possible. I don't know how doable that is.

Would the approach of requesting errors from Error Prone work in your case? (I don't know that there's an easy way to upgrade all the default warnings to errors (a hypothetical -XepAllWarningsAsErrors), though. You could at least eliminate warnings with -XepDisableAllWarnings, but then you might want to reenable some of those as errors.)

[donalmurtagh]

Would the approach of requesting errors from Error Prone work in your case?

Not really, because there are too many bug patterns that emit warnings by default for it to practical to upgrade them all to errors one-by-one

I don't know that there's an easy way to upgrade all the default warnings to errors (a hypothetical -XepAllWarningsAsErrors)

This hypothetical flag to promote warnings to errors is what I'm asking for. There is a flag to downgrade errors to warnings, so it's surprising that the opposite doesn't seem to be possible

[cushon]

There was some related discussion to -XepAllWarningsAsErrors in #1589 and #648.

I wondered if setting javac's --should-stop= policy to a later phase would help but it doesn't. I think the culprit is this logic in Error Prone that tries to stop analysis after any javac diagnostics are reported, since we don't want to try to run checks if there are underlying compilation errors:

https://github.com/google/error-prone/blob/90d939069d5b59cc404da5ac48b25509b2ebef40/check_api/src/main/java/com/google/errorprone/ErrorProneAnalyzer.java#L194

But here Error Prone reports a diagnostic, and that causes javac to report a werror diagnostic, and Error Prone stops:

https://github.com/openjdk/jdk/blob/3f00da84b3e6fb001e7d56acb198292b28d40c8b/src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java#L590-L594

I checked that hacking around that causes more diagnostics to be produced:

--- a/check_api/src/main/java/com/google/errorprone/ErrorProneAnalyzer.java
+++ b/check_api/src/main/java/com/google/errorprone/ErrorProneAnalyzer.java
@@ -191,7 +191,7 @@ public class ErrorProneAnalyzer implements TaskListener {
     if (taskEvent.getKind() != Kind.ANALYZE) {
       return;
     }
-    if (JavaCompiler.instance(context).errorCount() > errorProneErrors) {
+    if (JavaCompiler.instance(context).errorCount() > errorProneErrors + 1) {
       return;
     }
     TreePath path = JavacTrees.instance(context).getPath(taskEvent.getTypeElement());

diff --git a/build.gradle b/build.gradle
index 75cddee..0f55925 100644
--- a/build.gradle
+++ b/build.gradle
@@ -13,13 +13,14 @@ java {
 }

 repositories {
+       mavenLocal()
        mavenCentral()
 }

 dependencies {
        implementation "org.jspecify:jspecify:1.0.0"
        errorprone "com.uber.nullaway:nullaway:0.11.2"
-       errorprone "com.google.errorprone:error_prone_core:2.30.0"
+       errorprone "com.google.errorprone:error_prone_core:1.0-HEAD-SNAPSHOT"

        implementation 'org.springframework.boot:spring-boot-starter-web'
        testImplementation 'org.springframework.boot:spring-boot-starter-test'
@@ -35,4 +36,7 @@ tasks.named('compileJava') {
                option("NullAway:AnnotatedPackages", "com.example.demo")
        }
        options.compilerArgs << "-Werror"
+       options.compilerArgs << "--should-stop=generate"
+       options.compilerArgs << "-XDcompilePolicy=simple"
+       options.compilerArgs << "-XDverboseCompilePolicy"
 }

./gradlew compileJava

> Task :compileJava FAILED
[attribute com.example.demo.MissingNullableAnnotations]
[attribute com.example.demo.DemoApplication]
[flow com.example.demo.MissingNullableAnnotations]
/usr/local/google/home/cushon/src/spring-boot-demo/src/main/java/com/example/demo/MissingNullableAnnotations.java:9: warning: [NullAway] returning @Nullable expression from method with @NonNull return type
        return null;
        ^
    (see http://t.uber.com/nullaway )
error: warnings found and -Werror specified
/usr/local/google/home/cushon/src/spring-boot-demo/src/main/java/com/example/demo/DemoApplication.java:10: warning: [NullAway] passing @Nullable parameter 'null' where @NonNull is required
                new MissingNullableAnnotations(null);
                                               ^
    (see http://t.uber.com/nullaway )
1 error
2 warnings

[donalmurtagh]

@cushon Thanks for the investigation. To be clear about my requirements:

1. The build should fail if error prone detects any problems (warnings or errors).
2. All problems in the project should be reported

Specifically, I don't really care whether the problems are reported as errors or warnings as long as the build fails.

Instead of upgrading warnings to errors, would it be easier from an implementation point-of-view to add a flag -XepFailOnAnyWarning?

[codeleverage]

@donalmurtagh I've got similar requirements for a Maven project. Here's what I did as a workaround:

• Use the -XepAllErrorsAsWarnings Error Prone flag
• Conditionally set -Werror by using the failOnWarning property of maven-compiler-plugin
• Set maven.compiler.failOnWarning=true by default

Example:

<properties>
  <maven.compiler.failOnWarning>true</maven.compiler.failOnWarning>
</properties>

...

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-compiler-plugin</artifactId>
  <configuration>
    <release>${java.release}</release>
    <showWarnings>true</showWarnings>
    <compilerArgs combine.children="append">
      <arg>-XDcompilePolicy=simple</arg>
      <arg>-Xplugin:ErrorProne -XepDisableWarningsInGeneratedCode -XepAllErrorsAsWarnings</arg>
      <arg>--should-stop=ifError=FLOW</arg>
    </compilerArgs>
  </configuration>
</plugin>

Normal builds, including CI servers, can run mvn verify (replace with your usual command) and the build will fail on the first file that has a warning.

Developers can run mvn verify -Dmaven.compiler.failOnWarning=false locally. The build may succeed but warnings across all files will be reported.

I don't have much experience with Gradle but I think the concepts should translate over.