You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "viditochani (via GitHub)" <gi...@apache.org> on 2026/04/04 18:26:12 UTC

[I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

viditochani opened a new issue, #4124:
URL: https://github.com/apache/polaris/issues/4124

   ### Motivation
   
   Polaris uses `jakarta.annotation.Nullable` and `jakarta.annotation.Nonnull` for null safety annotations across the codebase (462 usages in 349 files). These annotations lack `@Target(ElementType.TYPE_USE)`, which causes two problems:
   
   1. **IDE compatibility**: Eclipse JDT (used by VS Code's Java extension) rejects these annotations on type-use positions (e.g., generic type parameters) with: *"Annotation types that do not specify explicit target element types cannot be applied here."* The code compiles fine with `javac` but shows errors in VS Code.
   
   2. **Limited expressiveness**: Without `TYPE_USE` support, null annotations cannot be applied to generic type arguments like `List<@Nullable String>` or `Map.@NonNull Entry<K, V>`.
   
   The codebase already acknowledges this — `AllRetained.java` contains:
   ```java
   // @NonNull is the jspecify variant, which allows type-usage.
   // Jakarta's @Nonnull does not allow type-usage.
   private final BloomFilter<Map.@NonNull Entry<String, String>> refsFilter;
   ```
   
   Two files in `persistence/nosql` already use `org.jspecify.annotations`, and JSpecify 1.0.0 is already a transitive dependency.
   
   ### Proposal
   
   Migrate from `jakarta.annotation.Nullable`/`Nonnull` to `org.jspecify.annotations.Nullable`/`NonNull` across the codebase.
   
   - Add `org.jspecify:jspecify:1.0.0` as an explicit dependency in the version catalog
   - Replace imports module-by-module in separate PRs to keep reviews manageable
   - Suggested PR order:
     1. `persistence/nosql/*` (~115 files, already has JSpecify in 2 files)
     2. `polaris-core` (~140 files, core SPI)
     3. `runtime/service` + `runtime/admin` (~41 files)
     4. Remaining modules: extensions, plugins, tools, persistence/jdbc (~53 files)
   
   ### Notes
   
   - JSpecify is the emerging industry standard for null annotations, adopted by Google's Error Prone, NullAway, and IntelliJ IDEA
   - This is a mechanical refactor: `@Nullable` stays `@Nullable`, `@Nonnull` becomes `@NonNull`
   - No behavioral changes — these annotations are used for documentation and static analysis only
   - `jakarta.annotation-api` dependency can be retained for other annotations it provides (e.g., `@PostConstruct`, `@PreDestroy`)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4411082139

   This issue is stale because it has been open 30 days with no activity. Remove stale label or comment if you think it's still relevant.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.
viditochani commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4198086994

   I read through the thread. If I understand correctly, it seems to be primarily about whether we should change the default behavior of null vs non-null, and what we should annotate.
   
   This migration does not change those semantics. As I see it, adopting @NullMarked would be a natural follow-up, but it's an independent decision that this PR doesn't presuppose.
   
   Happy to revive the dev list thread, but wanted to confirm I'm not missing anything here and whether this blocks the corresponding PR (#4130).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.
viditochani commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4193132099

   Sounds good, I’ll start on this. I think the existing test suite already covers Hibernate Validator, but I’ll double-check.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4197489693

   Just FYI, nullness annotations were discussed a long time ago in Polaris:
   
   https://lists.apache.org/thread/0mndr05o2lnj66rngr2j21r698n3sps9
   
   I would very much like to see JSpecify annotations being used consistently in all of Polaris. But unfortunately, by the time we discussed this, there was no clear consensus about whether the code should be biaised towards non-nullable refs by default (which is JSpecify's approach), or to nullable ones by default.
   
   It may be worth reviving this discussion.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.
viditochani commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4413960921

   This issue is still active. Waiting on #4240 to be merged. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.
viditochani commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4200429649

   The code changes themselves were done via find-and-replace, along with some manual fixes for TYPE_USE placement differences. Formatting was done with Spotless. I used an AI tool to help refine the issue description and PR writeup.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4192293205

   Hi @viditochani,
   
   while this, as you say, looks more cosmetic, the missing `@Target(ElementType.TYPE_USE)` has an actual use case (beside that Eclipse isn't happy without it).
   A "mechanical refactor" sounds good. The only thing I'm a bit worried about is whether Hibernate Validator continues to work fine with the JSpecify annotation. I _think_ (but I'm not 100% sure) that it is used in the generated Open API Java types/interfaces. That's likely worth checking and verifying.
   But beside that concern, I think you plan is solid, and I wouldn't mind if you start in the order you suggested.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.
viditochani commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4187548776

   Happy to pick this up. If we agree on the migration, I'll start with `persistence/nosql/*` as the first PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4208097335

   > If I understand correctly, it seems to be primarily about whether we should change the default behavior of null vs non-null, and what we should annotate.
   
   That's correct, the ML thread is a general discussion around nullability with implications on the annotations we use. You don't necessarily need to revive that thread now, I mentioned it mostly for completeness.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.
viditochani closed issue #4124: Migrate null annotations from jakarta.annotation to JSpecify
URL: https://github.com/apache/polaris/issues/4124


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4198497340

   Per ASF generative tooling guidance, could you confirm whether AI tools were used and add the appropriate `Generated-by:` token to the commit message?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4197673984

   Good point @adutra - I totally forgot about that discussion.
   
   @viditochani can you revive the dev-ML thread?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Migrate null annotations from jakarta.annotation to JSpecify [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.
viditochani commented on issue #4124:
URL: https://github.com/apache/polaris/issues/4124#issuecomment-4539456628

   Migration complete. Polaris no longer uses Jakarta null annotations.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org