SONARJAVA-4121: Read nullability annotations from the method declaration instead of the substituted type - #5906
Open
andret2344 wants to merge 1 commit into
Conversation
…notations from the method declaration Annotations inferred for a type variable at the call site were reported as annotations of the method itself, which made Optional.orElse look like it takes a @nonnull argument and never returns null.
Code Review ✅ ApprovedUpdates parameter and return type metadata to read annotations directly from method declarations rather than substituted types, fixing false positive nullability warnings on calls like Optional#orElse. No issues found. OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the false positive reported in SONARJAVA-4121 (and its twin JAVASE-87).
Discussed in https://community.sonarsource.com/t/false-positive-on-optional-orelse-null/82410
Problem
org.eclipse.jdt.annotation.NonNullandlombok.NonNullareTYPE_USEannotations, so ECJ keeps them inthe type binding. In the snippet above the type argument is inferred as
@NonNull String, the receiverbecomes
Optional<@NonNull String>and the substituted signature oforElsebecomes@NonNull String orElse(@NonNull String). Withjavax.annotation.Nonnull, which is notTYPE_USE, theFP does not appear.
The frontend copied those inferred annotations onto the symbols of the call:
JMethodSymbol#declarationParametersbuilt the parameter placeholder from the substituted parametertype, so
JSymbolMetadata#ofstored@NonNullamong the annotations of the parameter. The parameterwas then reported as
NON_NULLatVARIABLElevel, which is what S2637 reads.JSymbol#convertMetadataused the substituted return type, so the same annotation ended up on themethod symbol.
Optional#orElsewas reported asNON_NULLatMETHODlevel, which makes thesymbolic execution engine constrain the result of the call to
NOT_NULL.Note that the annotation only leaks when the type variable is substituted directly. For
map, whosereturn type is
Optional<@NonNull String>, the annotation correctly stays in the type argument metadata.Fix
Annotations are read from the declared types, while
type()andreturnType()keep the substitutedtypes, as checks rely on them. Annotations written explicitly on a declaration, including those on type
arguments of a declared parameter, are unaffected; only annotations inferred at the call site are
dropped.
JMethodSymbolandJVariableSymbol: parameter metadata comes from the declared parameter typeJSymbol: method metadata comes from the declared return typeTests
NullabilityWithInferredTypeArgumentreproduces the reported snippet and the null check that the leakedreturn type annotation would turn into an always-false condition. Two tests in
JSymbolMetadataTestcover the parameter and the method side; both fail without the change:
Ran on JDK 26:
java-frontend(1595 tests),java-checks-testkit(210 tests) andjava-checks(1706 tests) show no new failures compared to master.