fix #25220, infer generic type from constructor arguments

this makes it consistent in how constructors, generic functions, and literals are treated

includes changes in https://codereview.chromium.org/2115743002/

[email protected]

Review URL: https://codereview.chromium.org/2115173002 .

Author: John Messerly

pkg/analyzer/CHANGELOG.md

@@ -1,3 +1,6 @@
+## Next release
+* Strong mode breaking change: can now infer generic type arguments from the constructor invocation arguments (#25220).
+
 ## 0.27.4-alpha.16
 * (Internal) Corresponds with the analyzer/server in the `1.18.0-dev.4.0` SDK.
 

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

@@ -6723,6 +6723,16 @@ class ResolverVisitor extends ScopedVisitor {
       if (contextType is InterfaceType &&
           contextType.typeArguments != null &&
           contextType.typeArguments.length > 0) {
+        // 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)) {

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

@@ -12,6 +12,7 @@ import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.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/generated/java_engine.dart';
 import 'package:analyzer/src/generated/resolver.dart';
@@ -521,7 +522,7 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
   @override
   Object visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
     if (_strongMode) {
-      _inferGenericInvoke(node);
+      _inferGenericInvocationExpression(node);
     }
     DartType staticType = _computeInvokeReturnType(node.staticInvokeType);
     _recordStaticType(node, staticType);
@@ -574,6 +575,10 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
    */
   @override
   Object visitInstanceCreationExpression(InstanceCreationExpression node) {
+    if (_strongMode) {
+      _inferInstanceCreationExpression(node);
+    }
+
     _recordStaticType(node, node.constructorName.type.type);
     ConstructorElement element = node.staticElement;
     if (element != null && "Element" == element.enclosingElement.name) {
@@ -812,7 +817,7 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
     SimpleIdentifier methodNameNode = node.methodName;
     Element staticMethodElement = methodNameNode.staticElement;
     if (_strongMode) {
-      _inferGenericInvoke(node);
+      _inferGenericInvocationExpression(node);
     }
     // Record types of the variable invoked as a function.
     if (staticMethodElement is VariableElement) {
@@ -1605,6 +1610,33 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
     return returnType.type;
   }
 
+  /**
+   * Given a constructor for a generic type, returns the equivalent generic
+   * function type that we could use to forward to the constructor, or for a
+   * non-generic type simply returns the constructor type.
+   *
+   * For example given the type `class C<T> { C(T arg); }`, the generic function
+   * type is `<T>(T) -> C<T>`.
+   */
+  FunctionType _constructorToGenericFunctionType(
+      ConstructorElement constructor) {
+    // TODO(jmesserly): it may be worth making this available from the
+    // constructor. It's nice if our inference code can operate uniformly on
+    // function types.
+    ClassElement cls = constructor.enclosingElement;
+    FunctionType type = constructor.type;
+    if (cls.typeParameters.isEmpty) {
+      return type;
+    }
+
+    FunctionElementImpl function = new FunctionElementImpl("", -1);
+    function.synthetic = true;
+    function.returnType = type.returnType;
+    function.shareTypeParameters(cls.typeParameters);
+    function.shareParameters(type.parameters);
+    return function.type = new FunctionTypeImpl(function);
+  }
+
   DartType _findIteratedType(DartType type, DartType targetType) {
     // TODO(vsm): Use leafp's matchType here?
     // Set by _find if match is found
@@ -1864,8 +1896,8 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
   }
 
   /**
-   * Given an uninstantiated generic type, try to infer the instantiated generic
-   * type from the surrounding context.
+   * Given an uninstantiated generic function type, try to infer the
+   * instantiated generic function type from the surrounding context.
    */
   DartType _inferGenericInstantiationFromContext(
       DartType context, DartType type) {
@@ -1878,14 +1910,40 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
     return type;
   }
 
-  bool _inferGenericInvoke(InvocationExpression node) {
+  /**
+   * Given a possibly generic invocation like `o.m(args)` or `(f)(args)` try to
+   * infer the instantiated generic function type.
+   *
+   * This takes into account both the context type, as well as information from
+   * the argument types.
+   */
+  void _inferGenericInvocationExpression(InvocationExpression node) {
+    ArgumentList arguments = node.argumentList;
+    FunctionType inferred = _inferGenericInvoke(
+        node, node.function.staticType, node.typeArguments, arguments);
+    if (inferred != null && inferred != node.staticInvokeType) {
+      // Fix up the parameter elements based on inferred method.
+      arguments.correspondingStaticParameters = ResolverVisitor
+          .resolveArgumentsToParameters(arguments, inferred.parameters, null);
+      node.staticInvokeType = inferred;
+    }
+  }
+
+  /**
+   * Given a possibly generic invocation or instance creation, such as
+   * `o.m(args)` or `(f)(args)` or `new T(args)` try to infer the instantiated
+   * generic function type.
+   *
+   * This takes into account both the context type, as well as information from
+   * the argument types.
+   */
+  FunctionType _inferGenericInvoke(Expression node, DartType fnType,
+      TypeArgumentList typeArguments, ArgumentList argumentList) {
     TypeSystem ts = _typeSystem;
-    DartType fnType = node.function.staticType;
-    if (node.typeArguments == null &&
+    if (typeArguments == null &&
         fnType is FunctionType &&
         fnType.typeFormals.isNotEmpty &&
         ts is StrongTypeSystemImpl) {
-      ArgumentList argumentList = node.argumentList;
       // Get the parameters that correspond to the uninstantiated generic.
       List<ParameterElement> rawParameters = ResolverVisitor
           .resolveArgumentsToParameters(argumentList, fnType.parameters, null);
@@ -1900,20 +1958,48 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
         }
       }
 
-      FunctionType inferred = ts.inferGenericFunctionCall(_typeProvider, fnType,
-          paramTypes, argTypes, InferenceContext.getType(node));
+      return ts.inferGenericFunctionCall(_typeProvider, fnType, paramTypes,
+          argTypes, InferenceContext.getType(node));
+    }
+    return null;
+  }
 
-      if (inferred != node.staticInvokeType) {
-        // Fix up the parameter elements based on inferred method.
-        List<ParameterElement> inferredParameters =
-            ResolverVisitor.resolveArgumentsToParameters(
-                argumentList, inferred.parameters, null);
-        argumentList.correspondingStaticParameters = inferredParameters;
-        node.staticInvokeType = inferred;
-        return true;
-      }
+  /**
+   * Given an instance creation of a possibly generic type, infer the type
+   * arguments using the current context type as well as the argument types.
+   */
+  void _inferInstanceCreationExpression(InstanceCreationExpression node) {
+    ConstructorName constructor = node.constructorName;
+    ConstructorElement originalElement = constructor.staticElement;
+    // 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 (originalElement is! ConstructorMember) {
+      return;
+    }
+
+    // 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 as ConstructorMember).baseElement;
+
+    FunctionType constructorType =
+        _constructorToGenericFunctionType(rawElement);
+
+    ArgumentList arguments = node.argumentList;
+    FunctionType inferred = _inferGenericInvoke(
+        node, constructorType, constructor.type.typeArguments, arguments);
+
+    if (inferred != null && inferred != originalElement.type) {
+      // Fix up the parameter elements based on inferred method.
+      arguments.correspondingStaticParameters = ResolverVisitor
+          .resolveArgumentsToParameters(arguments, inferred.parameters, null);
+      inferConstructorName(constructor, inferred.returnType);
+      // TODO(jmesserly): should we fix up the staticElement as well?
     }
-    return false;
   }
 
   /**

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

@@ -167,8 +167,7 @@ class StrongTypeSystemImpl extends TypeSystem {
     }
 
     // Try to infer and instantiate the resulting type.
-    var resultType =
-        inferringTypeSystem._infer(fnType, allowPartialSolution: false);
+    var resultType = inferringTypeSystem._infer(fnType);
 
     // If the instantiation failed (because some type variable constraints
     // could not be solved, in other words, we could not find a valid subtype),
@@ -240,14 +239,7 @@ class StrongTypeSystemImpl extends TypeSystem {
           argumentTypes[i], correspondingParameterTypes[i]);
     }
 
-    // We are okay inferring some type variables and not others.
-    //
-    // This lets our return type be as precise as possible, which will help
-    // make any type information resulting from it more precise.
-    //
-    // This is simply a heuristic: the code is incorrect, and we'll issue an
-    // error on this call, to indicate that types don't match.
-    return inferringTypeSystem._infer(fnType, allowPartialSolution: true);
+    return inferringTypeSystem._infer(fnType);
   }
 
   /**
@@ -1282,7 +1274,7 @@ class _StrongInferenceTypeSystem extends StrongTypeSystemImpl {
 
   /// Given the constraints that were given by calling [isSubtypeOf], find the
   /// instantiation of the generic function that satisfies these constraints.
-  FunctionType _infer(FunctionType fnType, {bool allowPartialSolution: false}) {
+  FunctionType _infer(FunctionType fnType) {
     List<TypeParameterType> fnTypeParams =
         TypeParameterTypeImpl.getTypes(fnType.typeFormals);
 
@@ -1335,16 +1327,8 @@ class _StrongInferenceTypeSystem extends StrongTypeSystemImpl {
               bound.upper.substitute2(inferredTypes, fnTypeParams)) ||
           !isSubtypeOf(bound.lower.substitute2(inferredTypes, fnTypeParams),
               inferredTypes[i])) {
-        // Unless a partial solution was requested, bail.
-        if (!allowPartialSolution) {
-          return null;
-        }
-
-        inferredTypes[i] = DynamicTypeImpl.instance;
-        if (typeParam.element.bound != null) {
-          inferredTypes[i] =
-              typeParam.element.bound.substitute2(inferredTypes, fnTypeParams);
-        }
+        // Inference failed. Bail.
+        return null;
       }
     }
 

pkg/analyzer/test/generated/strong_mode_test.dart

@@ -798,7 +798,6 @@ class StrongModeDownwardsInferenceTest extends ResolverTestCase {
         A<int, String> a1 = new D.named(3);
       }
       void test8() {
-        // Currently we only allow variable constraints.  Test that we reject.
         A<C<int>, String> a0 = new E("hello");
       }
       void test9() { // Check named and optional arguments
@@ -921,7 +920,7 @@ class StrongModeDownwardsInferenceTest extends ResolverTestCase {
     {
       List<Statement> statements =
           AstFinder.getStatementsInTopLevelFunction(unit, "test8");
-      hasType(assertEOf([_isDynamic, _isDynamic]), rhs(statements[0]));
+      hasType(assertEOf([_isInt, _isString]), rhs(statements[0]));
     }
 
     {

pkg/analyzer/test/generated/type_system_test.dart

@@ -891,9 +891,7 @@ class StrongGenericFunctionInferenceTest {
     expect(_inferCall(clone, [foo.type, foo.type]), [foo.type]);
 
     // Something invalid...
-    expect(_inferCall(clone, [stringType, numType]), [
-      clonable.type.instantiate([dynamicType])
-    ]);
+    expect(_inferCall(clone, [stringType, numType]), null);
   }
 
   void test_genericCastFunction() {
@@ -1007,7 +1005,7 @@ class StrongGenericFunctionInferenceTest {
     // <T extends num>() -> T
     var t = TypeBuilder.variable('T', bound: numType);
     var f = TypeBuilder.function(types: [t], required: [], result: t);
-    expect(_inferCall(f, [], stringType), [numType]);
+    expect(_inferCall(f, [], stringType), null);
   }
 
   void test_unifyParametersToFunctionParam() {
@@ -1024,7 +1022,7 @@ class StrongGenericFunctionInferenceTest {
           TypeBuilder.function(required: [intType], result: dynamicType),
           TypeBuilder.function(required: [doubleType], result: dynamicType)
         ]),
-        [dynamicType]);
+        null);
   }
 
   void test_unusedReturnTypeIsDynamic() {
@@ -1045,7 +1043,7 @@ class StrongGenericFunctionInferenceTest {
       [DartType returnType]) {
     FunctionType inferred = typeSystem.inferGenericFunctionCall(typeProvider,
         ft, ft.parameters.map((p) => p.type).toList(), arguments, returnType);
-    return inferred.typeArguments;
+    return inferred?.typeArguments;
   }
 }
 

pkg/analyzer/test/src/summary/resynthesize_ast_test.dart

@@ -155,6 +155,51 @@ class AstInferredTypeTest extends AbstractResynthesizeTest
     super.test_circularReference_viaClosures_initializerTypes();
   }
 
+  @override
+  @failingTest
+  void test_constructors_inferFromArguments() {
+    // TODO(jmesserly): does this need to be implemented in AST summaries?
+    // The test might need a change as well to not be based on local variable
+    // types, which don't seem to be available.
+    super.test_constructors_inferFromArguments();
+  }
+
+  @override
+  @failingTest
+  void test_constructors_inferFromArguments_const() {
+    super.test_constructors_inferFromArguments_const();
+  }
+
+  @override
+  @failingTest
+  void test_constructors_inferFromArguments_factory() {
+    super.test_constructors_inferFromArguments_factory();
+  }
+
+  @override
+  @failingTest
+  void test_constructors_inferFromArguments_named() {
+    super.test_constructors_inferFromArguments_named();
+  }
+
+  @override
+  @failingTest
+  void test_constructors_inferFromArguments_namedFactory() {
+    super.test_constructors_inferFromArguments_namedFactory();
+  }
+
+  @override
+  @failingTest
+  void test_constructors_inferFromArguments_redirecting() {
+    super.test_constructors_inferFromArguments_redirecting();
+  }
+
+  @override
+  @failingTest
+  void test_constructors_inferFromArguments_redirectingFactory() {
+    super.test_constructors_inferFromArguments_redirectingFactory();
+  }
+
   @override
   @failingTest
   void test_genericMethods_inferJSBuiltin() {