Nullability annotations in ORM code base

[beikov]

I'd like to make sure the Hibernate ORM code base makes use of @Nullable or @NotNull (depending on what we decide for our default) for a few reasons:

• It's nicer for users to understand and use our APIs
• It's nice for us to catch possible bugs early
• It improves interop with languages like Kotlin
• It's a preparation for how the Java language model will most likely change for project Valhalla

The last point is actually what finally drove me to create this discussion. There are talks about exposing value storage optimizations to the Java language through the mechanism of nullability markers on parameters, return types, fields and local variables. It kind of makes sense that flat storage requires the type to be non-nullable. There are tricks the JVM can apply to encode nullability in unused bits, but the best possible performance can only be achieved by actually making sure that something is non-nullable.

Valhalla is still far away, but since the introduction of such annotations in a code base like ORM is not an easy undertaking and will take some time, I'm starting a discussion now, so that we can start working on this as soon as we decided on the approach. Nullability information will help us anyway. I guess we just shied away from tackling this because it is probably going to be a long process.

Here are a few questions to discuss:

1. Do we want our code base to be non-null by default or nullable by default?
2. Which annotation library should we use? Kotlin docs have a nice overview: https://kotlinlang.org/docs/java-interop.html#nullability-annotations
3. Should we use a tool like jspecify to make sure nullability is enforced at runtime?
4. How to handle generated code like JAXB models, since that is usually nullable by default?
5. Should we migrate package by package or infer annotations (https://www.jetbrains.com/help/idea/inferring-nullity.html) and fixup issues?

Here is my take on it:

1. My preference is to use non-null by default. I believe most stuff should be non-null and that nullable is the exception which should be documented in Javadoc.
2. I prefer the JSpecify annotations because it seems that is where the industry is going to. Interop with Kotlin is automatic since version 1.5.20. IntelliJ should automatically detect the annotations, so less config for ORM devs. The retention of the annotations is RUNTIME, but as far as I understood from a lengthy discussion, we can still use a compileOnly dependency without causing trouble, so no new dependency for users.
3. Yes, IMO we should enforce non-nullability at runtime as soon as we finished annotating the code base by configuring a plugin/annotation-processor.
4. Since JSpecify allows marking a package-info.java as @NullUnmarked, we can default to non-null by default and exclude certain packages from that default.
5. IMO it is easiest to start off with inferring the annotations and the fixup stuff package by package in separate PRs. We don't have to enable the runtime enforcement from the start, so there should not be any problem doing this.

[gavinking]

Do we want our code base to be non-null by default or nullable by default?

Not-null by default. If we're going to do it, do it properly.

• It's nicer for users to understand and use our APIs
• It's nice for us to catch possible bugs early
• It improves interop with languages like Kotlin

So the first and third items here only call for adding nullability annotations to out APIs, and don't require us to spray them over the whole codebase, which is a much bigger job, and I'm sort-of a bit skeptical that we will ever "finish" such a huge undertaking.

[yrodiere]

the first and third items here only call for adding nullability annotations to out APIs

I think the idea is that in order to check (through static analysis) that nullable elements in APIs are handled null-safely in internals, we need to annotate the whole codebase... Because we don't necessarily handle null-ness directly at API boundaries, and also because static analysis would probably return tons of unrelated false positives for parts we didn't annotate properly.

I'm sort-of a bit skeptical that we will ever "finish" such a huge undertaking.

+1 here. As much as I like the idea, it does look like a never-ending task.

[yrodiere]

That wasn't clear: I meant putting @Nullable annotations on our APIs is kind of useless if we don't have automated checks to assert:

1. That @Nullable elements are actually nullable (tests would work for that, but...)
2. That non-@Nullable elements are not just things we forgot to annotate (if there's a null-check on a non-@Nullable element, there's probably something wrong)

[beikov]

Ok, so we decided to use JSpecify annotations and try to work through package by package. Created https://hibernate.atlassian.net/browse/HHH-16389

[beikov]

Since JSpecify is still in development and there is no checker plugin yet for it, I opted to use the checkerframework for now.