uber / NullAway

A tool to help eliminate NullPointerExceptions (NPEs) in your Java code with low build-time overhead
MIT License
3.63k stars 293 forks source link

Detected NPE causes EP's Var rule to go nuts. #923

Closed xenoterracide closed 1 week ago

xenoterracide commented 7 months ago

maybe this is an issue with errorprone, I'll cross report. None of these @Var's are not effectively final. this.git is variable though.

Full source here https://github.com/xenoterracide/gradle-semver/blob/external/bug/ep/buildSrc/src/main/kotlin/our.javacompile.gradle.kts

/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/AbstractGitService.java:28: error: [NullAway] dereferenced expression this.git is @Nullable
    this.git.checkout();
            ^

> Task :compileJava
    (see http://t.uber.com/nullaway )
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:15: error: [Var] Non-constant variable missing @Var annotation
  DelegatingSystemReader(SystemReader reader) {
                                      ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'DelegatingSystemReader(@Var SystemReader reader) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:25: error: [Var] Non-constant variable missing @Var annotation
  public String getenv(String variable) {
                              ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public String getenv(@Var String variable) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:30: error: [Var] Non-constant variable missing @Var annotation
  public String getProperty(String key) {
                                   ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public String getProperty(@Var String key) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:35: error: [Var] Non-constant variable missing @Var annotation
  public FileBasedConfig openUserConfig(Config parent, FS fs) {
                                               ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public FileBasedConfig openUserConfig(@Var Config parent, FS fs) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:35: error: [Var] Non-constant variable missing @Var annotation
  public FileBasedConfig openUserConfig(Config parent, FS fs) {
                                                          ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public FileBasedConfig openUserConfig(Config parent, @Var FS fs) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:[40](https://github.com/xenoterracide/gradle-semver/actions/runs/8065969734/job/22032976374#step:4:41): error: [Var] Non-constant variable missing @Var annotation
  public FileBasedConfig openSystemConfig(Config parent, FS fs) {
                                                 ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public FileBasedConfig openSystemConfig(@Var Config parent, FS fs) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:40: error: [Var] Non-constant variable missing @Var annotation
  public FileBasedConfig openSystemConfig(Config parent, FS fs) {
                                                            ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public FileBasedConfig openSystemConfig(Config parent, @Var FS fs) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:[45](https://github.com/xenoterracide/gradle-semver/actions/runs/8065969734/job/22032976374#step:4:46): error: [Var] Non-constant variable missing @Var annotation
  public FileBasedConfig openJGitConfig(Config parent, FS fs) {
                                               ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public FileBasedConfig openJGitConfig(@Var Config parent, FS fs) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:45: error: [Var] Non-constant variable missing @Var annotation
  public FileBasedConfig openJGitConfig(Config parent, FS fs) {
                                                          ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public FileBasedConfig openJGitConfig(Config parent, @Var FS fs) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/DelegatingSystemReader.java:[55](https://github.com/xenoterracide/gradle-semver/actions/runs/8065969734/job/22032976374#step:4:56): error: [Var] Non-constant variable missing @Var annotation
  public int getTimezone(long when) {
                              ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public int getTimezone(@Var long when) {'?
/home/runner/work/gradle-semver/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/ExceptionTools.java:10: error: [Var] Non-constant variable missing @Var annotation
  static RuntimeException rethrow(Throwable e) {
                                            ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'static RuntimeException rethrow(@Var Throwable e) {'?

https://github.com/google/error-prone/issues/4305

aopalliance:aopalliance:1.0=annotationProcessor,errorprone,testAnnotationProcessor
com.github.ben-manes.caffeine:caffeine:3.0.5=annotationProcessor,errorprone,testAnnotationProcessor
com.github.kevinstern:software-and-algorithms:1.0=annotationProcessor,errorprone,testAnnotationProcessor
com.google.auto.service:auto-service-annotations:1.0.1=annotationProcessor,errorprone,testAnnotationProcessor
com.google.auto.value:auto-value-annotations:1.9=annotationProcessor,errorprone,testAnnotationProcessor
com.google.auto:auto-common:1.2.2=annotationProcessor,errorprone,testAnnotationProcessor
com.google.code.findbugs:jsr305:3.0.2=annotationProcessor,checkstyle,compileClasspath,errorprone,spotbugs,spotless1659553931,testAnnotationProcessor,testCompileClasspath
com.google.errorprone:error_prone_annotation:2.25.0=annotationProcessor,errorprone,testAnnotationProcessor
com.google.errorprone:error_prone_annotations:2.25.0=annotationProcessor,compileClasspath,errorprone,testAnnotationProcessor
com.google.errorprone:error_prone_check_api:2.25.0=annotationProcessor,errorprone,testAnnotationProcessor
com.google.errorprone:error_prone_core:2.25.0=annotationProcessor,errorprone,testAnnotationProcessor
com.google.errorprone:error_prone_type_annotations:2.25.0=annotationProcessor,errorprone,testAnnotationProcessor
com.google.guava:failureaccess:1.0.1=annotationProcessor,checkstyle,errorprone,spotless1659553931,testAnnotationProcessor
com.google.guava:guava-parent:32.1.1-jre=annotationProcessor,errorprone,testAnnotationProcessor
com.google.guava:guava:32.1.1-jre=annotationProcessor,errorprone,testAnnotationProcessor
com.google.inject:guice:5.1.0=annotationProcessor,errorprone,testAnnotationProcessor
com.google.protobuf:protobuf-java:3.19.6=annotationProcessor,errorprone,testAnnotationProcessor
com.uber.nullaway:nullaway:0.10.23=annotationProcessor,errorprone,testAnnotationProcessor
io.github.eisop:dataflow-errorprone:3.41.0-eisop1=annotationProcessor,errorprone,testAnnotationProcessor
io.github.java-diff-utils:java-diff-utils:4.12=annotationProcessor,errorprone,spotless1659553931,testAnnotationProcessor
javax.inject:javax.inject:1=annotationProcessor,errorprone,testAnnotationProcessor
org.checkerframework:checker-qual:3.33.0=annotationProcessor,errorprone,testAnnotationProcessor
org.checkerframework:dataflow-nullaway:3.40.0=annotationProcessor,errorprone,testAnnotationProcessor
org.pcollections:pcollections:4.0.1=annotationProcessor,errorprone,testAnnotationProcessor
msridhar commented 7 months ago

I can't reproduce this. I cloned the repo and checked out the external/bug/ep branch, and this is what I see:

$ ./gradlew compileJava --console=plain
Reusing configuration cache.

> Task :compileJava FAILED
/private/tmp/gradle-semver/src/main/java/com/xenoterracide/gradle/semver/AbstractGitService.java:28: error: [NullAway] dereferenced expression this.git is @Nullable
    this.git.checkout();
            ^
    (see http://t.uber.com/nullaway )
1 error

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileJava'.
> Compilation failed; see the compiler error output for details.

* Try:
> Run with --info option to get more log output.

BUILD FAILED in 1s
1 actionable task: 1 executed
Configuration cache entry reused.

I was building using JDK 17.

xenoterracide commented 7 months ago

CI reproduces it, tests are set to require java 21, while compile is 11.

https://github.com/xenoterracide/gradle-semver/actions/runs/8066098577/job/22033383615

msridhar commented 7 months ago

Interesting. I cannot repro locally on a Mac building with JDK 11, 17, or 21. Not sure what's happening. In any case I expect the issue is with the Var check and not with NullAway.

xenoterracide commented 7 months ago

hmm... I run linux locally, and so does the builder... I wonder if it could secretly have to do with Java on Linux... any chance you have the ability to try that? I sadly do not have access to a Mac.

msridhar commented 7 months ago

hmm... I run linux locally, and so does the builder... I wonder if it could secretly have to do with Java on Linux... any chance you have the ability to try that? I sadly do not have access to a Mac.

Maybe you can spin up a Mac in your CI config to see if there is a difference? Unfortunately I don't have time to try on Linux right now, don't have a box handy.

msridhar commented 3 months ago

@xenoterracide do you think we can close this one now? Still don't see how this is an issue in NullAway

xenoterracide commented 3 months ago

🤷🏻‍♂️ All I'll say is I don't feel like anybody has tried to figure out where the problem is... I can't Tell error prone where the problem is can you? Seems like this is going to be a point-the-finger issue... Leaving me frustrated without a solution.

msridhar commented 3 months ago

I can leave this open, but absent a consistent way to reproduce it's hard to dig further.

xenoterracide commented 3 months ago

I feel like you're asking me to spend money on a Mac VM... Whereas Linux VMS are free... I suppose if you want I could give you a docker container? I presume you can run docker which is a Linux VM...

msridhar commented 3 months ago

If you could create a docker container under which this repros that would be helpful. If that's a pain, if you can give the output of ./gradlew --version on a machine where this repros, I can see if there's anything that jumps out at me.

xenoterracide commented 1 week ago

So, still haven't gotten around to this, but I saw this issue yesterday with an UnusedVariable... so I'm going to close anyways. I wish it wasn't always in the middle of more complicated things.