Move handling of type parameter bounds from nullness flow analysis to…

… qualifier inference.

Add missing treatment of method return values into inference to make that work.

RELNOTES: None.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=227891663

check_api/src/main/java/com/google/errorprone/dataflow/nullnesspropagation/NullnessPropagationTransfer.java

@@ -25,7 +25,6 @@
 import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NULLABLE;
 import static com.sun.tools.javac.code.TypeTag.BOOLEAN;
 import static javax.lang.model.element.ElementKind.EXCEPTION_PARAMETER;
-import static javax.lang.model.element.ElementKind.TYPE_PARAMETER;
 import static org.checkerframework.javacutil.TreeUtils.elementFromDeclaration;
 import static org.checkerframework.javacutil.TreeUtils.enclosingOfClass;
 
@@ -88,14 +87,7 @@
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
-import javax.lang.model.element.AnnotationMirror;
-import javax.lang.model.element.Element;
-import javax.lang.model.element.TypeElement;
-import javax.lang.model.element.TypeParameterElement;
 import javax.lang.model.element.VariableElement;
-import javax.lang.model.type.TypeKind;
-import javax.lang.model.type.TypeMirror;
-import javax.lang.model.type.TypeVariable;
 import org.checkerframework.dataflow.analysis.Analysis;
 import org.checkerframework.dataflow.cfg.CFGBuilder;
 import org.checkerframework.dataflow.cfg.ControlFlowGraph;
@@ -766,7 +758,10 @@ Nullness standardFieldNullness(
     Optional<Nullness> declaredNullness = Nullness.fromAnnotationsOn(accessed.symbol);
     return declaredNullness.orElseGet(
         () ->
-            Nullness.fromAnnotations(inheritedAnnotations(accessed.symbol.type))
+            Nullness.fromAnnotations(
+                    MoreAnnotations.inheritedAnnotations(accessed.symbol.type).stream()
+                        .map(Object::toString)
+                        .collect(ImmutableList.toImmutableList()))
                 .orElse(defaultAssumption));
   }
 
@@ -788,54 +783,8 @@ private Nullness returnValueNullness(MethodInvocationNode node, @Nullable ClassA
       // We only care about inference results for methods that return a type variable.
       return assumedNullness;
     }
-
-    // Look for applicable annotations inherited from elsewhere.
-    // TODO(b/121398981): Treat type parameter bounds correctly
-    Nullness lowerBound =
-        Nullness.fromAnnotations(inheritedAnnotations(node.getTarget().getMethod().getReturnType()))
-            .orElse(NULLABLE);
-
     // Method has a generic result, so ask inference to infer a qualifier for that type parameter
-    return getInferredNullness(node).orElse(assumedNullness).greatestLowerBound(lowerBound);
-  }
-
-  /**
-   * Gathers all type annotations that are applicable to this TypeMirror and its bounds but are not
-   * applied syntactically to its declaration. This includes:
-   *
-   * <ul>
-   *   <li>annotations on type parameters at type use, e.g. {@code List<@Nullable String> xs = ...}
-   *   <li>annotations on generic type declarations, e.g. {@code class MyClass<@Nullable T> {...} }
-   *   <li>bounds on the above, e.g. {@code class MyClass<T extends @Nullable MyOtherClass> {...} }
-   * </ul>
-   */
-  private static ImmutableList<String> inheritedAnnotations(TypeMirror type) {
-    ImmutableSet.Builder<AnnotationMirror> inheritedAnnotations = ImmutableSet.builder();
-
-    // Annotations on type parameters at use-site
-    inheritedAnnotations.addAll(type.getAnnotationMirrors());
-
-    if (type.getKind() == TypeKind.TYPEVAR) {
-      TypeVariable typeVar = (TypeVariable) type;
-      // Annotations on bounds at type variable declaration
-      inheritedAnnotations.addAll(typeVar.getUpperBound().getAnnotationMirrors());
-      if (typeVar.asElement().getKind() == TYPE_PARAMETER) {
-        Element genericElt = ((TypeParameterElement) typeVar.asElement()).getGenericElement();
-        if (genericElt.getKind().isClass() || genericElt.getKind().isInterface()) {
-          ((TypeElement) genericElt)
-              .getTypeParameters().stream()
-                  .filter(
-                      typeParam ->
-                          typeParam.getSimpleName().equals(typeVar.asElement().getSimpleName()))
-                  .findFirst()
-                  // Annotations at class/interface type variable declaration
-                  .ifPresent(decl -> inheritedAnnotations.addAll(decl.getAnnotationMirrors()));
-        }
-      }
-    }
-    return inheritedAnnotations.build().stream()
-        .map(Object::toString)
-        .collect(ImmutableList.toImmutableList());
+    return getInferredNullness(node).orElse(assumedNullness);
   }
 
   @Nullable

check_api/src/main/java/com/google/errorprone/dataflow/nullnesspropagation/inference/NullnessQualifierInference.java

@@ -27,6 +27,7 @@
 import com.google.common.graph.GraphBuilder;
 import com.google.common.graph.MutableGraph;
 import com.google.common.util.concurrent.UncheckedExecutionException;
+import com.google.errorprone.dataflow.nullnesspropagation.Nullness;
 import com.sun.source.tree.ArrayAccessTree;
 import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.BlockTree;
@@ -58,6 +59,7 @@
 import java.util.ArrayDeque;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * Eagerly traverse one {@code MethodTree} at a time and accumulate constraints between nullness
@@ -143,6 +145,33 @@ private void generateConstraintsFromAnnotations(
             });
   }
 
+  private void generateConstraintsFromAnnotations(
+      MethodSymbol symbol, JCMethodInvocation sourceTree, ArrayDeque<Integer> argSelector) {
+    List<Type> typeArguments = sourceTree.type.getTypeArguments();
+    int numberOfTypeArgs = typeArguments.size();
+    for (int i = 0; i < numberOfTypeArgs; i++) {
+      argSelector.push(i);
+      generateConstraintsFromAnnotations(typeArguments.get(i), sourceTree, argSelector);
+      argSelector.pop();
+    }
+
+    // First check if the given symbol is directly annotated; if not, look for implicit annotations
+    // on the inferred type of the expression.  The latter for instance propagates a type parameter
+    // T instantiated as <@Nullable X> to the result of a method returning T.
+    Optional<InferenceVariable> fromAnnotations =
+        Nullness.fromAnnotationsOn(symbol).map(ProperInferenceVar::create);
+    if (!fromAnnotations.isPresent()) {
+      fromAnnotations = ProperInferenceVar.fromTypeIfAnnotated(sourceTree.type);
+    }
+    fromAnnotations.ifPresent(
+        annot -> {
+          qualifierConstraints.putEdge(
+              TypeArgInferenceVar.create(ImmutableList.copyOf(argSelector), sourceTree), annot);
+          qualifierConstraints.putEdge(
+              annot, TypeArgInferenceVar.create(ImmutableList.copyOf(argSelector), sourceTree));
+        });
+  }
+
   @Override
   public Void visitAssignment(AssignmentTree node, Void unused) {
     Type lhsType =
@@ -206,19 +235,21 @@ public Void visitMethodInvocation(MethodInvocationTree node, Void unused) {
                 .map(var -> var.type)
                 .collect(ImmutableList.toImmutableList());
 
-    // generate constraints for each argument write.
+    // Generate constraints for each argument write.
     Streams.forEachPair(
         formalParameters.stream(),
         sourceNode.getArguments().stream(),
         (formal, actual) -> {
           // formal parameter type
+          // TODO(b/116977632): constraints for actual parameter type (i.e. after type variable
+          // substitution) without ignoring annotations directly on the parameter or vararg
           generateConstraintsFromAnnotations(formal, actual, new ArrayDeque<>());
-          // actual parameter type (i.e. after type variable substitution)
-          // TODO(b/116977632): fix to generate constraints for the instantiation of type parameters
-          generateConstraintsFromAnnotations(actual.type, actual, new ArrayDeque<>());
         });
 
-    // if return type is parameterized by a generic type on receiver, collate references to that
+    // Generate constraints for method return
+    generateConstraintsFromAnnotations(callee, sourceNode, new ArrayDeque<>());
+
+    // If return type is parameterized by a generic type on receiver, collate references to that
     // generic between the receiver and the result/argument types.
     if (node.getMethodSelect() instanceof JCFieldAccess) {
       JCFieldAccess fieldAccess = ((JCFieldAccess) node.getMethodSelect());

check_api/src/main/java/com/google/errorprone/dataflow/nullnesspropagation/inference/ProperInferenceVar.java

@@ -17,6 +17,7 @@
 package com.google.errorprone.dataflow.nullnesspropagation.inference;
 
 import com.google.errorprone.dataflow.nullnesspropagation.Nullness;
+import com.google.errorprone.util.MoreAnnotations;
 import com.sun.tools.javac.code.Type;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -55,7 +56,9 @@ Nullness nullness() {
 
   static Optional<InferenceVariable> fromTypeIfAnnotated(Type type) {
     return Nullness.fromAnnotations(
-            type.getAnnotationMirrors().stream().map(Object::toString).collect(Collectors.toList()))
+            MoreAnnotations.inheritedAnnotations(type).stream()
+                .map(Object::toString)
+                .collect(Collectors.toList()))
         .map(ProperInferenceVar::create);
   }
 

check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java

@@ -18,8 +18,10 @@
 
 import static java.util.stream.Collectors.groupingBy;
 import static java.util.stream.Collectors.toList;
+import static javax.lang.model.element.ElementKind.TYPE_PARAMETER;
 
 import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Streams;
 import com.sun.tools.javac.code.Attribute;
 import com.sun.tools.javac.code.Attribute.Compound;
@@ -32,8 +34,15 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Stream;
+import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.AnnotationValue;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.TypeElement;
+import javax.lang.model.element.TypeParameterElement;
 import javax.lang.model.element.VariableElement;
+import javax.lang.model.type.TypeKind;
+import javax.lang.model.type.TypeMirror;
+import javax.lang.model.type.TypeVariable;
 import javax.lang.model.util.SimpleAnnotationValueVisitor8;
 
 /** Annotation-related utilities. */
@@ -165,5 +174,44 @@ public Stream<String> visitArray(List<? extends AnnotationValue> list, Void unus
         Stream.empty());
   }
 
+  /**
+   * Gathers all type annotations that are applicable to this TypeMirror and its bounds but are not
+   * applied syntactically to its declaration. This includes:
+   *
+   * <ul>
+   *   <li>annotations on type parameters at type use, e.g. {@code List<@Nullable String> xs = ...}
+   *   <li>annotations on generic type declarations, e.g. {@code class MyClass<@Nullable T> {...} }
+   *   <li>bounds on the above, e.g. {@code class MyClass<T extends @Nullable MyOtherClass> {...} }
+   * </ul>
+   */
+  // TODO(b/121398981): distinguish upper and lower bounds of type variables
+  public static ImmutableSet<AnnotationMirror> inheritedAnnotations(TypeMirror type) {
+    ImmutableSet.Builder<AnnotationMirror> inheritedAnnotations = ImmutableSet.builder();
+
+    // Annotations on type parameters at use-site
+    inheritedAnnotations.addAll(type.getAnnotationMirrors());
+
+    if (type.getKind() == TypeKind.TYPEVAR) {
+      TypeVariable typeVar = (TypeVariable) type;
+      // Annotations on bounds at type variable declaration
+      // TODO(b/121398981): handle intersection bounds
+      inheritedAnnotations.addAll(typeVar.getUpperBound().getAnnotationMirrors());
+      if (typeVar.asElement().getKind() == TYPE_PARAMETER) {
+        Element genericElt = ((TypeParameterElement) typeVar.asElement()).getGenericElement();
+        if (genericElt.getKind().isClass() || genericElt.getKind().isInterface()) {
+          ((TypeElement) genericElt)
+              .getTypeParameters().stream()
+                  .filter(
+                      typeParam ->
+                          typeParam.getSimpleName().equals(typeVar.asElement().getSimpleName()))
+                  .findFirst()
+                  // Annotations at class/interface type variable declaration
+                  .ifPresent(decl -> inheritedAnnotations.addAll(decl.getAnnotationMirrors()));
+        }
+      }
+    }
+    return inheritedAnnotations.build();
+  }
+
   private MoreAnnotations() {}
 }