search

FasterXML / jackson-core

Core part of Jackson that defines Streaming API as well as basic shared abstractions
Apache License 2.0
2.26k stars 792 forks source link

Explicitly indicate Nullability of arguments and return values #1105

Open k163377 opened 1 year ago

k163377 commented 1 year ago

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. https://github.com/FasterXML/jackson-dataformat-xml/issues/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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). image

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

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