support generic methods in FunctionExpressionInvocation node

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

Index: pkg/analyzer/lib/src/generated/element_resolver.dart
diff --git a/pkg/analyzer/lib/src/generated/element_resolver.dart b/pkg/analyzer/lib/src/generated/element_resolver.dart
index 57b8060e5a88a58076ae9e7d7b372c382969978a..44efe2d7a1bcce8fa59a77acbb0d5f742753a9bc 100644
--- a/pkg/analyzer/lib/src/generated/element_resolver.dart
+++ b/pkg/analyzer/lib/src/generated/element_resolver.dart
@@ -426,17 +426,26 @@ class ElementResolver extends SimpleAstVisitor<Object> {
 
   @override
   Object visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
-    // TODO(brianwilkerson) Can we ever resolve the function being invoked?
-    Expression expression = node.function;
-    if (expression is FunctionExpression) {
-      FunctionExpression functionExpression = expression;
-      ExecutableElement functionElement = functionExpression.element;
-      ArgumentList argumentList = node.argumentList;
-      List<ParameterElement> parameters =
-          _resolveArgumentsToFunction(false, argumentList, functionElement);
-      if (parameters != null) {
-        argumentList.correspondingStaticParameters = parameters;
-      }
+    Expression function = node.function;
+    DartType staticInvokeType =
+        _resolveGenericMethod(function.staticType, node.typeArguments, node);
+    DartType propagatedInvokeType = _resolveGenericMethod(
+        function.propagatedType, node.typeArguments, node);
+
+    node.staticInvokeType = staticInvokeType;
+    node.propagatedInvokeType =
+        _propagatedInvokeTypeIfBetter(propagatedInvokeType, staticInvokeType);
+
+    List<ParameterElement> parameters =
+        _computeCorrespondingParameters(node.argumentList, staticInvokeType);
+    if (parameters != null) {
+      node.argumentList.correspondingStaticParameters = parameters;
+    }
+
+    parameters = _computeCorrespondingParameters(
+        node.argumentList, propagatedInvokeType);
+    if (parameters != null) {
+      node.argumentList.correspondingPropagatedParameters = parameters;
     }
     return null;
   }
@@ -652,17 +661,16 @@ class ElementResolver extends SimpleAstVisitor<Object> {
     methodName.propagatedElement = propagatedElement;
 
     node.staticInvokeType = staticInvokeType;
-    if (propagatedInvokeType != null &&
-        (staticInvokeType == null ||
-            propagatedInvokeType.isMoreSpecificThan(staticInvokeType))) {
-      // Don't store the propagated invoke type unless it's more specific than
-      // the static type. We still need to record the propagated parameter
-      // elements however, as that is used for the propagatedType downwards
-      // inference of lambda parameters.
-      node.propagatedInvokeType = propagatedInvokeType;
-    } else {
-      node.propagatedInvokeType = null;
-    }
+    //
+    // Store the propagated invoke type if it's more specific than the static
+    // type.
+    //
+    // We still need to record the propagated parameter elements however,
+    // as they are used in propagatedType downwards inference of lambda
+    // parameters. So we don't want to clear the propagatedInvokeType variable.
+    //
+    node.propagatedInvokeType =
+        _propagatedInvokeTypeIfBetter(propagatedInvokeType, staticInvokeType);
 
     ArgumentList argumentList = node.argumentList;
     if (staticInvokeType != null) {
@@ -1328,6 +1336,25 @@ class ElementResolver extends SimpleAstVisitor<Object> {
     return null;
   }
 
+  DartType _computeMethodInvokeType(MethodInvocation node, Element element) {
+    if (element == null) {
+      // TODO(jmesserly): should we return `dynamic` in this case?

[Brian Wilkerson, 2016/01/08 01:02:21]

No.

(1) We use `null` to mean that there is no type better than static, and doing so
gives us the ability to not do extra work when there's no need to. We don't
check for dynamic, so we'd end up carrying this type forward needlessly.

(2) When we ask for the 'best' type (for example, in code completion) we use the
propagated type when it's available (not null), and the static when it isn't. We
assume that the propagated type is always better without checking. Therefore,
returning dynamic would potentially worsen the type information used in lots of
other places.

[Jennifer Messerly, 2016/01/11 18:49:09]

So right now this method is used to populate "staticInvokeType" and
"propagatedInvokeType".

Does it ever make sense to have a `null` staticInvokeType? That was my main
concern that led to adding a TODO.

However, even for propagatedInvokeType: if we return `dynamic`, we will end up
storing `null`, because of the logic that discards propagatedInvokeType unless
it is better. `dynamic` can never be more specific than another type. Or at
least I would hope so.

Because of that I think returning `dynamic` would have no effect on the
propagated type code paths. Hence the TODO.

I added some more text in an attempt to clarify it. 

[Brian Wilkerson, 2016/01/11 18:56:41]

No. I was distracted by the propagated path. We should always have static type
information, even if it's 'dynamic'.
That's reasonable.

+      return null;
+    }
+
+    DartType invokeType;
+    if (element is PropertyAccessorElement) {
+      invokeType = element.returnType;
+    } else if (element is ExecutableElement) {
+      invokeType = element.type;
+    } else if (element is VariableElement) {
+      invokeType = _promoteManager.getStaticType(element);
+    }
+
+    return _resolveGenericMethod(
+        invokeType, node.typeArguments, node.methodName);
+  }
+
   /**
    * If the given [element] is a setter, return the getter associated with it.
    * Otherwise, return the element unchanged.
@@ -1464,49 +1491,6 @@ class ElementResolver extends SimpleAstVisitor<Object> {
     return staticType;
   }
 
-  DartType _computeMethodInvokeType(MethodInvocation node, Element element) {
-    if (element == null) {
-      return null;
-    }
-
-    DartType invokeType;
-    if (element is PropertyAccessorElement) {
-      invokeType = element.returnType;
-    } else if (element is ExecutableElement) {
-      invokeType = element.type;
-    } else if (element is VariableElement) {
-      invokeType = _promoteManager.getStaticType(element);
-    }
-
-    //
-    // Check for a generic method & apply type arguments if any were passed.
-    //
-    // TODO(jmesserly): support generic "call" methods on InterfaceType.
-    if (invokeType is FunctionType) {
-      FunctionType type = invokeType;
-      List<TypeParameterElement> parameters = type.typeFormals;
-
-      NodeList<TypeName> arguments = node.typeArguments?.arguments;
-      if (arguments != null && arguments.length != parameters.length) {
-        // Wrong number of type arguments. Ignore them
-        arguments = null;
-        _resolver.reportErrorForNode(
-            StaticTypeWarningCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS,
-            node.methodName,
-            [type, parameters.length, arguments?.length ?? 0]);
-      }
-      if (parameters.isNotEmpty) {
-        if (arguments == null) {
-          invokeType = _resolver.typeSystem.instantiateToBounds(type);
-        } else {
-          invokeType = type.instantiate(arguments.map((n) => n.type).toList());
-        }
-      }
-    }
-
-    return invokeType;
-  }
-
   /**
    * Return `true` if the given [expression] is a prefix for a deferred import.
    */
@@ -1751,6 +1735,20 @@ class ElementResolver extends SimpleAstVisitor<Object> {
   }
 
   /**
+   * Determines if the [propagatedType] of the invoke is better (more specific)
+   * than the [staticType]. If so it will be returned, otherwise returns null.
+   */
+  DartType _propagatedInvokeTypeIfBetter(

[Brian Wilkerson, 2016/01/08 01:02:21]

I think this is now being done in multiple places. It would be good to make it a
utility somewhere. (Perhaps a TODO and address it as part of another CL.)

[Jennifer Messerly, 2016/01/11 18:49:09]

Yeah, good idea, we can try and merge this logic with the other "propagated type
if better" logic. I'll add a TODO. (The reason I couldn't reuse the existing
code is it expects to set the Expression.propagatedType field. Likely with some
restructuring we could combine them but it didn't look totally straightforward,
so kept it out of this CL.)

+      DartType propagatedType, DartType staticType) {
+    if (propagatedType != null &&
+        (staticType == null || propagatedType.isMoreSpecificThan(staticType))) {
+      return propagatedType;
+    } else {
+      return null;
+    }
+  }
+
+  /**
    * Record that the given [node] is undefined, causing an error to be reported
    * if appropriate. The [declaringElement] is the element inside which no
    * declaration was found. If this element is a proxy, no error will be
@@ -2074,6 +2072,36 @@ class ElementResolver extends SimpleAstVisitor<Object> {
   }
 
   /**
+   * Check for a generic method & apply type arguments if any were passed.
+   */
+  DartType _resolveGenericMethod(

[Brian Wilkerson, 2016/01/08 01:02:21]

Ditto

[Jennifer Messerly, 2016/01/11 18:49:09]

Yeah, I was trying to make this method be the shared utility. This logic doesn't
happen anywhere else, as far as I know. Perhaps it needs a better home, but it's
not clear where would be better, as the error reporting logic ties it to the
Resolver family (ElementResolver/Resolver/StaticTypeAnalyzer) for now.

I'm open to ideas but it's not clear what action to take, at least not yet.

[Brian Wilkerson, 2016/01/11 18:56:41]

Agreed. A later CL is fine.

+      DartType invokeType, TypeArgumentList typeArguments, AstNode node) {
+    // TODO(jmesserly): support generic "call" methods on InterfaceType.
+    if (invokeType is FunctionType) {
+      FunctionType type = invokeType;
+      List<TypeParameterElement> parameters = type.typeFormals;
+
+      NodeList<TypeName> arguments = typeArguments?.arguments;
+      if (arguments != null && arguments.length != parameters.length) {
+        // Wrong number of type arguments. Ignore them
+        arguments = null;
+        _resolver.reportErrorForNode(
+            StaticTypeWarningCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS,
+            node,
+            [type, parameters.length, arguments?.length ?? 0]);
+      }
+      if (parameters.isNotEmpty) {
+        if (arguments == null) {
+          invokeType = _resolver.typeSystem.instantiateToBounds(type);
+        } else {
+          invokeType = type.instantiate(arguments.map((n) => n.type).toList());
+        }
+      }
+    }
+    return invokeType;
+  }
+
+  /**
    * Given an invocation of the form 'm(a1, ..., an)', resolve 'm' to the
    * element being invoked. If the returned element is a method, then the method
    * will be invoked. If the returned element is a getter, the getter will be