SONARJAVA-4121: Read nullability annotations from the method declaration instead of the substituted type

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;
+  }
+}

java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java

@@ -92,9 +92,15 @@ public List<Symbol> 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));
         }
       }
     }

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());
     }

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

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 {