Explicitly indicate Nullability of arguments and return values

[k163377]

Currently nullability is not explicitly stated for Jackson.
On the other hand, there are actually APIs that do not allow null input.
Also, there are cases where inadvertent destructive changes can occur, as in the issue submitted below.
FasterXML/jackson-dataformat-xml#607

Personally, I think that Nullability annotations should be given to public interfaces to avoid such problems.
Jackson is a huge project and we cannot fix everything at once, but I think we can proceed gradually.

There are several possible annotations to use, but to minimize the impact, it is better to choose one whose RetentionPolicy is not RUNTIME.

[cowtowncoder]

The main problem here is that I really do not want to add a dependency to external annotations package. If JDK provided annotation with Java 8, I'd be happy to add those, but otherwise not.

[k163377]

Unfortunately, as far as I know, JDK does not provide such annotations.
I think adding external packages is the easiest and most effective way.

I agree that external packages should not be added thoughtlessly.
However, there are significant advantages to implementing it, both in terms of convenience and maintainability.
I also believe that the impact can be minimized if the RetentionPolicy is not RUNTIME.

Alternatively, we could introduce our own annotations, although support by the IDE would be weaker.

[cowtowncoder]

If it was possible to have combination of not-RUNTIME retention and scope of provided (so downstream would not require dependency), maybe I'd go with that.

For now I don't think I want to add these annotations here.
Will leave the issue open in case my mind changes.

[k163377]

If it was possible to have combination of not-RUNTIME retention and scope of provided

JetBrains Java Annotations seems to be able to meet this requirement.
https://mvnrepository.com/artifact/org.jetbrains/annotations
https://github.com/JetBrains/java-annotations/blob/ac4b67f071bef18abd4f9c8e7fe1a8d2a861dddf/common/src/main/java/org/jetbrains/annotations/NotNull.java#L28

I will share how I used jOOQ-meta with Intellij as an example.
https://github.com/jOOQ/jOOQ/blob/54d663bcba6a30e58c10a87a6715a6dbeb42bf16/jOOQ-meta/pom.xml#L51-L56

The return value of the getNamespace method is annotated as NotNull, but if define it to return null, a warning is displayed(not compile error).

The NotNull annotation is not bundled with the dependencies and is not available unless added.

A sample project is uploaded below (requires Java 17).
maven-java-sandbox.zip

[winfriedgerlach]

From today's perspective, the way to go seems to be JSpecify annotations. At least that seems to be what big players like Jetbrains, Google etc. have agreed upon.

[pjfanning]

From today's perspective, the way to go seem to be JSpecify annotations. At least that seems to be what big players like Jetbrains, Google etc. have agreed upon.

As specified above, we do not want to add jar dependencies to jackson-core. In theory, we could add a separate module that has the jspecify dependency. I practice, we would probably also need changes in jackson-databind to integrate with this new module. I suspect that this issue is not related to jackson-core - it still remains the case that we don't want jackson-databind to have 3rd party lib dependencies either.

[cowtowncoder]

Yeah, if JDK version we required had such annotation it'd be good idea. But otherwise, I don't want addition. And adding new module just for tiny bit of metadata that tooling may or may not use does not sound very tempting either.

Will leave open so no one re-files request. :)

[k163377]

And adding new module just for tiny bit of metadata that tooling may or may not use does not sound very tempting either.

These annotations are very important information for developers.

Mistaking nullability can cause bugs.
In fact, the bug I shared at the beginning happened because the nullability was not made explicit.
I also remember seeing a comment in the databind code that it was checking for null because it didn't know the nullability (sorry, I can't find the actual comment).

Just having the nullability explicitly stated by annotations prevents bugs and avoids writing extra processing.
Even if we focus only on Jackson maintainers, I think there is enough benefit in making the nullability explicit.

Of course, I use Kotlin, so it is also convenient for me to inherit content from Jackson in Kotlin.

[cowtowncoder]

If there were just Javadoc annotations without need to have an extra dependency, I'd be +1. But I am strictly against adding a new annotation-only dependency.

I am also +1 for indicating in Javadocs nullability (in descriptions) of arguments, return values.

[lobaorn-bitso]

Hi @cowtowncoder @k163377 , any change of opinion regarding this, especially for Jackson 3?

My personal feeling is that everything done already and being done this year (https://github.com/FasterXML/jackson/wiki/Jackson-Release-3.0) is pushing Jackson 3 to be at the ecosystem "state-of-the-art", considering things like: Java 17 baseline, full JPMS compatibility, SBOM...

Considering that, I believe that the only "remaining" topic would be to address Nullability with JSpecify+ErrorProne+NullAway. Spring was the biggest library that announced that in their next major release this year, they will use it in their versions for Framework 7 and Boot 4. And the most recent example of a pervasive framework moving to use JSpecify extensively was JUnit: junit-team/junit-framework#4557 . JUnit also has full JPMS compatibility like Jackson, unlike Spring.

Also, other libraries are following the idea for their newest major, like AssertJ: assertj/assertj#3727

I am not sure if this is sufficient to change an opinion or move the needle in Jackson, but I thought it was a good idea to make this suggestion for a possible JSTEP-15.

[cowtowncoder]

@lobaorn-bitso If you have access to modify Wiki, please go ahead and create JSTEP-15 -- and then a Poll Discussion (on jackson-future-ideas)? I think it would make sense to discuss, and I am willing to follow "Vox Populi" if others feel this is something that should be done.

[lobaorn-bitso]

Thanks @cowtowncoder I created the Discussion/Poll since I do not have the permissions to change the wiki. Here it is: FasterXML/jackson-future-ideas#87