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

Replace @Nonnull(when = When.MAYBE) by @CheckForNull in @Nullable #27183

Closed
xenoterracide opened this issue Jul 15, 2021 · 16 comments
Closed

Replace @Nonnull(when = When.MAYBE) by @CheckForNull in @Nullable #27183

xenoterracide opened this issue Jul 15, 2021 · 16 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@xenoterracide
Copy link

it appears this is found in the findbugs jsr305 annotations my reason why is I want to enable -Werror and I'm using java modules module-info.java and it appears you can't use jsr305 as the automatic module name. sadly I don't actually know what When does or if there's a replacement. most tools seem to respect annotations named Nullable or NonNull now.

/home/xeno/.gradle/caches/modules-2/files-2.1/org.springframework/spring-core/5.3.8/da9b87dacaa5bbf80fad0f7b483988372a00a152/spring-core-5.3.8.jar(/org/springframework/lang/Nullable.class): warning: Cannot find annotation method 'when()' in type 'Nonnull': class file for javax.annotation.Nonnull not found
warning: unknown enum constant When.MAYBE
  reason: class file for javax.annotation.meta.When not found
error: warnings found and -Werror specified
1 error
2 warnings
❯ ./gradlew --version && ./gradlew :authn:dependencyInsight --configuration runtimeClasspath --dependency spring-core                                                                      # backend -> master + !

------------------------------------------------------------
Gradle 7.1.1
------------------------------------------------------------

Build time:   2021-07-02 12:16:43 UTC
Revision:     774525a055494e0ece39f522ac7ad17498ce032c

Kotlin:       1.4.31
Groovy:       3.0.7
Ant:          Apache Ant(TM) version 1.10.9 compiled on September 27 2020
JVM:          11.0.11 (AdoptOpenJDK 11.0.11+9)
OS:           Linux 5.10.42-1-MANJARO amd64

Type-safe dependency accessors is an incubating feature.
Type-safe project accessors is an incubating feature.

> Task :authn:dependencyInsight
org.springframework:spring-core:5.3.8
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 15, 2021
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 10, 2021
@serv-inc
Copy link

serv-inc commented May 25, 2022

Does https://stackoverflow.com/questions/61140322/jetty-run-error-java-lang-typenotpresentexception-type-javax-annotation-meta-wh help ?

    <dependency>
        <groupId>com.google.code.findbugs</groupId>
        <artifactId>jsr305</artifactId>
        <version>3.0.2</version>
    </dependency>

@sbrannen
Copy link
Member

Do you have jsr305-3.0.2.jar in the class path or module path?

it appears you can't use jsr305 as the automatic module name.

What happens when you attempt to do that?

Are you encountering issues with split packages?

@chirag-ji
Copy link

I've also getting this, but adding jsr305 worked.

I wonder why it is not working as it was available as a transitive dependency in the classpath, but working fine after adding it as 'direct dependency`. Any one please share some views on it.

@sdeleuze
Copy link
Contributor

sdeleuze commented Jul 5, 2022

This is a known side effect of current JSR 305 support, the Java compiler generates warning on annotation with enum values when not present in the classpath while it is lenient for the annotations themselves. As suggested by Sam, the workaround is to have the jsr305 in the classpath or module path.

If I am not mistaken, this only occurs when user code is using Spring null-safety annotations, so that's on purpose that we don't add jsr305 to the transitive dependencies in a mandatory way.

The middle term fix will be to switch to https://github.com/jspecify/jspecify (where Spring team is involved) and remove those JSR305 annotations from Spring ones.

@sbrannen What about creating an issue dedicated to switching from JSR 305 to JSpecify in the 6.x Backlog in order to give visibility to our users and closing this one?

@sbrannen
Copy link
Member

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2022
@sbrannen sbrannen added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 12, 2022
@cpovirk
Copy link

cpovirk commented Apr 1, 2024

I am all in favor of a migration to JSpecify, but FYI, I suspect that you can prevent this warning by changing...

...to just...

@CheckForNull

We do this for a common annotation inside Google, and it works well with Kotlin.

@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 2, 2024

Thanks for the insight, I am going to give it a try.

@sdeleuze sdeleuze changed the title remove javax.annotation.meta.When Replace @Nonnull(when = When.MAYBE) by @CheckForNull in @Nullable Apr 2, 2024
@sdeleuze sdeleuze self-assigned this Apr 2, 2024
@sdeleuze sdeleuze added type: enhancement A general enhancement and removed status: superseded An issue that has been superseded by another labels Apr 2, 2024
@sdeleuze sdeleuze added this to the 6.2.0-M1 milestone Apr 2, 2024
@sdeleuze sdeleuze reopened this Apr 2, 2024
@xenoterracide
Copy link
Author

xenoterracide commented Apr 3, 2024

curious, what problem does using meta annotations actually solve in todays world? how is that consumed? most tools that would have used it now support these things just by looking at Nonnull/Nullable, I think.

@cpovirk
Copy link

cpovirk commented Apr 3, 2024

I think that the tool that people usually care most about is Kotlin:

The meta annotations are also supported by IntelliJ when editing Java code.

And I assume that SpotBugs has inherited FindBugs's support for them.

I'd be interested in knowing of other tools that are of interest to people, too.

@xenoterracide
Copy link
Author

Yes, but spotbugs and IntelliJ also support not having those specific meta annotations. Other annotations work now. Unless you know something that I don't about how those tools work. Which is quite possible. I guess I'm just questioning on whether or not there's any tooling that requires those annotations or any part of those toolings that require those meta annotations. There's a lot of nullity annotations out there right now that don't have the meta annotations. So a lot of tools are supporting without, I believe.

It's probably worth saying that my primary nullness checker is errorprone null away. Checker framework is also pretty popular.

I think it might be worth testing the assumption that these meta annotations are still really buying you anything at all.

@cpovirk
Copy link

cpovirk commented Apr 3, 2024

I suspect that they don't buy a ton except for Kotlin.

I'd have to look into IntelliJ and SpotBugs:

  • I assume that, at minimum, they let you provide your own list of annotations to treat like @Nullable or @CheckForNull. (Of course, it's nice for the average user not to have to.) And they even might recognize any annotations with those simple class names, as tools like NullAway do.
  • I think that the defaulting annotations, like @NonNullApi, probably do require the JSR-305 annotations in order to work in IntelliJ and SpotBugs, possibly in contrast to simple @Nullable and @NonNull annotations.

@xenoterracide
Copy link
Author

xenoterracide commented Apr 3, 2024

Fair, I was just suggesting verifying that the world is still needing this for something.

Uh, If you see that very inappropriate thing show up in an email... I was speech to texting and Google translated something from my TV. Sorry I honestly want you to know I didn't say that.

@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 9, 2024

Seems to work as expected based on my tests:

  • No side effects detected by NullAway when building Spring Framework
  • Kotlin null-safety seems to be effective like before the change
  • No more unknown enum constant When.MAYBE compilation warnings

@sdeleuze sdeleuze modified the milestones: 6.2.0-M1, 6.2.x Apr 10, 2024
@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 10, 2024

I had sadly to revert the related commit as, as observed by @jhoeller, IntelliJ IDEA is not able to recognize @Nullable semantics with that change.

@cpovirk Let's maybe discuss that with JetBrains.

@sdeleuze sdeleuze reopened this Apr 10, 2024
@cpovirk
Copy link

cpovirk commented Apr 10, 2024

Oh, that's a shame :( Thanks for letting me know. I may try to switch the one we're using in Google now.

I wondered if maybe IntelliJ would do what we want if we were to use @Nullable instead of @CheckForNull, but it looks like it does not. (I'm actually not surprised by that: The JSR-305 @Nullable technically is more like "unknown nullness" than it is what people normally mean by "nullable." I don't remember offhand if Kotlin behaves the same way.)

I've filed IDEA-351380 to see if we can get support for @TypeQualifierNickname @CheckForNull. Star, vote, etc. :)

@nkonev
Copy link

nkonev commented Jun 19, 2024

According to https://youtrack.jetbrains.com/issue/IDEA-351380/Support-CheckForNull-with-TypeQualifierNickname seems that IDE's behavior is fixed in IntelliJ IDEA 2024.1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

9 participants