JSR-305 custom nullability qualifiers

[dzharkov]

Discussion for the proposal

[sdeleuze]

See my questions and remarks about ElementType.TYPE_USE support for generic type arguments and varargs and array elements here.

[sdeleuze]

After a lot of work on Reactor side, various discussion with @dzharkov, and considering the impact of doing the same work on a project as big as Spring Framework, I think what is supported currently is not mature enough so while this is a difficult decision to make I think we are going to postpone generic type argument nullability until things are fully finished on Kotlin and IDEA sides. That's a key feature for us but it is too early, we can't go GA with current support.

Based on Reactor experience and considering Spring Framework codebase as well, please find bellow our point of view about the missing pieces and open questions.

1. We don't plan to use JSR 305 default nullability qualifiers applied to TYPE_USE because TYPE_USE scope is super wide, and even if that would allow to define only generic type arguments nullability for now that's not something we consider compatible with @NonNullApi semantics on the long run considering other tools than IDEA that could support full TYPE_USE scope and raise unwanted warnings.

2. TYPE_USE level usage is something we want to leverage for @NonNull and @Nullable via @Target, but it seems IDEA-153093 prevents proper support.

3. @NonNull and @Nullable should be supported at varargs and array element level like String @Nullable ... and String @Nullable []

4. We think that @Nullable and @NonNull at class (class Flux <@NonNull T> { }) and method TYPE_PARAMETER levels should be supported in order to be able to define a default for the whole class (or method) in order to limit the number of annotations to add. If possible and if that make sense, that would be even better to support JSR 305 default nullability qualifiers applied to TYPE_PARAMETER to make that even less verbose, and much less focused than TYPE_USE so much more useful.

5. We think Foo<@NonNull ?> should be supported and should not require Foo<? extends @NonNull Object> because while this is ok to annotate existing API, this is not ok to modify class definition even if that should be in therory equivalent + that leads to ugly Java code. Imagine if have to modify Class<?> to Class<? extends @NonNull Object> in Spring Framework, not doable really. We think when you have Foo<T extends Bar> or Foo<T super Bar>, Foo<@NonNull T extends Bar> or Foo<@NonNull T super Bar> make sense and should be interpreted like Foo<T extends @NonNull Bar> or Foo<T super @NonNull Bar> in order to match to Kotlin semantics.

6. Also maybe a strange question, but I prefer to ask it explicitly, are we sure that @TypeQualifierDefault({ElementType.METHOD, ElementType.PARAMETER}) scope could not also apply to the generic type arguments, varargs and array elements of the related parameters and return values? Note sure if this behavior would be compliant with JSR 305 spirit, but after why not, and that would be quite handy ...

We would like to discuss these 5 points that are currently the missing pieces for us to be able to complete this very important null-safety topic. We may introduce that in Reactor 3.1.x / Spring 5.0.x release when available in Kotlin and IDEA.

[dzharkov]

@sdeleuze Thank you a lot for your feedback!

[sdeleuze]

I have updated point 4 mentioning JSR 305 default nullability qualifiers applied to TYPE_PARAMETER is worth exploring (scope is more focused so more usable than TYPE_USE).

[dzharkov]

@sdeleuze We're going to update the proposal to fix issues you found

1. If @Nonnull annotation has TYPE_USE target used with varargs it must be applied to vararg element types, i.e. it should be loaded as fun foo(vararg x: String), not fun foo(vararg x: String!)
2. We're going to add @ApplyToTypeArguments meta-annotation that targets a default qualifier at all types nested in the type arguments of what it was otherwise applicable to. It means you'll be able to add it to @NotNullApi to make it applicable to type arguments even without TYPE_USE agument for @TypeQualifierDefault
3. We're going to support @A<@Nonnull ?> as an alias for @A<? extends @Nonnull>

[sdeleuze]

@dzharkov Nice, sounds like a plan! One point I would like to clarify with you is where @ApplyToTypeArguments will belong.

Spring Framework currently only depends on jsr305.jar from FindBugs for meta annotations, and this has some side effect like for example projects with code using our null-safety annotations need to use compile only jsr305.jar in order to avoid compile-time warnings. It also needs to be handled on Jigsaw or OSGI side. We carefully evaluated these impacts and are ok with it, but we would like to avoid adding an additional dependency for a single meta-annotation.

Another important aspect is that null-safety annotations are not Kotlin specific but also apply to Java developers via tooling like IDEA. I do think this is important to continue following this strategy for consistency especially for frameworks targeting Java + Kotlin like Spring

Having this 2 points in mind, and given the fact that FindBugs is not active anymore, could Jetbrains provide a JSR 305 GitHub repository and JAR containing the same annotations than FindBugs one + @ApplyToTypeArguments and eventually updated JavaDoc? That way we could just change from FindBugs to JetBrains dependency and leverage @ApplyToTypeArguments.

[dzharkov]

@sdeleuze Currently, @ApplyToTypeArguments is planned to belong to a separate artifact.
But it's compile-time only for library maintainer, since there is no need for Spring users to have it in the classpath

It's all complicated with FindBugs modification/redistributing because it has unknown licence status

[sdeleuze]

@dzharkov I understand BSD versus Apache 2 + javax.annotation related questions can be an issue.

When you create the repo and the artifact for this annotation, could you please let the door open (with a general enough name and a package not directly to kotlin for example) to later allow adding more annotations that could be used as an alternative to JSR 305 javax.annotation meta-annotations ?

[sdeleuze]

Another example why adding jsr 305 alternative annotations in this new JetBrain owned library would be welcome by the community : openzipkin/zipkin@eeeeb3c

[sdeleuze]

More details about the Java 9 related issues a JetBrain JSR 305 alternative would solve: https://blog.codefx.org/java/jsr-305-java-9/

[dzharkov]

The feature has been released in Kotlin 1.2, thus I'm closing the KEEP. Do not hesitate to open a YouTrack issue for any additional suggestions.

[sdeleuze]

@dzharkov Should I create an issue to track the release of the null-safety annotation library we discussed (with @ApplyToTypeArguments and potentially JSR 305 equivalent without the licensing/package issues) and which is still really needed?

[dzharkov]

@sdeleuze No, I will create a separate issue right here

[dzharkov]

Another discussion belongs to KT-30789