fix #29108, improve inference error messages

[email protected]

Review-Url: https://codereview.chromium.org/2757233004 .

Author: Jennifer Messerly

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

@@ -236,12 +236,7 @@ class StrongTypeSystemImpl extends TypeSystem {
     // subtypes (or supertypes) as necessary, and track the constraints that
     // are implied by this.
     var inferrer = new _GenericInferrer(typeProvider, this, fnType.typeFormals);
-
-    // Since we're trying to infer the instantiation, we want to ignore type
-    // formals as we check the parameters and return type.
-    var inferFnType =
-        fnType.instantiate(TypeParameterTypeImpl.getTypes(fnType.typeFormals));
-    inferrer.constrainReturnType(inferFnType, contextType);
+    inferrer.constrainGenericFunctionInContext(fnType, contextType);
 
     // Infer and instantiate the resulting type.
     return inferrer.infer(fnType, fnType.typeFormals,
@@ -1603,6 +1598,19 @@ class _GenericInferrer {
     _matchSubtypeOf(declaredType, contextType, null, origin, covariant: true);
   }
 
+  /// Constrain a universal function type [fnType] used in a context
+  /// [contextType].
+  void constrainGenericFunctionInContext(
+      FunctionType fnType, DartType contextType) {
+    var origin = new _TypeConstraintFromFunctionContext(fnType, contextType);
+
+    // Since we're trying to infer the instantiation, we want to ignore type
+    // formals as we check the parameters and return type.
+    var inferFnType =
+        fnType.instantiate(TypeParameterTypeImpl.getTypes(fnType.typeFormals));
+    _matchSubtypeOf(inferFnType, contextType, null, origin, covariant: true);
+  }
+
   /// Apply an argument constraint, which asserts that the [argument] staticType
   /// is a subtype of the [parameterType].
   void constrainArgument(
@@ -1851,20 +1859,31 @@ class _GenericInferrer {
       var constraints = _constraints[typeParam.element];
       var typeParamBound =
           typeParam.bound.substitute2(inferredTypes, fnTypeParams);
-      if (!typeParamBound.isDynamic) {
-        constraints
-            .add(new _TypeConstraint.fromExtends(typeParam, typeParamBound));
-      }
+
       var inferred = inferredTypes[i];
-      if (constraints.any((c) => !c.isSatisifedBy(_typeSystem, inferred))) {
-        // Heuristic: keep the erroneous type, it should satisfy at least some
-        // of the constraints (e.g. the return context). If we fall back to
-        // instantiateToBounds, we'll typically get more errors (e.g. because
-        // `dynamic` is the most common bound).
-        knownTypes[typeParam] = inferred;
-        errorReporter?.reportErrorForNode(StrongModeCode.COULD_NOT_INFER,
-            errorNode, [typeParam, _formatError(inferred, constraints)]);
-      } else if (UnknownInferredType.isKnown(inferred)) {
+      bool success =
+          constraints.every((c) => c.isSatisifedBy(_typeSystem, inferred));
+      if (success && !typeParamBound.isDynamic) {
+        // If everything else succeeded, check the `extends` constraint.
+        var extendsConstraint =
+            new _TypeConstraint.fromExtends(typeParam, typeParamBound);
+        constraints.add(extendsConstraint);
+        success = extendsConstraint.isSatisifedBy(_typeSystem, inferred);
+      }
+
+      if (!success) {
+        errorReporter?.reportErrorForNode(
+            StrongModeCode.COULD_NOT_INFER,
+            errorNode,
+            [typeParam, _formatError(typeParam, inferred, constraints)]);
+
+        // Heuristic: even if we failed, keep the erroneous type.
+        // It should satisfy at least some of the constraints (e.g. the return
+        // context). If we fall back to instantiateToBounds, we'll typically get
+        // more errors (e.g. because `dynamic` is the most common bound).
+      }
+
+      if (UnknownInferredType.isKnown(inferred)) {
         knownTypes[typeParam] = inferred;
       }
     }
@@ -2039,22 +2058,23 @@ class _GenericInferrer {
     return result;
   }
 
-  String _formatError(
-      DartType inferred, Iterable<_TypeConstraint> constraints) {
-    var intro = "Inferred type '$inferred' does not work with constraints:";
+  String _formatError(TypeParameterType typeParam, DartType inferred,
+      Iterable<_TypeConstraint> constraints) {
+    var intro = "Tried to infer '$inferred' for '$typeParam'"
+        " which doesn't work:";
 
     var constraintsByOrigin = <_TypeConstraintOrigin, List<_TypeConstraint>>{};
     for (var c in constraints) {
       constraintsByOrigin.putIfAbsent(c.origin, () => []).add(c);
     }
 
+    // Only report unique constraint origins.
     Iterable<_TypeConstraint> isSatisified(bool expected) => constraintsByOrigin
         .values
         .where((l) =>
             l.every((c) => c.isSatisifedBy(_typeSystem, inferred)) == expected)
         .expand((i) => i);
 
-    // Only report unique constraint origins.
     String unsatisified = _formatConstraints(isSatisified(false));
     String satisified = _formatConstraints(isSatisified(true));
 
@@ -2074,16 +2094,19 @@ class _GenericInferrer {
             .toList();
 
     int prefixMax = lineParts.map((p) => p[0].length).fold(0, math.max);
-    int middleMax = lineParts.map((p) => p[1].length).fold(0, math.max);
 
     // Use a set to prevent identical message lines.
     // (It's not uncommon for the same constraint to show up in a few places.)
     var messageLines = new Set<String>.from(lineParts.map((parts) {
       var prefix = parts[0];
       var middle = parts[1];
       var prefixPad = ' ' * (prefixMax - prefix.length);
-      var middlePad = ' ' * (middleMax - middle.length);
-      return '  $prefix$prefixPad $middle$middlePad ${parts[2]}'.trimRight();
+      var middlePad = ' ' * (prefixMax);
+      var end = "";
+      if (parts.length > 2) {
+        end = '\n  $middlePad ${parts[2]}';
+      }
+      return '  $prefix$prefixPad $middle$end';
     }));
 
     return messageLines.join('\n');
@@ -2121,13 +2144,13 @@ class _TypeConstraintFromArgument extends _TypeConstraintOrigin {
       //     "Map value"
       prefix = "${genericType.name} $parameterName";
     } else {
-      prefix = "Argument '$parameterName'";
+      prefix = "Parameter '$parameterName'";
     }
 
     return [
       prefix,
-      "inferred as '$argumentType'",
-      "must be a '$parameterType'."
+      "declared as     '$parameterType'",
+      "but argument is '$argumentType'."
     ];
   }
 }
@@ -2143,7 +2166,23 @@ class _TypeConstraintFromReturnType extends _TypeConstraintOrigin {
     return [
       "Return type",
       "declared as '$declaredType'",
-      "used where a '$contextType' is required."
+      "used where  '$contextType' is required."
+    ];
+  }
+}
+
+class _TypeConstraintFromFunctionContext extends _TypeConstraintOrigin {
+  final DartType contextType;
+  final DartType functionType;
+
+  _TypeConstraintFromFunctionContext(this.functionType, this.contextType);
+
+  @override
+  formatError() {
+    return [
+      "Function type",
+      "declared as '$functionType'",
+      "used where  '$contextType' is required."
     ];
   }
 }
@@ -2158,8 +2197,7 @@ class _TypeConstraintFromExtendsClause extends _TypeConstraintOrigin {
   formatError() {
     return [
       "Type parameter '$typeParam'",
-      "declared to extend '$extendsType'.",
-      ""
+      "declared to extend '$extendsType'."
     ];
   }
 }

pkg/analyzer/test/generated/strong_mode_test.dart

@@ -1098,6 +1098,176 @@ class StrongModeLocalInferenceTest extends ResolverTestCase {
     _isInstantiationOf(_hasElement(elementC))([_isDynamic])(cType);
   }
 
+  test_inference_error_arguments() async {
+    Source source = addSource(r'''
+typedef R F<T, R>(T t);
+
+F<T, T> g<T>(F<T, T> f) => (x) => f(f(x));
+
+test() {
+  var h = g((int x) => 42.0);
+}
+ ''');
+    await computeAnalysisResult(source);
+    _expectInferenceError(
+        source,
+        [
+          StrongModeCode.COULD_NOT_INFER,
+          StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE
+        ],
+        r'''
+Couldn't infer type parameter 'T'.
+
+Tried to infer 'double' for 'T' which doesn't work:
+  Parameter 'f' declared as     '(T) → T'
+                but argument is '(int) → double'.
+
+Consider passing explicit type argument(s) to the generic.
+
+''');
+  }
+
+  test_inference_error_arguments2() async {
+    Source source = addSource(r'''
+typedef R F<T, R>(T t);
+
+F<T, T> g<T>(F<T, T> a, F<T, T> b) => (x) => a(b(x));
+
+test() {
+  var h = g((int x) => 42.0, (double x) => 42);
+}
+ ''');
+    await computeAnalysisResult(source);
+    _expectInferenceError(
+        source,
+        [
+          StrongModeCode.COULD_NOT_INFER,
+          StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE,
+          StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE
+        ],
+        r'''
+Couldn't infer type parameter 'T'.
+
+Tried to infer 'num' for 'T' which doesn't work:
+  Parameter 'a' declared as     '(T) → T'
+                but argument is '(int) → double'.
+  Parameter 'b' declared as     '(T) → T'
+                but argument is '(double) → int'.
+
+Consider passing explicit type argument(s) to the generic.
+
+''');
+  }
+
+  test_inference_error_extendsFromReturn() async {
+    // This is not an inference error because we successfully infer Null.
+    Source source = addSource(r'''
+T max<T extends num>(T x, T y) => x;
+
+test() {
+  String hello = max(1, 2);
+}
+ ''');
+    var analysisResult = await computeAnalysisResult(source);
+    assertErrors(source, [
+      StrongModeCode.INVALID_CAST_LITERAL,
+      StrongModeCode.INVALID_CAST_LITERAL
+    ]);
+    var unit = analysisResult.unit;
+    var h = (AstFinder.getStatementsInTopLevelFunction(unit, "test")[0]
+            as VariableDeclarationStatement)
+        .variables
+        .variables[0];
+    var call = h.initializer as MethodInvocation;
+    expect(call.staticInvokeType.toString(), '(Null, Null) → Null');
+  }
+
+  test_inference_error_extendsFromReturn2() async {
+    Source source = addSource(r'''
+typedef R F<T, R>(T t);
+F<T, T> g<T extends num>() => (y) => y;
+
+test() {
+  F<String, String> hello = g();
+}
+ ''');
+    await computeAnalysisResult(source);
+    _expectInferenceError(
+        source,
+        [
+          StrongModeCode.COULD_NOT_INFER,
+        ],
+        r'''
+Couldn't infer type parameter 'T'.
+
+Tried to infer 'String' for 'T' which doesn't work:
+  Type parameter 'T' declared to extend 'num'.
+The type 'String' was inferred from:
+  Return type declared as '(T) → T'
+              used where  '(String) → String' is required.
+
+Consider passing explicit type argument(s) to the generic.
+
+''');
+  }
+
+
+  test_inference_error_genericFunction() async {
+    Source source = addSource(r'''
+T max<T extends num>(T x, T y) => x < y ? y : x;
+abstract class Iterable<T> {
+  T get first;
+  S fold<S>(S s, S f(S s, T t));
+}
+test(Iterable values) {
+  num n = values.fold(values.first as num, max);
+}
+ ''');
+    await computeAnalysisResult(source);
+    _expectInferenceError(
+        source,
+        [
+          StrongModeCode.COULD_NOT_INFER,
+          StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE
+        ],
+        r'''
+Couldn't infer type parameter 'T'.
+
+Tried to infer 'dynamic' for 'T' which doesn't work:
+  Function type declared as '<T extends num>(T, T) → T'
+                used where  '(num, dynamic) → num' is required.
+
+Consider passing explicit type argument(s) to the generic.
+
+''');
+  }
+
+  test_inference_error_returnContext() async {
+    Source source = addSource(r'''
+typedef R F<T, R>(T t);
+
+F<T, T> g<T>(T t) => (x) => t;
+
+test() {
+  F<num, int> h = g(42);
+}
+ ''');
+    await computeAnalysisResult(source);
+    _expectInferenceError(
+        source,
+        [StrongModeCode.COULD_NOT_INFER],
+        r'''
+Couldn't infer type parameter 'T'.
+
+Tried to infer 'num' for 'T' which doesn't work:
+  Return type declared as '(T) → T'
+              used where  '(num) → int' is required.
+
+Consider passing explicit type argument(s) to the generic.
+
+''');
+  }
+
   test_inference_hints() async {
     Source source = addSource(r'''
       void main () {
@@ -2137,6 +2307,23 @@ num test(Iterable values) => values.fold(values.first as num, max);
     check("f3", _isListOf((DartType type) => _isListOf(_isInt)(type)));
   }
 
+  /// Verifies the source has the expected [errorCodes] as well as the
+  /// expected [errorMessage].
+  void _expectInferenceError(
+      Source source, List<ErrorCode> errorCodes, String errorMessage) {
+    assertErrors(source, errorCodes);
+    var errors = analysisResults[source]
+        .errors
+        .where((e) => e.errorCode == StrongModeCode.COULD_NOT_INFER)
+        .map((e) => e.message)
+        .toList();
+    expect(errors.length, 1);
+    var actual = errors[0];
+    expect(actual,
+        errorMessage, // Print the literal error message for easy copy+paste:
+        reason: 'Actual error did not match expected error:\n$actual');
+  }
+
   /// Helper method for testing `FutureOr<T>`.
   ///
   /// Validates that [code] produces [errors]. It should define a function