Skip to content

Migrate persistence/nosql to JSpecify null annotations#4130

Open
viditochani wants to merge 1 commit intoapache:mainfrom
viditochani:jspecify-nosql-migration
Open

Migrate persistence/nosql to JSpecify null annotations#4130
viditochani wants to merge 1 commit intoapache:mainfrom
viditochani:jspecify-nosql-migration

Conversation

@viditochani
Copy link
Copy Markdown

@viditochani viditochani commented Apr 7, 2026

Migrates persistence/nosql modules from Jakarta @Nullable/@Nonnull to JSpecify @Nullable/@NonNull annotations. 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.0 to each module's build config (matching the scope of existing jakarta.annotation.api declarations). jakarta.annotation.api is 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 become byte @Nullable [] (the original form annotates the element type, not the array reference). Similarly, @NonNull Outer.Inner had to become Outer.@NonNull Inner since TYPE_USE annotations on a qualified type apply to the scoping construct, not the inner type. Also removed a stale workaround in AllRetained.java that previously imported JSpecify alongside Jakarta.

Testing

Verified with ./gradlew format compileAll and ./gradlew check (intTest excluded due to a pre-existing Keycloak environment issue unrelated to this change). Hibernate Validator compatibility is unaffected since validation uses jakarta.validation.constraints.* which we don't touch, and the existing integration tests cover the full validation stack.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Migrate null annotations from jakarta.annotation to JSpecify #4124
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

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
Copy link
Copy Markdown
Member

@snazy snazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Author

@viditochani viditochani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@viditochani
Copy link
Copy Markdown
Author

@snazy - any further feedback on this, or can we merge?

Copy link
Copy Markdown
Contributor

@flyrain flyrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Apr 12, 2026
Copy link
Copy Markdown
Author

@viditochani viditochani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, JSpecify is a single JAR with zero dependencies. Also, we already had JSpecify as a dependency so this should not add anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants