Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

false positive non null return with gradle and jetbrains annotations #1123

Open
xenoterracide opened this issue Jan 6, 2025 · 2 comments
Open

Comments

@xenoterracide
Copy link

xenoterracide commented Jan 6, 2025

/home/xeno/IdeaProjects/untitled/src/main/java/org/example/Main.java:10: error: [NullAway] returning @Nullable expression from method with @NonNull return type

it's talking about the map lambda

    project.getObjects().property(String.class).map( s -> null);

but the gradle implementation very clearly allows a nullable return.

copypasta of gradles source

public interface Transformer<OUT extends @org.jetbrains.annotations.Nullable Object, IN> {
    OUT transform(IN in);
}
tasks.withType<JavaCompile>().configureEach {
  options.errorprone {
    option("NullAway:AnnotatedPackages", listOf("com", "org", "net", "io", "dev", "graphql").joinToString(","))
    error("NullAway")
  }
}

bug.tar.gz

reproducer uses dynamic versions, so here's the at time of writing resolutions that I think are important.

  id("net.ltgt.errorprone") version "4.1.0"
+--- com.google.errorprone:error_prone_core:2.+ -> 2.36.0
\--- com.uber.nullaway:nullaway:0.+ -> 0.12.2

afaik, gradle api is comprehensively annotated with non null as the default. It's used by kotlin consumers with null safety in mind.

@xenoterracide xenoterracide changed the title incorrect non null return with gradle and jetbrains annotations false positive non null return with gradle and jetbrains annotations Jan 6, 2025
@msridhar
Copy link
Collaborator

msridhar commented Jan 6, 2025

Will take me a bit of time to dig into this one. There are some places where we initially special-cased JSpecify annotations, and I think that was a mistake; we should just accept any @Nullable annotation like we do elsewhere in NullAway, until we see it causing problems. So we can fix that.

But beyond that, I think we will need proper support for inference on generic methods (#1075) before we can support code like this. In this case I assume map is a generic method, and we need to infer that it's ok to treat its output type as @Nullable.

@xenoterracide
Copy link
Author

it is, here's the source for that https://github.com/gradle/gradle/blob/5cb81304ee750fd32c943fefba19b5387000d226/subprojects/core-api/src/main/java/org/gradle/api/provider/Provider.java#L127

note: worth saying that I believe that gradle will be moving to jspecify in version 9, there's a ticket related to that ... somewhere, linked in another ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants