Add `Objects.requireNonNullElse/ElseGet` support to `AnnotateNullableParameters` by timtebeek · Pull Request #743 · openrewrite/rewrite-static-analysis

timtebeek · 2025-09-27T10:34:36Z

Summary

Enhanced AnnotateNullableParameters recipe to recognize Objects.requireNonNullElse() and Objects.requireNonNullElseGet() calls as indicators of nullable parameters
These methods provide default values when arguments are null, implying they accept nullable parameters

Why this change?

Objects.requireNonNull() throws an exception if the argument is null, enforcing non-null parameters
Objects.requireNonNullElse() and Objects.requireNonNullElseGet() provide default values for null arguments, indicating the parameters can be null
Parameters that use these fallback methods should be annotated with @Nullable

Changes

Added Objects.requireNonNullElse(..) and Objects.requireNonNullElseGet(..) to the list of null-checking method matchers
Implemented visitMethodInvocation to handle standalone calls (not just those in if conditions)
Added comprehensive test coverage for both methods

Test plan

Added test for Objects.requireNonNullElse with default values
Added test for Objects.requireNonNullElseGet with supplier functions
Added test for multiple parameters with mixed usage of both methods
All existing tests continue to pass

🤖 Generated with Claude Code

The recipe now recognizes Objects.requireNonNull calls as null-checking operations and will annotate parameters passed to requireNonNull with @nullable. This handles both standalone calls and assignment statements using requireNonNull. - Added Objects.requireNonNull to the list of null-checking method matchers - Added visitMethodInvocation to handle standalone requireNonNull calls - Added tests for single and multiple parameter cases with requireNonNull

timtebeek · 2025-09-27T10:38:32Z

Not sure I agree with Claude here; when using requireNonNull I'd expect the variable to not be nullable, as we don't expect folks to rely on the NPE that gets thrown.

Objects.requireNonNull throws an exception if the argument is null, enforcing non-null parameters. In contrast, requireNonNullElse and requireNonNullElseGet provide default values for null arguments, indicating they accept nullable parameters. - Replaced Objects.requireNonNull with requireNonNullElse and requireNonNullElseGet - These methods imply the parameter can be null since they provide fallback values - Updated all tests to use the new methods that actually indicate nullable parameters

timtebeek · 2025-09-27T10:55:44Z

@stefanodallapalma this seems right up your alley ; any suggestions or concerns with this addition?

Added Optional.ofNullable to the list of null-checking methods in the AnnotateNullableParameters recipe. When a method parameter is passed to Optional.ofNullable(), it will now be annotated with @nullable to indicate that the parameter can accept null values. This improves the recipe's ability to identify nullable parameters, as Optional.ofNullable is specifically designed to handle potentially null values. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

stefanodallapalma · 2025-09-27T11:15:03Z

Not sure I agree with Claude here; when using requireNonNull I'd expect the variable to not be nullable, as we don't expect folks to rely on the NPE that gets thrown.

@timtebeek I had a similar concern here.

Objects.requireNonNullElse() and requireNonNullElseGet make sense. Now I realized we could support Optional.ofNullable() too

timtebeek · 2025-09-27T12:20:47Z

Thanks yes requireNonNull is quite the opposite, but requireNonNullElse* and Optional.ofNullable do indicate nullable parameters I think, or at the very least defensive coding. From your comment it's not yet clear whether you'd agree with the changes made here for those specific methods (not including requireNonNull from an earlier iteration). Is the current diff ok to merge you think?

stefanodallapalma · 2025-09-27T12:44:13Z

Oh yeah, thanks. The current diff looks good to me. I only left a minor comment

stefanodallapalma · 2025-09-27T12:43:05Z

src/test/java/org/openrewrite/staticanalysis/AnnotateNullableParametersTest.java

+                      }
+
+                      public Optional<String> conditionalWrap(@Nullable String data) {
+                          if (data != null && data.length() > 5) {


I find this test case a little redundant due to data != null. I'd rather test something like the following:

Optional.ofNullable(data).ifPresent(it -> ...);

What do you think?

Yes makes sense, thanks! Will merge with that change.

moderne-meeseeks bot added this to OpenRewrite Sep 27, 2025

github-project-automation bot moved this to In Progress in OpenRewrite Sep 27, 2025

timtebeek changed the title ~~Add Objects.requireNonNull support to AnnotateNullableParameters~~ Add Objects.requireNonNullElse/ElseGet support to AnnotateNullableParameters Sep 27, 2025

timtebeek changed the title ~~Add Objects.requireNonNullElse/ElseGet support to AnnotateNullableParameters~~ Add Objects.requireNonNullElse/ElseGet support to AnnotateNullableParameters Sep 27, 2025

timtebeek self-assigned this Sep 27, 2025

timtebeek added the enhancement New feature or request label Sep 27, 2025

timtebeek added 2 commits September 27, 2025 12:45

Merge tests

6885955

Only check the first argument, not any fallback

0bf363c

timtebeek marked this pull request as ready for review September 27, 2025 10:55

timtebeek moved this from In Progress to Ready to Review in OpenRewrite Sep 27, 2025

stefanodallapalma approved these changes Sep 27, 2025

View reviewed changes

Adjust test as suggested

b9fcc55

timtebeek merged commit 0bd9340 into main Sep 27, 2025
2 checks passed

timtebeek deleted the add-requirenonnull-to-annotate-nullable-params branch September 27, 2025 13:43

github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Sep 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Objects.requireNonNullElse/ElseGet` support to `AnnotateNullableParameters`#743

Add `Objects.requireNonNullElse/ElseGet` support to `AnnotateNullableParameters`#743
timtebeek merged 6 commits intomainfrom
add-requirenonnull-to-annotate-nullable-params

timtebeek commented Sep 27, 2025 •

edited

Loading

Uh oh!

timtebeek commented Sep 27, 2025

Uh oh!

timtebeek commented Sep 27, 2025

Uh oh!

stefanodallapalma commented Sep 27, 2025

Uh oh!

timtebeek commented Sep 27, 2025

Uh oh!

stefanodallapalma commented Sep 27, 2025

Uh oh!

stefanodallapalma Sep 27, 2025

Uh oh!

timtebeek Sep 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

timtebeek commented Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this change?

Changes

Test plan

Uh oh!

timtebeek commented Sep 27, 2025

Uh oh!

timtebeek commented Sep 27, 2025

Uh oh!

stefanodallapalma commented Sep 27, 2025

Uh oh!

timtebeek commented Sep 27, 2025

Uh oh!

stefanodallapalma commented Sep 27, 2025

Uh oh!

stefanodallapalma Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

timtebeek Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

timtebeek commented Sep 27, 2025 •

edited

Loading