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.