Add checks for type bounds on generic methods

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

Index: pkg/analyzer/lib/src/generated/error_verifier.dart
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index c99057ad123a0ffe0645274036a38d55461dfbe9..410b45142727b7d16096d863121feb3be13781d0 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -770,6 +770,8 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       _errorReporter.reportErrorForNode(
           StaticTypeWarningCode.INVOCATION_OF_NON_FUNCTION_EXPRESSION,
           functionExpression);
+    } else if (expressionType is FunctionType) {
+      _checkTypeArguments(expressionType.element, node.typeArguments);
     }
     return super.visitFunctionExpressionInvocation(node);
   }
@@ -943,6 +945,8 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     }
     _checkForMissingRequiredParam(
         node.staticInvokeType, node.argumentList, methodName);
+    _checkTypeArguments(
+        node.methodName.staticElement, node.typeArguments, target?.staticType);
     return super.visitMethodInvocation(node);
   }
 
@@ -5217,13 +5221,14 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       int loopThroughIndex =
           math.min(typeNameArgList.length, boundingElts.length);
 
+      bool shouldSubstitute = typeArguments.length != 0 &&
+          typeArguments.length == typeParameters.length;
       for (int i = 0; i < loopThroughIndex; i++) {
         TypeName argTypeName = typeNameArgList[i];
         DartType argType = argTypeName.type;
         DartType boundType = boundingElts[i].bound;
         if (argType != null && boundType != null) {
-          if (typeArguments.length != 0 &&
-              typeArguments.length == typeParameters.length) {
+          if (shouldSubstitute) {
             boundType = boundType.substitute2(typeArguments, typeParameters);
           }
           if (!_typeSystem.isSubtypeOf(argType, boundType)) {
@@ -5646,6 +5651,79 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     }
   }
 
+  /**
+   * Verify that the given [typeArguments] are all within their bounds, as
+   * defined by the given [element].
+   *
+   * See [StaticTypeWarningCode.TYPE_ARGUMENT_NOT_MATCHING_BOUNDS].
+   */
+  void _checkTypeArguments(Element element, TypeArgumentList typeArguments,

[Jennifer Messerly, 2016/04/28 17:30:02]

one idea here: FunctionExpressionInvocation and MethodInvocation both implement
InvocationExpression, which would allow passing in the "node" and  having it be
like:

void _checkTypeArguments(InvocationExpression node) {
  Element element = node.function.staticElement;
  TypeArgumentList typeArguments = node.typeArguments;
  ... existing body ...
}

(I just rediscovered this trick yesterday working on generic methods in DDC. Was
quite pleased how much code I could share between FunctionExprInvocation and
MethodInvocation :) )

[Brian Wilkerson, 2016/04/28 20:46:11]

Yes, but I was hoping to use the same method for tear-offs, which don't
implement InvocationExpression.

+      [DartType targetType]) {
+    if (element == null || typeArguments == null) {
+      return;
+    }
+    void reportError(
+        TypeName argument, DartType argumentType, DartType parameterType) {
+      _errorReporter.reportTypeErrorForNode(
+          StaticTypeWarningCode.TYPE_ARGUMENT_NOT_MATCHING_BOUNDS,
+          argument,
+          [argumentType, parameterType]);
+    }
+    if (element is FunctionTypedElement) {

[Jennifer Messerly, 2016/04/28 17:30:02]

Would it make sense to use the type instead of the element? e.g.

DartType invokeType = node.function.staticType;
if (invokeType is FunctionType && invokeType.typeFormals.isNotEmpty) {
  // check type args match bounds
}

that way it works for any kind of element, since the element-to-staticType step
was already handled by element_resolver (_getInvokeType)

BTW, there is code already to check for number of type arguments passed:
https://github.com/dart-lang/sdk/blob/ee8bb2cea93b9b6e87d8c9309e2f22b46331864...

Not sure if it would make sense to consolidate?

[Brian Wilkerson, 2016/04/28 20:46:11]

Not sure. I'll look into that as part of the next CL.

+      _checkTypeArgumentsAgainstBounds(
+          element.typeParameters, typeArguments, targetType, reportError);
+    } else if (element is ClassElement) {

[Jennifer Messerly, 2016/04/28 17:30:02]

is ClassElement possible here? (it looks like it's used for generic
methods/functions)

[Brian Wilkerson, 2016/04/28 20:46:11]

Sorry, you're seeing an incomplete thought.

It isn't possible yet, but I want to convert
_checkForTypeArgumentNotMatchingBounds to share this code, at which point it
will be possible.

+      _checkTypeArgumentsAgainstBounds(
+          element.typeParameters, typeArguments, targetType, reportError);
+    } else if (element is ParameterElement || element is LocalVariableElement) {
+      // TODO(brianwilkerson) Implement this case
+    } else {
+      print('Unhandled element type: ${element.runtimeType}');
+    }
+  }
+
+  void _checkTypeArgumentsAgainstBounds(
+      List<TypeParameterElement> typeParameters,
+      TypeArgumentList typeArgumentList,
+      DartType targetType,
+      void reportError(
+          TypeName argument, DartType argumentType, DartType parameterType)) {
+    NodeList<TypeName> typeArguments = typeArgumentList.arguments;
+    int argumentsLength = typeArguments.length;
+    int maxIndex = math.min(typeParameters.length, argumentsLength);
+
+    bool shouldSubstitute =
+        argumentsLength != 0 && argumentsLength == typeParameters.length;
+    List<DartType> argumentTypes = shouldSubstitute
+        ? typeArguments.map((TypeName typeName) => typeName.type).toList()
+        : null;
+    List<DartType> parameterTypes = shouldSubstitute
+        ? typeParameters
+            .map((TypeParameterElement element) => element.type)
+            .toList()
+        : null;
+    for (int i = 0; i < maxIndex; i++) {
+      TypeName argTypeName = typeArguments[i];
+      DartType argType = argTypeName.type;
+      DartType boundType = typeParameters[i].bound;
+      if (argType != null && boundType != null) {
+        if (targetType is ParameterizedType) {
+          boundType = boundType.substitute2(
+              targetType.typeArguments,
+              targetType.typeParameters
+                  .map((TypeParameterElement element) => element.type)
+                  .toList());

[scheglov, 2016/04/28 17:14:52]

It looks that this expression can be extracted.

targetType.typeParameters
                  .map((TypeParameterElement element) => element.type)
                   .toList()

Consider computing it lazily.

[Brian Wilkerson, 2016/04/28 20:46:11]

Done

+        }
+        if (shouldSubstitute) {
+          boundType = boundType.substitute2(argumentTypes, parameterTypes);
+        }
+        if (!_typeSystem.isSubtypeOf(argType, boundType)) {
+          reportError(argTypeName, argType, boundType);
+        }
+      }
+    }
+  }
+
   DartType _computeReturnTypeForMethod(Expression returnExpression) {
     // This method should never be called for generators, since generators are
     // never allowed to contain return statements with expressions.