Skip to content

Introduce JSpecify for nullability annotations #4091

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.

Sign up for GitHub

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

Jump to bottom
Merged
chemicL merged 18 commits into from
Sep 18, 2025
Merged

Introduce JSpecify for nullability annotations #4091

chemicL merged 18 commits into from
Sep 18, 2025

Conversation

@chemicL
Copy link
Member

@chemicL chemicL commented Sep 17, 2025 •
edited
Loading

The JSR-305-based reactor.util.annotation.* classes have been deprecated in favour of JSpecify annotations.
The build runs NullAway errorprone plugin. Several public APIs have been refined to better reflect the nullability characteristics and also some internal implementations have also been adjusted based on the new compilation warnings.
This PR replaces #4016.

chemicL added 16 commits August 6, 2025 15:49
@chemicL
WIP: Replace reactor.util.annotation with org.jspecify.annotations
b57aba5 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Replace reactor.util.annotation.NonNullApi with org.jspecify.ann…
25c659a 
…otations.NullMarked

Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Temporarily remove contents of null-safety.adoc
60a3655 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Deprecate nullability annotations
9f02797 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Update copyright headers
db24a45 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Move method nullability annotation to return type
8251d6e 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Replace wrong nullability annotations usage (JSR305)
53fbbde 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Add NullAway
94a941b 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Add @nullable to scanUnsafe overrides
076499f 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Add missing @nullable annotations
d6b16bb 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: Add checkstyle setting, disable warn->err for NullAway, add miss…
a171c4f 
…ing imports

Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
WIP: public API nullability corrections
aea5e14 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
Update NullAway and configure
4ec1bd2 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
Remove redundant @nonnull annotations from Tuple classes
b59a05f 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
Further nullability improvements
1d033c1 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL
More assertions and nullability annotations
85b3779 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL chemicL added this to the 3.8.0-RC1 milestone Sep 17, 2025
@chemicL chemicL added type/enhancement A general enhancement warn/deprecation This issue/PR introduces deprecations labels Sep 17, 2025
@chemicL chemicL mentioned this pull request Sep 17, 2025
Closed
@chemicL
Merge branch 'main' into nullability-jspecify
b2c763e 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 17, 2025
View reviewed changes
reactor-core/src/main/java/reactor/core/publisher/FluxUsingWhen.java

@Nullable
public Object scanUnsafe(Attr key) {
public @Nullable Object scanUnsafe(Attr key) {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
Scannable.scanUnsafe
Loading
; it is advisable to add an Override annotation.
This method overrides
InnerProducer.scanUnsafe
Loading
; it is advisable to add an Override annotation.
reactor-core/src/main/java/reactor/core/publisher/InternalFluxOperator.java

@Nullable
public abstract CoreSubscriber<? super I> subscribeOrReturn(CoreSubscriber<? super O> actual) throws Throwable;
public abstract @Nullable CoreSubscriber<? super I> subscribeOrReturn(CoreSubscriber<? super O> actual) throws Throwable;

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
OptimizableOperator<O,I>.subscribeOrReturn
Loading
; it is advisable to add an Override annotation.
reactor-core/src/main/java/reactor/core/publisher/InternalMonoOperator.java

@Nullable
public abstract CoreSubscriber<? super I> subscribeOrReturn(CoreSubscriber<? super O> actual) throws Throwable;
public abstract @Nullable CoreSubscriber<? super I> subscribeOrReturn(CoreSubscriber<? super O> actual) throws Throwable;

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
OptimizableOperator<O,I>.subscribeOrReturn
Loading
; it is advisable to add an Override annotation.
reactor-core/src/main/java/reactor/core/publisher/MonoStreamCollector.java
}

R accumulatedValue() {
@Nullable R accumulatedValue() {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
BaseFluxToMonoOperator<T,R>.accumulatedValue
Loading
; it is advisable to add an Override annotation.
sdeleuze
sdeleuze reviewed Sep 17, 2025
View reviewed changes
Copy link
Contributor

@sdeleuze sdeleuze left a comment

Choose a reason for hiding this comment

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

LGTM, modulo 2 minor comments and the remark I shared previously on double checking arrays nullability manually as the JSpecifyMode is not yet enabled yet.

reactor-core/src/main/java/reactor/core/publisher/ConnectableFluxOnAssembly.java
}

@Override
public final CoreSubscriber<? super T> subscribeOrReturn(CoreSubscriber<? super T> actual) {
Copy link
Contributor

@sdeleuze sdeleuze Sep 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is the removal of final done on purpose?

Copy link
Member Author

@chemicL chemicL Sep 17, 2025

Choose a reason for hiding this comment

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

yep, final class, redundant marker

reactor-core/src/main/java/reactor/core/publisher/DelegateProcessor.java
// super.getError() returns null by default and this method would throw NPE in that
// case, however the parent's contract is to return nullable Throwable
@Override
@Nullable
Copy link
Contributor

@sdeleuze sdeleuze Sep 17, 2025

Choose a reason for hiding this comment

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

Have you tried explicit @NonNull instead of @SuppressWarnings?

Copy link
Member Author

@chemicL chemicL Sep 17, 2025

Choose a reason for hiding this comment

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

The problem is actually in the argument passed to scanOrDefault which is required to be non-null, otherwise the method throws. Somehow the contract of that method is that it should not accept null values, yet there is extra verification using Object.requireNonNull in place in the body. This is the only weird use of it that can pass on a null argument. Since the class is deprecated I don't want to spend much more time investigating.

@chemicL
Move remaining @nullable annotations to type and minor cleanup
822d87c 
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
@chemicL chemicL marked this pull request as ready for review September 18, 2025 09:36
@chemicL chemicL requested a review from a team as a code owner September 18, 2025 09:36
@chemicL chemicL merged commit 300f627 into main Sep 18, 2025
10 of 11 checks passed
@chemicL chemicL deleted the nullability-jspecify branch September 18, 2025 13:03
@chemicL chemicL mentioned this pull request Oct 31, 2025
Merged
chemicL added a commit that referenced this pull request Oct 31, 2025
@chemicL
Address NullAway warnings and refine StepVerifier nullability (#4126)
03521d9 
This change addresses the remaining NullAway warnings left after #4091.

It also contains public API refinenements around StepVerifier and
TestPublisher from reactor-test to allow verifying non-compliant
Publishers against null values.

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

Reviewers

1 more reviewer

@sdeleuze sdeleuze sdeleuze left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/enhancement A general enhancement warn/deprecation This issue/PR introduces deprecations

Projects

None yet

Milestone

3.8.0-RC1

Development

Successfully merging this pull request may close these issues.

3 participants

@chemicL @sdeleuze