support generic methods in FunctionExpressionInvocation node

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

 // Copyright (c) 2014, 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.element_resolver;
 
 import 'dart:collection';
 
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
@@ skipping 408 matching lines @@
   }
 
   @override
   Object visitFunctionDeclaration(FunctionDeclaration node) {
     setMetadata(node.element, node);
     return null;
   }
 
   @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;
   }
 
   @override
   Object visitFunctionTypeAlias(FunctionTypeAlias node) {
     setMetadata(node.element, node);
     return null;
   }
 
@@ skipping 195 matching lines @@
     DartType propagatedInvokeType =
         _computeMethodInvokeType(node, propagatedElement);
 
     //
     // Record the results.
     //
     methodName.staticElement = staticElement;
     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) {
       List<ParameterElement> parameters =
           _computeCorrespondingParameters(argumentList, staticInvokeType);
       if (parameters != null) {
         argumentList.correspondingStaticParameters = parameters;
       }
     }
     if (propagatedInvokeType != null) {
@@ skipping 645 matching lines @@
       if (callMethod != null) {
         return _resolveArgumentsToFunction(false, argumentList, callMethod);
       }
     } else if (type is FunctionType) {
       return _resolveArgumentsToParameters(
           false, argumentList, type.parameters);
     }
     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.
    */
   Element _convertSetterToGetter(Element element) {
     // TODO(brianwilkerson) Determine whether and why the element could ever be
     // a setter.
     if (element is PropertyAccessorElement) {
       return element.variable.getter;
     }
@@ skipping 116 matching lines @@
     if (staticType is FunctionType) {
       //
       // All function types are subtypes of 'Function', which is itself a
       // subclass of 'Object'.
       //
       staticType = _resolver.typeProvider.functionType;
     }
     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.
    */
   bool _isDeferredPrefix(Expression expression) {
     if (expression is! SimpleIdentifier) {
       return false;
     }
     Element element = (expression as SimpleIdentifier).staticElement;
     if (element is! PrefixElement) {
       return false;
@@ skipping 224 matching lines @@
         // Internal error: Unmapped assignment operator.
         AnalysisEngine.instance.logger.logError(
             "Failed to map ${operator.lexeme} to it's corresponding operator");
         return operator;
       }
       break;
     }
   }
 
   /**
+   * 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
    * reported. If null, then an error will always be reported. The [errorCode]
    * is the error code to report. The [arguments] are the arguments to the error
    * message.
    */
   void _recordUndefinedNode(Element declaringElement, ErrorCode errorCode,
       AstNode node, List<Object> arguments) {
     if (_doesntHaveProxy(declaringElement)) {
@@ skipping 303 matching lines @@
     if (element == null) {
       element = classElement.getMethod(name);
     }
     if (element != null && element.isAccessibleIn(_definingLibrary)) {
       return element;
     }
     return null;
   }
 
   /**
+   * 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
    * invoked without arguments and the result of that invocation will then be
    * invoked with the arguments. The [methodName] is the name of the method
    * being invoked ('m').
    */
   Element _resolveInvokedElement(SimpleIdentifier methodName) {
     //
     // Look first in the lexical scope.
@@ skipping 483 matching lines @@
 
   @override
   Element get staticElement => null;
 
   @override
   accept(AstVisitor visitor) => null;
 
   @override
   void visitChildren(AstVisitor visitor) {}
 }