From acb3275a86b74bdfc998969a6db9486645114883 Mon Sep 17 00:00:00 2001 From: Andret2344 Date: Fri, 7 Aug 2026 17:56:34 +0200 Subject: [PATCH] SONARJAVA-4121: Changed parameter and return type metadata to read annotations 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. --- .../NullabilityWithInferredTypeArgument.java | 29 +++++++++++++++++++ .../org/sonar/java/model/JMethodSymbol.java | 8 ++++- .../java/org/sonar/java/model/JSymbol.java | 10 +++++-- .../org/sonar/java/model/JVariableSymbol.java | 9 ++++-- .../sonar/java/model/JSymbolMetadataTest.java | 29 +++++++++++++++++++ 5 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/java/annotations/nullability/no_default/NullabilityWithInferredTypeArgument.java diff --git a/java-checks-test-sources/default/src/main/java/annotations/nullability/no_default/NullabilityWithInferredTypeArgument.java b/java-checks-test-sources/default/src/main/java/annotations/nullability/no_default/NullabilityWithInferredTypeArgument.java new file mode 100644 index 00000000000..3104f6fb2a2 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/annotations/nullability/no_default/NullabilityWithInferredTypeArgument.java @@ -0,0 +1,29 @@ +package annotations.nullability.no_default; + +import java.util.Optional; +import org.eclipse.jdt.annotation.NonNull; + +public class NullabilityWithInferredTypeArgument { + + private final String value = "value"; + + @NonNull + public String getValue() { + return value; + } + + public String callOrElse(NullabilityWithInferredTypeArgument a) { + // "map" infers "Optional<@NonNull String>", the @NonNull is part of the type argument and says nothing about the parameter of "orElse" + return Optional.ofNullable(a) + .map(NullabilityWithInferredTypeArgument::getValue) + .orElse(null); + } + + public boolean nullCheckAfterOrElse(NullabilityWithInferredTypeArgument a) { + String result = Optional.ofNullable(a) + .map(NullabilityWithInferredTypeArgument::getValue) + .orElse(null); + // the inferred @NonNull must not make "orElse" look like it never returns null, the check below is not always false + return result == null; + } +} diff --git a/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java b/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java index 5ae3d502130..c32c6739b8a 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java @@ -92,9 +92,15 @@ public List declarationParameters() { } else { parameters = new ArrayList<>(); IMethodBinding methodBinding = methodBinding(); + IMethodBinding methodDeclaration = methodBinding.getMethodDeclaration(); ITypeBinding[] parameterTypeBindings = methodBinding.getParameterTypes(); + ITypeBinding[] declaredParameterTypeBindings = methodDeclaration.getParameterTypes(); for (int i = 0; i < parameterTypeBindings.length; i++) { - parameters.add(new JVariableSymbol.ParameterPlaceholderSymbol(i, sema, methodBinding.getMethodDeclaration(), parameterTypeBindings[i])); + // Annotations must come from the declared type: type annotations of the substituted type can be inferred from the call site + // (e.g. "Optional.of(x).map(A::nonNullMethod).orElse(null)" infers "Optional<@NonNull String>", making "orElse" look like it takes + // a @NonNull argument), which does not tell anything about the parameter of the declared method. + ITypeBinding declaredParameterType = i < declaredParameterTypeBindings.length ? declaredParameterTypeBindings[i] : parameterTypeBindings[i]; + parameters.add(new JVariableSymbol.ParameterPlaceholderSymbol(i, sema, methodDeclaration, parameterTypeBindings[i], declaredParameterType)); } } } diff --git a/java-frontend/src/main/java/org/sonar/java/model/JSymbol.java b/java-frontend/src/main/java/org/sonar/java/model/JSymbol.java index dcd11673412..b3c757641a1 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JSymbol.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JSymbol.java @@ -386,12 +386,18 @@ private SymbolMetadata convertMetadata() { } return convertMetadata(type); case IBinding.METHOD: - ITypeBinding returnType = ((IMethodBinding) binding).getReturnType(); + IMethodBinding methodBinding = (IMethodBinding) binding; + ITypeBinding returnType = methodBinding.getReturnType(); // In rare circumstances, when the semantic information is incomplete, returnType can be null. if (returnType == null) { return Symbols.EMPTY_METADATA; } - return convertMetadata(returnType); + // Annotations are read from the declared return type, so that annotations inferred for a type variable at the call site + // (e.g. "Optional.of(x).map(A::nonNullMethod)" infers "Optional<@NonNull String>", making "orElse" look like it never returns null) + // are not mistaken for annotations of the method itself. + IMethodBinding methodDeclaration = methodBinding.getMethodDeclaration(); + ITypeBinding declaredReturnType = methodDeclaration == null ? null : methodDeclaration.getReturnType(); + return convertMetadata(declaredReturnType == null ? returnType : declaredReturnType); default: return new JSymbolMetadata(sema, this, binding.getAnnotations()); } diff --git a/java-frontend/src/main/java/org/sonar/java/model/JVariableSymbol.java b/java-frontend/src/main/java/org/sonar/java/model/JVariableSymbol.java index ae6458f39de..0e5bf9fbdb3 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JVariableSymbol.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JVariableSymbol.java @@ -89,11 +89,16 @@ static class ParameterPlaceholderSymbol extends Symbols.DefaultSymbol implements private final Type type; private final SymbolMetadata metadata; - ParameterPlaceholderSymbol(int index, JSema sema, IMethodBinding owner, ITypeBinding typeBinding) { + /** + * @param typeBinding the type of the parameter at the call site, after type substitution + * @param declaredTypeBinding the type of the parameter as declared, before type substitution. Annotations are read from it, so that + * annotations inferred for a type variable are not mistaken for annotations of the parameter itself. + */ + ParameterPlaceholderSymbol(int index, JSema sema, IMethodBinding owner, ITypeBinding typeBinding, ITypeBinding declaredTypeBinding) { this.name = "arg" + index; this.owner = sema.methodSymbol(owner); this.type = sema.type(typeBinding); - this.metadata = JSymbolMetadata.of(sema, this, typeBinding, owner.getParameterAnnotations(index)); + this.metadata = JSymbolMetadata.of(sema, this, declaredTypeBinding, owner.getParameterAnnotations(index)); } @Override diff --git a/java-frontend/src/test/java/org/sonar/java/model/JSymbolMetadataTest.java b/java-frontend/src/test/java/org/sonar/java/model/JSymbolMetadataTest.java index 60a2f838c2f..e9ddce8bef7 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/JSymbolMetadataTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/JSymbolMetadataTest.java @@ -41,6 +41,7 @@ import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.ReturnStatementTree; import org.sonar.plugins.java.api.tree.Tree; import static org.assertj.core.api.Assertions.assertThat; @@ -237,6 +238,34 @@ void generics_nullability() throws IOException { .isSameAs(invocation2ParamData); } + @Test + void nullability_is_not_inferred_from_type_arguments() throws IOException { + Path sourceFile = NULLABILITY_SOURCE_DIR.resolve(Paths.get("no_default", "NullabilityWithInferredTypeArgument.java")); + CompilationUnitTree cut = JParserTestUtils.parse(sourceFile.toRealPath().toFile(), JParserTestUtils.checksTestClassPath()); + ClassTree classTree = (ClassTree) cut.types().get(0); + MethodTree callOrElse = (MethodTree) classTree.members().get(2); + + MethodInvocationTree orElseInvocation = (MethodInvocationTree) ((ReturnStatementTree) callOrElse.block().body().get(0)).expression(); + Symbol orElseParameter = orElseInvocation.methodSymbol().declarationParameters().get(0); + + assertThat(orElseParameter.metadata().annotations()).isEmpty(); + assertThat(orElseParameter.metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION); + } + + @Test + void method_nullability_is_not_inferred_from_type_arguments() throws IOException { + Path sourceFile = NULLABILITY_SOURCE_DIR.resolve(Paths.get("no_default", "NullabilityWithInferredTypeArgument.java")); + CompilationUnitTree cut = JParserTestUtils.parse(sourceFile.toRealPath().toFile(), JParserTestUtils.checksTestClassPath()); + ClassTree classTree = (ClassTree) cut.types().get(0); + MethodTree callOrElse = (MethodTree) classTree.members().get(2); + + MethodInvocationTree orElseInvocation = (MethodInvocationTree) ((ReturnStatementTree) callOrElse.block().body().get(0)).expression(); + SymbolMetadata orElseMetadata = orElseInvocation.methodSymbol().metadata(); + + assertThat(orElseMetadata.symbolAnnotations()).isEmpty(); + assertThat(orElseMetadata.nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION); + } + @Nested class NullabilityDataTest {