Add Objects.requireNonNullElse/ElseGet support to AnnotateNullableParameters
#743
Conversation
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
|
Not sure I agree with Claude here; when using |
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
changed the title
timtebeek
changed the title
Objects.requireNonNullElse/ElseGet support to AnnotateNullableParameters
|
@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 <noreply@anthropic.com>
@timtebeek I had a similar concern here.
|
|
Thanks yes |
|
Oh yeah, thanks. The current diff looks good to me. I only left a minor comment |
| } | ||
| public Optional<String> conditionalWrap(@Nullable String data) { | ||
| if (data != null && data.length() > 5) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yes makes sense, thanks! Will merge with that change.
timtebeek
deleted the
add-requirenonnull-to-annotate-nullable-params
branch
2 participants
Summary
AnnotateNullableParametersrecipe to recognizeObjects.requireNonNullElse()andObjects.requireNonNullElseGet()calls as indicators of nullable parametersWhy this change?
Objects.requireNonNull()throws an exception if the argument is null, enforcing non-null parametersObjects.requireNonNullElse()andObjects.requireNonNullElseGet()provide default values for null arguments, indicating the parameters can be null@NullableChanges
Objects.requireNonNullElse(..)andObjects.requireNonNullElseGet(..)to the list of null-checking method matchersvisitMethodInvocationto handle standalone calls (not just those in if conditions)Test plan
Objects.requireNonNullElsewith default valuesObjects.requireNonNullElseGetwith supplier functions🤖 Generated with Claude Code