spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

Migrate to JSpecify annotations for nullability constraints

Open sbrannen opened this issue 3 years ago • 11 comments
trafficstars

Overview

Once JSpecify releases a relatively stable (potentially beta) version, we should migrate to JSpecify annotations for nullability constraints.

The previous plan was to meta-annotate annotations in the org.springframework.lang package with JSpecify annotations alongside the JSR-305 annotations, but JSpecify won't provide such meta annotations for various reasons. So the new plan is to leverage directly JSpecify annotations and deprecate Spring Framework null-safety annotations.

Related Issues

  • #27183

sbrannen avatar Jul 12 '22 12:07 sbrannen

An interesting related blog post by Meta.

An important improvement would be IMO to update the Spring Framework build to check null-safety at build-time, not just rely on IntelliJ IDEA warnings, and potentially to publish guidelines for Spring portfolio projects and Spring applications that want to do the same.

sdeleuze avatar Nov 22 '22 17:11 sdeleuze

In addition to the change of artifact and meta-annotations, the switch to jSpecify is expected to allow Spring to refine the following aspects of null-safety:

  • Support null-safety at generics, varargs and array elements levels (to be checked depending on which jSpecify version we leverage).
  • Better tooling integration, with a build-time check in Spring Framework Gradle build.
  • Prevent the unknown enum constant When.MAYBE compile-time warning when Spring null-safety annotations are used without JSR 305 available in the classpath.

sdeleuze avatar Jun 12 '23 13:06 sdeleuze

usage of jsr305 jar breaks JPMS. So requires monkey patching to deal with this if -Werror is used. and then makes the build also incompatible with javax annotations-api (v1). Although I'm sure people here are well aware of the issues around the 3? jars... I'd hope.

xenoterracide avatar Feb 13 '24 21:02 xenoterracide

I have dropped a comment in https://github.com/jakartaee/common-annotations-api/issues/124#issuecomment-1943490978 as it seems not realistic to me to just drop a bunch of annotations there, this topic is much more complex than that.

Hey, Spring Framework committer here. I am not sure that would be as simple as adding those 2 additional annotations since:

  • Spring use case is leveraging meta annotations
  • The scope where null-safety applies needs to be specified, in JSR 305, we use @TypeQualifierDefault
  • Annotation attributes like When.MAYBE need to be expressed in another way in order to avoid warnings from javac
  • Need to have specifications and tooling support

In a nutshell, going beyond simple @Nonnull and @Nullable annotations is a huge task that is much more involved that it seems, as proven in https://github.com/jspecify/jspecify related work.

@xenoterracide Could you please give a try to JSpecify 0.3.0 on a sample application and see if it fulfils your JPMS needs (mostly to provide a feedback to the JSpecify team)?

Myself I plan to experiment with a Spring Framework branch using JSpecify annotations instead of JSR 305 to provide also feedback to JSpecify as they need that for an upcoming release.

sdeleuze avatar Feb 14 '24 10:02 sdeleuze

It looks like meta-annotation capabilities have been removed from JSpecify 1.0 so using them to meta annotate Spring annotations will not be possible.

The other issue is that the "annote package to set the default to non-null" works differently in JSpecify than with JSR 305. It is expressed with the @NullMarked annotation which does not allow to differentiate the scope where those defaults are applied (while Spring allow to target APIs and field distinctly). I need to have a deeper look to @NullMarked to understand what would be the best path to move forward.

sdeleuze avatar Mar 27 '24 17:03 sdeleuze

They have decided that for version 1.0 they aren't covering lazy initialized fields. So for situations like JPA it is not a complete solution. This message is simply an FYI.

xenoterracide avatar Mar 27 '24 22:03 xenoterracide

JSpecify 1.0.0 has been released. It does not provide meta annotation / implication capabilities yet so we will probably need to wait for JSpecify 1.1 to be able to leverage it in Spring.

sdeleuze avatar Jul 18 '24 09:07 sdeleuze