fixes #27586, prefer context type in generic inference

pkg/analyzer/lib/src/generated/resolver.dart

Index: pkg/analyzer/lib/src/generated/resolver.dart
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 6287f80eda76cd463a4b9d1f5ebe9d307fe27100..4abb2e6b198bc73f359ef41cc112c6ab8d658911 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -19,6 +19,7 @@ import 'package:analyzer/exception/exception.dart';
 import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/ast/utilities.dart';
 import 'package:analyzer/src/dart/element/element.dart';
+import 'package:analyzer/src/dart/element/member.dart' show ConstructorMember;
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/resolver/inheritance_manager.dart';
 import 'package:analyzer/src/dart/resolver/scope.dart';
@@ -4169,35 +4170,6 @@ class InferenceContext {
   }
 
   /**
-   * Like [getContext] but expands a union type into a list of types.
-   */
-  Iterable<DartType> getTypes(AstNode node) {
-    DartType t = getContext(node);
-    if (t == null) {
-      return DartType.EMPTY_LIST;
-    }
-    if (t is InterfaceType && t.isDartAsyncFutureOr) {
-      var tArg = t.typeArguments[0]; // The T in FutureOr<T>
-      return [
-        _typeProvider.futureType.instantiate([tArg]),
-        tArg
-      ];
-    }
-    return [t];
-  }
-
-  /**
-   * Match type [t1] against type [t2] as follows.
-   * If `t1 = I<dynamic, ..., dynamic>`, then look for a supertype
-   * of t1 of the form `K<S0, ..., Sm>` where `t2 = K<S0', ..., Sm'>`
-   * If the supertype exists, use the constraints `S0 <: S0', ... Sm <: Sm'`
-   * to derive a concrete instantation for I of the form `<T0, ..., Tn>`,
-   * such that `I<T0, .., Tn> <: t2`
-   */
-  List<DartType> matchTypes(DartType t1, DartType t2) =>
-      (t1 is InterfaceType && t2 is InterfaceType) ? _matchTypes(t1, t2) : null;
-
-  /**
    * Pop a return type off of the return stack.
    *
    * Also record any inferred return type using [setType], unless this node
@@ -4251,113 +4223,6 @@ class InferenceContext {
     _errorReporter.reportErrorForNode(error, node, [node, type]);
   }
 
-  List<DartType> _matchTypes(InterfaceType t1, InterfaceType t2) {
-    if (t1 == t2) {
-      return t2.typeArguments;
-    }
-    List<DartType> tArgs1 = t1.typeArguments;
-    List<DartType> tArgs2 = t2.typeArguments;
-    // If t1 isn't a raw type, bail out
-    if (tArgs1 != null && tArgs1.any((t) => !t.isDynamic)) {
-      return null;
-    }
-
-    // This is our inferred type argument list.  We start at all dynamic,
-    // and fill in with inferred types when we reach a match.
-    List<DartType> actuals =
-        new List<DartType>.filled(tArgs1.length, _typeProvider.dynamicType);
-
-    // When we find the supertype of t1 with the same
-    // classname as t2 (see below), we have the following:
-    // If t1 is an instantiation of a class T1<X0, ..., Xn>
-    // and t2 is an instantiation of a class T2<Y0, ...., Ym>
-    // of the form t2 = T2<S0, ..., Sm>
-    // then we want to choose instantiations for the Xi
-    // T0, ..., Tn such that T1<T0, ..., Tn> <: t2 .
-    // To find this, we simply instantate T1 with
-    // X0, ..., Xn, and then find its superclass
-    // T2<T0', ..., Tn'>.  We then solve the constraint
-    // set T0' <: S0, ..., Tn' <: Sn for the Xi.
-    // Currently, we only handle constraints where
-    // the Ti' is one of the Xi'.  If there are multiple
-    // constraints on some Xi, we choose the lower of the
-    // two (if it exists).
-    bool permute(List<DartType> permutedArgs) {
-      if (permutedArgs == null) {
-        return false;
-      }
-      List<TypeParameterElement> ps = t1.typeParameters;
-      List<DartType> ts = ps.map((p) => p.type).toList();
-      for (int i = 0; i < permutedArgs.length; i++) {
-        DartType tVar = permutedArgs[i];
-        DartType tActual = tArgs2[i];
-        int index = ts.indexOf(tVar);
-        if (index >= 0 && _typeSystem.isSubtypeOf(tActual, actuals[index])) {
-          actuals[index] = tActual;
-        }
-      }
-      return actuals.any((x) => !x.isDynamic);
-    }
-
-    // Look for the first supertype of t1 with the same class name as t2.
-    bool match(InterfaceType t1, Set<Element> visited) {
-      if (t1.element == t2.element) {
-        return permute(t1.typeArguments);
-      }
-
-      if (t1 == _typeProvider.objectType) {
-        return false;
-      }
-
-      Element element = t1.element;
-      if (visited == null) {
-        visited = new HashSet<Element>();
-      }
-      if (element == null || !visited.add(element)) {
-        return false;
-      }
-      try {
-        if (match(t1.superclass, visited)) {
-          return true;
-        }
-
-        List<InterfaceType> mixins = t1.mixins;
-        int mixinLength = mixins.length;
-        for (int i = 0; i < mixinLength; i++) {
-          if (match(mixins[i], visited)) {
-            return true;
-          }
-        }
-
-        List<InterfaceType> interfaces = t1.interfaces;
-        int interfaceLength = interfaces.length;
-        for (int j = 0; j < interfaceLength; j++) {
-          if (match(interfaces[j], visited)) {
-            return true;
-          }
-        }
-      } finally {
-        visited.remove(element);
-      }
-      return false;
-    }
-
-    // We have that t1 = T1<dynamic, ..., dynamic>.
-    // To match t1 against t2, we use the uninstantiated version
-    // of t1, essentially treating it as an instantiation with
-    // fresh variables, and solve for the variables.
-    // t1.element.type will be of the form T1<X0, ..., Xn>
-    if (!match(t1.element.type, null)) {
-      return null;
-    }
-    DartType newT1 = t1.element.type.instantiate(actuals);
-    // If we found a solution, return it.
-    if (_typeSystem.isSubtypeOf(newT1, t2)) {
-      return actuals;
-    }
-    return null;
-  }
-
   /**
    * Clear the type information assocated with [node].
    */
@@ -4366,28 +4231,40 @@ class InferenceContext {
   }
 
   /**
-   * Look for contextual type information attached to [node].  Returns
-   * the type if found, otherwise null.
+   * Look for contextual type information attached to [node], and returns
+   * the type if found.
    *
-   * If [node] has a contextual union type like `T | Future<T>` this will be
-   * returned. You can use [getType] if you prefer to only get the `T`.
+   * If [node] has a contextual union type like `T | Future<T>` or a type that
+   * contains `?` this will be returned. You can use [getValueContext] if you
+   * prefer to eliminate the Future union, or [getType] to eliminate both.
    */
   static DartType getContext(AstNode node) => node?.getProperty(_typeProperty);
 
   /**
-    * Look for a single contextual type attached to [node], and returns the type
-    * if found, otherwise null.
-    *
-    * If [node] has a contextual union type like `T | Future<T>` this will
-    * simplify it to only return `T`. If the caller can handle a union type,
-    * [getContext] should be used instead.
-    */
+   * Look for a single contextual type attached to [node], and returns the type
+   * if found, otherwise null.
+   *
+   * If [node] has a contextual union type like `T | Future<T>` this will
+   * simplify it to only return `T`. If the caller can handle a union type,
+   * [getContext] should be used instead.
+   */
   static DartType getType(AstNode node) {
     DartType t = getContext(node);
     if (t is InterfaceType && t.isDartAsyncFutureOr) {
-      return t.typeArguments[0]; // The T in FutureOr<T>
+      return _substituteForUnknown(t.typeArguments[0]); // The T in FutureOr<T>
     }
-    return t;
+    return _substituteForUnknown(t);
+  }
+
+  static DartType _substituteForUnknown(DartType t) {
+    if (t == null) return null;
+    // Since the type is being used for downwards inference, the expression
+    // type E must be a subtype of the context type T, i.e. T is an upper bound.
+    //
+    // TODO(jmesserly): our downwards inference code is not designed to handle

[Leaf, 2017/02/17 06:41:21]

What's the status of this?  We have user facing Null available now.  Should we
follow up in another CL, or can we DTRT here now?

[Jennifer Messerly, 2017/03/14 02:07:06]

fixed! new code handles Null

+    // the bottom type, so we need to prevent it from resulting here.
+    // Instead use `dynamic`. See `UnknownInferredType.upperBoundForType`.
+    return UnknownInferredType.substituteDynamic(t);
   }
 
   /**
@@ -5441,7 +5318,7 @@ class ResolverVisitor extends ScopedVisitor {
 
   @override
   Object visitArgumentList(ArgumentList node) {
-    DartType callerType = InferenceContext.getType(node);
+    DartType callerType = InferenceContext.getContext(node);
     if (callerType is FunctionType) {
       Map<String, DartType> namedParameterTypes =
           callerType.namedParameterTypes;
@@ -6079,7 +5956,7 @@ class ResolverVisitor extends ScopedVisitor {
   Object visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
     node.function?.accept(this);
     node.accept(elementResolver);
-    _inferArgumentTypesFromContext(node);
+    _inferArgumentTypesForInvocation(node);
     node.argumentList?.accept(this);
     node.accept(typeAnalyzer);
     return null;
@@ -6177,56 +6054,8 @@ class ResolverVisitor extends ScopedVisitor {
 
   @override
   Object visitInstanceCreationExpression(InstanceCreationExpression node) {
-    TypeName classTypeName = node.constructorName.type;
-    // TODO(leafp): Currently, we may re-infer types here, since we
-    // sometimes resolve multiple times.  We should really check that we
-    // have not already inferred something.  However, the obvious ways to
-    // check this don't work, since we may have been instantiated
-    // to bounds in an earlier phase, and we *do* want to do inference
-    // in that case.
-    if (classTypeName.typeArguments == null) {
-      // Given a union of context types ` T0 | T1 | ... | Tn`, find the first
-      // valid instantiation `new C<Ti>`, if it exists.
-      // TODO(jmesserly): if we support union types for real, `new C<Ti | Tj>`
-      // will become a valid possibility. Right now the only allowed union is
-      // `T | Future<T>` so we can take a simple approach.
-      for (var contextType in inferenceContext.getTypes(node)) {
-        if (contextType is InterfaceType &&
-            contextType.typeArguments != null &&
-            contextType.typeArguments.isNotEmpty) {
-          // TODO(jmesserly): for generic methods we use the
-          // StrongTypeSystemImpl.inferGenericFunctionCall, which appears to
-          // be a tad more powerful than matchTypes.
-          //
-          // For example it can infer this case:
-          //
-          //     class E<S, T> extends A<C<S>, T> { ... }
-          //     A<C<int>, String> a0 = /*infer<int, String>*/new E("hello");
-          //
-          // See _inferArgumentTypesFromContext in this file for use of it.
-          List<DartType> targs =
-              inferenceContext.matchTypes(classTypeName.type, contextType);
-          if (targs != null && targs.any((t) => !t.isDynamic)) {
-            ClassElement classElement =
-                resolutionMap.typeForTypeName(classTypeName).element;
-            InterfaceType rawType = classElement.type;
-            InterfaceType fullType =
-                rawType.substitute2(targs, rawType.typeArguments);
-            // The element resolver uses the type on the constructor name, so
-            // infer it first
-            typeAnalyzer.inferConstructorName(node.constructorName, fullType);
-            break;
-          }
-        }
-      }
-    }
     node.constructorName?.accept(this);
-    FunctionType constructorType = resolutionMap
-        .staticElementForConstructorReference(node.constructorName)
-        ?.type;
-    if (constructorType != null) {
-      InferenceContext.setType(node.argumentList, constructorType);
-    }
+    _inferArgumentTypesForInstanceCreate(node);
     node.argumentList?.accept(this);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
@@ -6241,18 +6070,18 @@ class ResolverVisitor extends ScopedVisitor {
 
   @override
   Object visitListLiteral(ListLiteral node) {
-    DartType contextType = InferenceContext.getType(node);
-    List<DartType> targs = null;
+    InterfaceType listT;
+
     if (node.typeArguments != null) {
-      targs = node.typeArguments.arguments.map((t) => t.type).toList();
-    } else if (contextType is InterfaceType) {
-      InterfaceType listD =
-          typeProvider.listType.instantiate([typeProvider.dynamicType]);
-      targs = inferenceContext.matchTypes(listD, contextType);
-    }
-    if (targs != null && targs.length == 1 && !targs[0].isDynamic) {
-      DartType eType = targs[0];
-      InterfaceType listT = typeProvider.listType.instantiate([eType]);
+      var targs = node.typeArguments.arguments.map((t) => t.type).toList();
+      if (targs.length == 1 && !targs[0].isDynamic) {
+        listT = typeProvider.listType.instantiate([targs[0]]);
+      }
+    } else if (strongMode) {
+      listT = typeAnalyzer.inferListType(node, downwards: true);
+    }
+    if (listT != null) {
+      DartType eType = listT.typeArguments[0];
       for (Expression child in node.elements) {
         InferenceContext.setType(child, eType);
       }
@@ -6266,19 +6095,18 @@ class ResolverVisitor extends ScopedVisitor {
 
   @override
   Object visitMapLiteral(MapLiteral node) {
-    DartType contextType = InferenceContext.getType(node);
-    List<DartType> targs = null;
+    InterfaceType mapT;
     if (node.typeArguments != null) {
-      targs = node.typeArguments.arguments.map((t) => t.type).toList();
-    } else if (contextType is InterfaceType) {
-      InterfaceType mapD = typeProvider.mapType
-          .instantiate([typeProvider.dynamicType, typeProvider.dynamicType]);
-      targs = inferenceContext.matchTypes(mapD, contextType);
-    }
-    if (targs != null && targs.length == 2 && targs.any((t) => !t.isDynamic)) {
-      DartType kType = targs[0];
-      DartType vType = targs[1];
-      InterfaceType mapT = typeProvider.mapType.instantiate([kType, vType]);
+      var targs = node.typeArguments.arguments.map((t) => t.type).toList();
+      if (targs.length == 2 && targs.any((t) => !t.isDynamic)) {
+        mapT = typeProvider.mapType.instantiate([targs[0], targs[1]]);
+      }
+    } else if (strongMode) {
+      mapT = typeAnalyzer.inferMapType(node, downwards: true);
+    }
+    if (mapT != null) {
+      DartType kType = mapT.typeArguments[0];
+      DartType vType = mapT.typeArguments[1];
       for (MapLiteralEntry entry in node.entries) {
         InferenceContext.setType(entry.key, kType);
         InferenceContext.setType(entry.value, vType);
@@ -6324,7 +6152,7 @@ class ResolverVisitor extends ScopedVisitor {
     node.target?.accept(this);
     node.typeArguments?.accept(this);
     node.accept(elementResolver);
-    _inferArgumentTypesFromContext(node);
+    _inferArgumentTypesForInvocation(node);
     node.argumentList?.accept(this);
     node.accept(typeAnalyzer);
     return null;
@@ -6617,12 +6445,13 @@ class ResolverVisitor extends ScopedVisitor {
         // If it's sync* we expect Iterable<T>
         // If it's async* we expect Stream<T>
         InterfaceType rawType = isAsynchronous
-            ? typeProvider.streamDynamicType
-            : typeProvider.iterableDynamicType;
+            ? typeProvider.streamType
+            : typeProvider.iterableType;
         // Match the types to instantiate the type arguments if possible
-        List<DartType> typeArgs =
-            inferenceContext.matchTypes(rawType, declaredType);
-        return (typeArgs?.length == 1) ? typeArgs[0] : null;
+        List<DartType> targs = declaredType.typeArguments;
+        if (targs.length == 1 && rawType.instantiate(targs) == declaredType) {
+          return targs[0];
+        }
       }
       // async functions expect `Future<T> | T`
       var futureTypeParam = declaredType.flattenFutures(typeSystem);
@@ -6721,33 +6550,101 @@ class ResolverVisitor extends ScopedVisitor {
     return false;
   }
 
-  void _inferArgumentTypesFromContext(InvocationExpression node) {
-    if (!strongMode) {
-      // Use propagated type inference for lambdas if not in strong mode.
-      _inferFunctionExpressionsParametersTypes(node.argumentList);
+  FunctionType _inferArgumentTypesForGeneric(
+      AstNode inferenceNode,
+      DartType uninstantiatedType,
+      TypeArgumentList typeArguments,
+      {AstNode errorNode}) {
+    errorNode ??= inferenceNode;
+    TypeSystem ts = typeSystem;
+    if (typeArguments == null &&
+        uninstantiatedType is FunctionType &&
+        uninstantiatedType.typeFormals.isNotEmpty &&
+        ts is StrongTypeSystemImpl) {
+      return ts.inferGenericFunctionOrType/*<FunctionType>*/(
+          uninstantiatedType,
+          ParameterElement.EMPTY_LIST,
+          DartType.EMPTY_LIST,
+          uninstantiatedType.returnType,
+          InferenceContext.getContext(inferenceNode),
+          downwards: true,
+          errorReporter: errorReporter,
+          errorNode: errorNode);
+    }
+    return null;
+  }
+
+  void _inferArgumentTypesForInstanceCreate(InstanceCreationExpression node) {
+    ConstructorName constructor = node.constructorName;
+    TypeName classTypeName = constructor?.type;
+    if (classTypeName == null || !strongMode) {
       return;
     }
 
-    DartType contextType = node.staticInvokeType;
-    if (contextType is FunctionType) {
-      DartType originalType = node.function.staticType;
-      DartType returnContextType = InferenceContext.getContext(node);
-      TypeSystem ts = typeSystem;
-      if (returnContextType != null &&
-          node.typeArguments == null &&
-          originalType is FunctionType &&
-          originalType.typeFormals.isNotEmpty &&
-          ts is StrongTypeSystemImpl) {
-        contextType = ts.inferGenericFunctionCall(
-            originalType,
-            DartType.EMPTY_LIST,
-            DartType.EMPTY_LIST,
-            originalType.returnType,
-            returnContextType);
+    ConstructorElement originalElement =
+        resolutionMap.staticElementForConstructorReference(constructor);
+    FunctionType inferred;
+    // If the constructor is generic, we'll have a ConstructorMember that
+    // substitutes in type arguments (possibly `dynamic`) from earlier in
+    // resolution.
+    //
+    // Otherwise we'll have a ConstructorElement, and we can skip inference
+    // because there's nothing to infer in a non-generic type.
+    if (classTypeName.typeArguments == null &&
+        originalElement is ConstructorMember) {
+      // TODO(leafp): Currently, we may re-infer types here, since we
+      // sometimes resolve multiple times.  We should really check that we
+      // have not already inferred something.  However, the obvious ways to
+      // check this don't work, since we may have been instantiated
+      // to bounds in an earlier phase, and we *do* want to do inference
+      // in that case.
+
+      // Get back to the uninstantiated generic constructor.
+      // TODO(jmesserly): should we store this earlier in resolution?
+      // Or look it up, instead of jumping backwards through the Member?
+      var rawElement = originalElement.baseElement;
+
+      FunctionType constructorType =
+          typeAnalyzer.constructorToGenericFunctionType(rawElement);
+
+      inferred = _inferArgumentTypesForGeneric(node, constructorType,
+          constructor.type.typeArguments, errorNode: node.constructorName);
+
+      if (inferred != null) {
+        ArgumentList arguments = node.argumentList;
+        InferenceContext.setType(arguments, inferred);
+        // Fix up the parameter elements based on inferred method.
+        arguments.correspondingStaticParameters =
+            resolveArgumentsToParameters(arguments, inferred.parameters, null);
+
+        constructor.type.type = inferred.returnType;
+        if (!UnknownInferredType.typeIsUnknown(inferred)) {
+          inferenceContext.recordInference(node, inferred.returnType);
+        }
+
+        // Update the static element as well. This is used in some cases, such
+        // as computing constant values. It is stored in two places.
+        constructor.staticElement =
+            ConstructorMember.from(rawElement, inferred.returnType);
+        node.staticElement = constructor.staticElement;
       }
+    }
+
+    if (inferred == null) {
+      InferenceContext.setType(node.argumentList, originalElement?.type);
+    }
+  }
 
-      InferenceContext.setType(node.argumentList, contextType);
+  void _inferArgumentTypesForInvocation(InvocationExpression node) {
+    if (!strongMode) {
+      // Use propagated type inference for lambdas if not in strong mode.
+      _inferFunctionExpressionsParametersTypes(node.argumentList);
+      return;
     }
+    DartType inferred = _inferArgumentTypesForGeneric(node,
+        node.function.staticType, node.typeArguments);
+    InferenceContext.setType(
+        node.argumentList, inferred ?? node.staticInvokeType);
   }
 
   void _inferFormalParameterList(FormalParameterList node, DartType type) {