Skip to content

[improve][misc] Migrate from multiple nullness annotation libraries to JSpecify annotations #24239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 3, 2025

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented May 2, 2025

Motivation

We are currently using multiple nullness annotations libraries.
Many JDK, tool and library vendors have standardized on JSpecify nullness annotations so that each vendor doesn't introduce its own classes for this purpose. We should migrate too.

In addition, one additional motivation is that recent Google Guava versions no longer contains a transitive dependency on checker-qual library.

Modifications

migrate these to org.jspecify.annotations.NonNull

  • org.checkerframework.checker.nullness.qual.NonNull
  • javax.annotation.Nonnull
  • org.jetbrains.annotations.NotNull

migrate these to org.jspecify.annotations.Nullable

  • org.checkerframework.checker.nullness.qual.Nullable
  • javax.annotation.Nullable
  • org.jetbrains.annotations.Nullable

In addition there was code that was using javax.validation annotations for nullness information. That's invalid since those annotations are used for user input validation and not static code analysis.

Add checkstyle rules:

  • to prevent introducing the replaced deprecated annotations
  • to prevent introducing javax.validation annotations

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari requested a review from heesung-sn May 2, 2025 18:03
lhotari added a commit to lhotari/pulsar that referenced this pull request May 2, 2025
@codecov-commenter
Copy link

codecov-commenter commented May 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.17%. Comparing base (bbc6224) to head (0792ef2).
Report is 1063 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24239      +/-   ##
============================================
+ Coverage     73.57%   74.17%   +0.59%     
+ Complexity    32624    32557      -67     
============================================
  Files          1877     1866      -11     
  Lines        139502   144976    +5474     
  Branches      15299    16561    +1262     
============================================
+ Hits         102638   107530    +4892     
- Misses        28908    28915       +7     
- Partials       7956     8531     +575     
Flag Coverage Δ
inttests 26.67% <ø> (+2.09%) ⬆️
systests 23.26% <ø> (-1.06%) ⬇️
unittests 73.65% <ø> (+0.80%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rg/apache/bookkeeper/mledger/impl/AckSetState.java 100.00% <ø> (ø)
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 79.21% <ø> (-0.09%) ⬇️
...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java 81.19% <ø> (+0.53%) ⬆️
...thentication/oidc/OpenIDProviderMetadataCache.java 78.75% <ø> (+0.26%) ⬆️
...pache/pulsar/broker/admin/impl/NamespacesBase.java 76.09% <ø> (+3.01%) ⬆️
...pulsar/broker/admin/impl/PersistentTopicsBase.java 70.14% <ø> (+4.69%) ⬆️
...rg/apache/pulsar/broker/admin/v3/Transactions.java 69.17% <ø> (-6.85%) ⬇️
...elayed/bucket/BookkeeperBucketSnapshotStorage.java 81.25% <ø> (-2.49%) ⬇️
...ulsar/broker/namespace/NamespaceEphemeralData.java 86.66% <ø> (-8.79%) ⬇️
...ache/pulsar/broker/namespace/NamespaceService.java 73.18% <ø> (+0.95%) ⬆️
... and 25 more

... and 1049 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodece nodece merged commit 9a11245 into apache:master May 3, 2025
52 checks passed
lhotari added a commit that referenced this pull request May 5, 2025
…o JSpecify annotations (#24239)

(cherry picked from commit 9a11245)
lhotari added a commit that referenced this pull request May 5, 2025
…o JSpecify annotations (#24239)

(cherry picked from commit 9a11245)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 8, 2025
…o JSpecify annotations (apache#24239)

(cherry picked from commit 9a11245)
(cherry picked from commit 62c505d)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 8, 2025
…o JSpecify annotations (apache#24239)

(cherry picked from commit 9a11245)
(cherry picked from commit 62c505d)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 14, 2025
…o JSpecify annotations (apache#24239)

(cherry picked from commit 9a11245)
(cherry picked from commit 62c505d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants