fix #29108, improve inference error messages

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

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library analyzer.src.generated.type_system;
 
 import 'dart:collection';
 import 'dart:math' as math;
 
 import 'package:analyzer/dart/ast/ast.dart' show AstNode;
@@ skipping 218 matching lines @@
       {ErrorReporter errorReporter, AstNode errorNode}) {
     if (contextType.typeFormals.isNotEmpty || fnType.typeFormals.isEmpty) {
       return fnType;
     }
 
     // Create a TypeSystem that will allow certain type parameters to be
     // inferred. It will optimistically assume these type parameters can be
     // 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,
         errorReporter: errorReporter, errorNode: errorNode);
   }
 
   /// Infers a generic type, function, method, or list/map literal
   /// instantiation, using the downward context type as well as the argument
   /// types if available.
   ///
@@ skipping 1341 matching lines @@
     }
   }
 
   /// Apply a return type constraint, which asserts that the [declaredType]
   /// is a subtype of the [contextType].
   void constrainReturnType(DartType declaredType, DartType contextType) {
     var origin = new _TypeConstraintFromReturnType(declaredType, contextType);
     _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(
       DartType argumentType, DartType parameterType, String parameterName,
       {DartType genericType}) {
     var origin = new _TypeConstraintFromArgument(
         argumentType, parameterType, parameterName,
         genericType: genericType);
     _matchSubtypeOf(argumentType, parameterType, null, origin,
         covariant: false);
@@ skipping 228 matching lines @@
 
     // Check the inferred types against all of the constraints.
     var knownTypes = new HashMap<TypeParameterType, DartType>(
         equals: (x, y) => x.element == y.element,
         hashCode: (x) => x.element.hashCode);
     for (int i = 0; i < fnTypeParams.length; i++) {
       TypeParameterType typeParam = fnTypeParams[i];
       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];
+      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);
       }
-      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)) {
+
+      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;
       }
     }
 
     // Use instantiate to bounds to finish things off.
     var hasError = new List<bool>.filled(fnTypeParams.length, false);
     var result = _typeSystem.instantiateToBounds(genericType,
         hasError: hasError, knownTypes: knownTypes) as dynamic/*=T*/;
 
     // Report any errors from instantiateToBounds.
@@ skipping 154 matching lines @@
       if (t2 is InterfaceType && t2.isDartAsyncFutureOr) {
         // GLB(A, FutureOr<B>) ==  GLB(FutureOr<A>, B)
         return _getGreatestLowerBound(t2, t1);
       }
       // TODO(jmesserly): fix this rule once we support non-nullable types.
       return typeProvider.nullType;
     }
     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));
 
     assert(unsatisified.isNotEmpty);
     if (satisified.isNotEmpty) {
       satisified = "\nThe type '$inferred' was inferred from:\n$satisified";
     }
 
     return '\n\n$intro\n$unsatisified$satisified\n\n'
         'Consider passing explicit type argument(s) to the generic.\n\n';
   }
 
   static String _formatConstraints(Iterable<_TypeConstraint> constraints) {
     List<List<String>> lineParts =
         new Set<_TypeConstraintOrigin>.from(constraints.map((c) => c.origin))
             .map((o) => o.formatError())
             .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');
   }
 }
 
 /// The origin of a type constraint, for the purposes of producing a human
 /// readable error message during type inference as well as determining whether
 /// the constraint was used to fix the type parameter or not.
 abstract class _TypeConstraintOrigin {
@@ skipping 17 matching lines @@
     // available.
     String prefix;
     if ((genericType.name == "List" || genericType.name == "Map") &&
         genericType?.element?.library?.isDartCore == true) {
       // This will become:
       //     "List element"
       //     "Map key"
       //     "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'."
     ];
   }
 }
 
 class _TypeConstraintFromReturnType extends _TypeConstraintOrigin {
   final DartType contextType;
   final DartType declaredType;
 
   _TypeConstraintFromReturnType(this.declaredType, this.contextType);
 
   @override
   formatError() {
     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."
+    ];
+  }
+}
 
 class _TypeConstraintFromExtendsClause extends _TypeConstraintOrigin {
   final TypeParameterType typeParam;
   final DartType extendsType;
 
   _TypeConstraintFromExtendsClause(this.typeParam, this.extendsType);
 
   @override
   formatError() {
     return [
       "Type parameter '$typeParam'",
-      "declared to extend '$extendsType'.",
-      ""
+      "declared to extend '$extendsType'."
     ];
   }
 }
 
 class _TypeRange {
   /// The upper bound of the type parameter. In other words, T <: upperBound.
   ///
   /// In Dart this can be written as `<T extends UpperBoundType>`.
   ///
   /// In inference, this can happen as a result of parameters of function type.
@@ skipping 159 matching lines @@
     if (type is InterfaceTypeImpl) {
       return type.typeArguments.any(isUnknown);
     }
     if (type is FunctionType) {
       return isUnknown(type.returnType) ||
           type.parameters.any((p) => isUnknown(p.type));
     }
     return false;
   }
 }