Migrate persistence/nosql to JSpecify null annotations#4130
Migrate persistence/nosql to JSpecify null annotations#4130viditochani wants to merge 1 commit intoapache:mainfrom
Conversation
Replace jakarta.annotation.Nullable/Nonnull with JSpecify equivalents (org.jspecify.annotations.Nullable/NonNull) across the persistence/nosql module. JSpecify annotations support TYPE_USE targeting, enabling null annotations on generic type parameters and resolving Eclipse JDT compatibility issues. Notable non-mechanical changes: - Array annotations use TYPE_USE syntax (byte @nullable [] instead of @nullable byte[]) so Immutables correctly treats them as optional fields - Inner class type annotations placed on the inner type (Outer.@nonnull Inner) per TYPE_USE rules - Removed workaround dual-imports in AllRetained.java and AddressResolver.java that previously mixed Jakarta and JSpecify Fixes apache#4124
snazy
left a comment
There was a problem hiding this comment.
Thanks @viditochani for your contribution.
This is quite a big change, have you actually hit the Eclipse JDT issue yourself, or did you find it while reading the code? I'm curious what your workflow looks like, because I've been using IntelliJ and never noticed this.
Also, when you ran the tests, did anything surprising come up, or was it really as mechanical as it looks?
viditochani
left a comment
There was a problem hiding this comment.
Thanks! Yeah, I ran into the Eclipse JDT issue myself. I use VSCode with the Java extension (Eclipse JDT under the hood) and it was flagging errors that javac doesn't care about.
Most of it was mechanical, but the tests caught a real difference. Four test fixtures had @Nullable byte[] binary(), which works fine with Jakarta since the annotation targets the method. With JSpecify's TYPE_USE targeting, @Nullable byte[] annotates the element type byte instead of the array, so Immutables stops treating the field as optional. Had to change it to byte @Nullable [] binary().
So not purely mechanical, but the existing test suite caught the subtle stuff.
|
@snazy - any further feedback on this, or can we merge? |
flyrain
left a comment
There was a problem hiding this comment.
Given that a few others moved to JSpecify as well, the change is fine to me.
- Frameworks like Spring Framework 7 / Spring Boot 4 have moved to JSpecify
- Libraries like Guava are adopting it
- IntelliJ IDEA (2025.3+) treats it as the preferred nullability source when available
The only concern is whether JSpecify brings in other dependencies, which potentially pollute the dep tree. It seems fine per my understanding.
viditochani
left a comment
There was a problem hiding this comment.
AFAIK, JSpecify is a single JAR with zero dependencies. Also, we already had JSpecify as a dependency so this should not add anything.
Migrates
persistence/nosqlmodules from Jakarta@Nullable/@Nonnullto JSpecify@Nullable/@NonNullannotations. Jakarta's null annotations lack@Target(ElementType.TYPE_USE), so they can't annotate generic type parameters (e.g.,List<@Nullable String>) and cause Eclipse JDT errors in IDEs.Part of #4124
Changes
The bulk of this is a mechanical import swap across ~152 files, plus adding
org.jspecify:jspecify:1.0.0to each module's build config (matching the scope of existingjakarta.annotation.apideclarations).jakarta.annotation.apiis retained since it still provides@PostConstruct,@PreDestroy, etc.A few places needed non-mechanical fixes because TYPE_USE annotations have stricter placement rules than Jakarta's. Primitive array annotations like
@Nullable byte[]had to becomebyte @Nullable [](the original form annotates the element type, not the array reference). Similarly,@NonNull Outer.Innerhad to becomeOuter.@NonNull Innersince TYPE_USE annotations on a qualified type apply to the scoping construct, not the inner type. Also removed a stale workaround inAllRetained.javathat previously imported JSpecify alongside Jakarta.Testing
Verified with
./gradlew format compileAlland./gradlew check(intTest excluded due to a pre-existing Keycloak environment issue unrelated to this change). Hibernate Validator compatibility is unaffected since validation usesjakarta.validation.constraints.*which we don't touch, and the existing integration tests cover the full validation stack.Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)