-
Notifications
You must be signed in to change notification settings - Fork 678
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
NulllableUtils only considers javax.annotation nullability annotations, should it also consider jakarta.annotation nullability annotations? #3100
Comments
TL;DR: Supporting JSR-305 annotations ( Annotated elements in Java do not cause class loading issues if annotations aren't found, instead, annotations not available during runtime are just invisible to the reflection API. That is the source of why we fall back to Spring's annotations if JSR-305 isn't on the class path. We do not intend adding class-file parsing as such an approach would resort in many other issues that we would have to solve and added complexity. If you want to evaluate nullability rules for bare JSR-305-annotated elements, then JSR-305 classes must be present during runtime. I do not agree that our nullable utils are a harm. Instead, the entire package migration from We do not intend to be a holistic facility for all possible null annotations. We merely take advantage from some annotations being present in a natural consequence. Each framework comes with its own, slightly adjusted semantics, there are efforts to create yet-another-non-null annotation framework. That being said, this is a problem to be solved in a different place while we're dogfooding based on Reactor and Spring annotations. |
I want to make sure I understand your position. Is it fair to rephrase your position as the following? "Even if you upgrade your project to SB 3, Spring6, and Hibernate 6, and want to lean into the jakarta nullability annotations, if you happen to have the |
Yeah, that is fair to say. Going forward, did you file a ticket against Spring Framework to support Jakarta nullability annotations? |
Forgive me, but hasn't that ship already sailed?
Doesn't the "namespace change" cover this? Someone playing devil's advocate would point to this.
But I would argue the meta-annotations are to support findbugs/spotbugs tooling. The meta-annotations do not mean that JSR305 Am I making any sense? |
We discussed nullability annotation coverage in the team. With JSpecify on the horizon, it makes sense to revisit our nullability arrangement. The growth in nullability annotations requires on one hand a more flexible model, on the other hand, we cannot support each and every nullability annotation. A nuanced path forward is to support:
|
FYI JSpecify 1.0.0 has been released. |
Greetings. I am upgrading from Spring Boot 2.7 to Spring Boot 3. That means I'm going from
spring-data-commons:2.7.18
tospring-data-commons:3.2.5
. Additionally this upgrade is bringing along a Hibernate 5.4 to 6.4.4 migration. Hibernate 6+ is all in onjakarta.annotation
stuff.In fact the automation my company wrote to help with the upgrade generously refactored all usages in my repo of
javax.annotation
tojakarta.annotation
.However, now a JPA query class I have is making
org.springframework.data.repository.core.support.MethodInvocationValidator
sad because the method is annotated withjakarta.annotation.Nullable
but NullableUtils does evaluate that annotation when deciding whether something is or is not documented as "nullable".Here are some questions to guide this discussion:
org.springframework.lang.Nullable
,reactor.util.lang.NonNullApi
, andrg.springframework.lang.NonNullApi
if it can't findjavax.annotation.Nullable
. Is it fair to say "this strategy could be improved upon"?a. Evaluate what nullability annotations are on the classpath (javax, jakarta, spring, reactor, spotbugs, findbugs, intellij, etc)
b. When attempting to evaluate if a given parameter or return type are nullable, consult all annotations that are present on the method or parameter in a deterministic order, if any are present, use them.
c. bonus points: if there is a disagreement in the annotations (e.g. both Nullable and Nonnull) throw an exception
I look forward to hearing back from you
The text was updated successfully, but these errors were encountered: