clean up generic methods in resolution

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 9f0cda01242452fad472655826b09be07e19b09d..400db3640f37ef72eda3825f9bb161b936f73aa6 100644
--- a/pkg/analyzer/lib/src/generated/element_resolver.dart
+++ b/pkg/analyzer/lib/src/generated/element_resolver.dart
@@ -9,14 +9,12 @@ import 'dart:collection';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/element.dart';
-import 'package:analyzer/src/dart/element/member.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/error.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/scanner.dart' as sc;
-import 'package:analyzer/src/generated/utilities_dart.dart';
 
 /**
  * An object used by instances of [ResolverVisitor] to resolve references within
@@ -639,64 +637,34 @@ class ElementResolver extends SimpleAstVisitor<Object> {
         }
       }
     }
-    //
-    // Check for a generic method & apply type arguments if any were passed.
-    //
-    if (staticElement is MethodElement || staticElement is FunctionElement) {
-      FunctionType type = (staticElement as ExecutableElement).type;
-      List<TypeParameterElement> parameters = type.boundTypeParameters;
-
-      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,
-            methodName,
-            [type, parameters.length, arguments.length]);
-      }
-      if (parameters.isNotEmpty) {
-        List<DartType> typeArgs;
-        if (arguments == null) {
-          typeArgs = new List<DartType>.filled(
-              parameters.length, DynamicTypeImpl.instance);
-        } else {
-          typeArgs = new List<DartType>.from(arguments.map((n) => n.type));
-        }
-        type = type.instantiate(typeArgs);
-
-        if (staticElement is MethodMember) {
-          MethodMember member = staticElement;
-          staticElement =
-              new MethodMember(member.baseElement, member.definingType, type);
-        } else if (staticElement is MethodElement) {
-          ClassElement clazz = staticElement.enclosingElement;
-          staticElement = new MethodMember(staticElement, clazz.type, type);
-        } else {
-          staticElement =
-              new FunctionMember(staticElement as FunctionElement, type);
-        }
-      }
-    }
 
     staticElement = _convertSetterToGetter(staticElement);
     propagatedElement = _convertSetterToGetter(propagatedElement);
+
+    DartType staticInvokeType = _computeMethodInvokeType(node, staticElement);
+    DartType propagatedInvokeType =

[Brian Wilkerson, 2016/01/06 21:57:34]

If the propagatedInvokeType is not `null` and is not more specific than the
staticInvokeType, then we should set it to `null`.

[Jennifer Messerly, 2016/01/06 22:25:51]

Yeah that's a good point. I think we get this for free because only one of
"staticElement" or "propagatedElement" should be non-null, and that's how the
old code was able to ensure only one of static/propagated parameter types were
computed.

But I'm not sure if that's always the case.

[Brian Wilkerson, 2016/01/06 22:47:01]

Unfortunately, no. It's perfectly valid for both to be non-null (which means the
old code was probably wrong). For example, given

class A {
  int m() {}
}
class B extends A {
  int m() {}
}
f() {
  A a = new B();
  a.m();
}

The static type of 'a' will be 'A' but the propagated type will be 'B', so in
the invocation of 'm' we will have both a static element (A.m) and a propagated
element (B.m). But because the return type will be the same either way, we want
to not record the propagated type (in order to reduce the amount of follow-on
work we do).

[Jennifer Messerly, 2016/01/06 22:52:48]

Ah that makes sense. Does something like this look right:

if (propagatedInvokeType != null &&
    !propagatedInvokeType.isMoreSpecificThan(staticInvokeType)) {

  // If the propagated type is no better than the static type, discard it.
  propagatedInvokeType = null;
}

[Brian Wilkerson, 2016/01/07 00:24:22]

See ResolverVisitor.recordPropagatedTypeIfBetter.

[Jennifer Messerly, 2016/01/07 00:33:29]

Ah, nice! Looking through there, I think the logic I added is almost the same,
but it should explicitly "null" out the type if it is not needed. I'll add that
& then I think it's good.

+        _computeMethodInvokeType(node, propagatedElement);
+
     //
     // Record the results.
     //
     methodName.staticElement = staticElement;
     methodName.propagatedElement = propagatedElement;
+
+    node.staticInvokeType = staticInvokeType;
+    node.propagatedInvokeType = propagatedInvokeType;
+
     ArgumentList argumentList = node.argumentList;
-    if (staticElement != null) {
+    if (staticInvokeType != null) {
       List<ParameterElement> parameters =
-          _computeCorrespondingParameters(argumentList, staticElement);
+          _computeCorrespondingParameters(argumentList, staticInvokeType);
       if (parameters != null) {
         argumentList.correspondingStaticParameters = parameters;
       }
     }
-    if (propagatedElement != null) {
+    if (propagatedInvokeType != null) {
       List<ParameterElement> parameters =
-          _computeCorrespondingParameters(argumentList, propagatedElement);
+          _computeCorrespondingParameters(argumentList, propagatedInvokeType);
       if (parameters != null) {
         argumentList.correspondingPropagatedParameters = parameters;
       }
@@ -1336,44 +1304,16 @@ class ElementResolver extends SimpleAstVisitor<Object> {
    * arguments, or `null` if no correspondence could be computed.
    */
   List<ParameterElement> _computeCorrespondingParameters(
-      ArgumentList argumentList, Element element) {
-    if (element is PropertyAccessorElement) {
-      //
-      // This is an invocation of the call method defined on the value returned
-      // by the getter.
-      //
-      FunctionType getterType = element.type;
-      if (getterType != null) {
-        DartType getterReturnType = getterType.returnType;
-        if (getterReturnType is InterfaceType) {
-          MethodElement callMethod = getterReturnType.lookUpMethod(
-              FunctionElement.CALL_METHOD_NAME, _definingLibrary);
-          if (callMethod != null) {
-            return _resolveArgumentsToFunction(false, argumentList, callMethod);
-          }
-        } else if (getterReturnType is FunctionType) {
-          List<ParameterElement> parameters = getterReturnType.parameters;
-          return _resolveArgumentsToParameters(false, argumentList, parameters);
-        }
-      }
-    } else if (element is ExecutableElement) {
-      return _resolveArgumentsToFunction(false, argumentList, element);
-    } else if (element is VariableElement) {
-      VariableElement variable = element;
-      DartType type = _promoteManager.getStaticType(variable);
-      if (type is FunctionType) {
-        FunctionType functionType = type;
-        List<ParameterElement> parameters = functionType.parameters;
-        return _resolveArgumentsToParameters(false, argumentList, parameters);
-      } else if (type is InterfaceType) {
-        // "call" invocation
-        MethodElement callMethod = type.lookUpMethod(
-            FunctionElement.CALL_METHOD_NAME, _definingLibrary);
-        if (callMethod != null) {
-          List<ParameterElement> parameters = callMethod.parameters;
-          return _resolveArgumentsToParameters(false, argumentList, parameters);
-        }
+      ArgumentList argumentList, DartType type) {
+    if (type is InterfaceType) {
+      MethodElement callMethod =
+          type.lookUpMethod(FunctionElement.CALL_METHOD_NAME, _definingLibrary);
+      if (callMethod != null) {
+        return _resolveArgumentsToFunction(false, argumentList, callMethod);
       }
+    } else if (type is FunctionType) {
+      return _resolveArgumentsToParameters(
+          false, argumentList, type.parameters);
     }
     return null;
   }
@@ -1514,6 +1454,50 @@ 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): should we support for generic "call" methods on

[Brian Wilkerson, 2016/01/06 21:57:34]

"should we support for"?

[Jennifer Messerly, 2016/01/06 22:25:51]

DOH. Good catch! It got stuck halfway between an edit from "should we have
support for" and "should we support". Sometimes I think my comments would be so
much better if I didn't try edit them.... always end up losing words in the
process. :)

+    // 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]);
+      }
+      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.
    */
@@ -1973,77 +1957,9 @@ class ElementResolver extends SimpleAstVisitor<Object> {
    */
   List<ParameterElement> _resolveArgumentsToParameters(bool reportError,
       ArgumentList argumentList, List<ParameterElement> parameters) {
-    List<ParameterElement> requiredParameters = new List<ParameterElement>();
-    List<ParameterElement> positionalParameters = new List<ParameterElement>();
-    HashMap<String, ParameterElement> namedParameters =
-        new HashMap<String, ParameterElement>();
-    for (ParameterElement parameter in parameters) {
-      ParameterKind kind = parameter.parameterKind;
-      if (kind == ParameterKind.REQUIRED) {
-        requiredParameters.add(parameter);
-      } else if (kind == ParameterKind.POSITIONAL) {
-        positionalParameters.add(parameter);
-      } else {
-        namedParameters[parameter.name] = parameter;
-      }
-    }
-    List<ParameterElement> unnamedParameters =
-        new List<ParameterElement>.from(requiredParameters);
-    unnamedParameters.addAll(positionalParameters);
-    int unnamedParameterCount = unnamedParameters.length;
-    int unnamedIndex = 0;
-    NodeList<Expression> arguments = argumentList.arguments;
-    int argumentCount = arguments.length;
-    List<ParameterElement> resolvedParameters =
-        new List<ParameterElement>(argumentCount);
-    int positionalArgumentCount = 0;
-    HashSet<String> usedNames = new HashSet<String>();
-    bool noBlankArguments = true;
-    for (int i = 0; i < argumentCount; i++) {
-      Expression argument = arguments[i];
-      if (argument is NamedExpression) {
-        SimpleIdentifier nameNode = argument.name.label;
-        String name = nameNode.name;
-        ParameterElement element = namedParameters[name];
-        if (element == null) {
-          ErrorCode errorCode = (reportError
-              ? CompileTimeErrorCode.UNDEFINED_NAMED_PARAMETER
-              : StaticWarningCode.UNDEFINED_NAMED_PARAMETER);
-          _resolver.reportErrorForNode(errorCode, nameNode, [name]);
-        } else {
-          resolvedParameters[i] = element;
-          nameNode.staticElement = element;
-        }
-        if (!usedNames.add(name)) {
-          _resolver.reportErrorForNode(
-              CompileTimeErrorCode.DUPLICATE_NAMED_ARGUMENT, nameNode, [name]);
-        }
-      } else {
-        if (argument is SimpleIdentifier && argument.name.isEmpty) {
-          noBlankArguments = false;
-        }
-        positionalArgumentCount++;
-        if (unnamedIndex < unnamedParameterCount) {
-          resolvedParameters[i] = unnamedParameters[unnamedIndex++];
-        }
-      }
-    }
-    if (positionalArgumentCount < requiredParameters.length &&
-        noBlankArguments) {
-      ErrorCode errorCode = (reportError
-          ? CompileTimeErrorCode.NOT_ENOUGH_REQUIRED_ARGUMENTS
-          : StaticWarningCode.NOT_ENOUGH_REQUIRED_ARGUMENTS);
-      _resolver.reportErrorForNode(errorCode, argumentList,
-          [requiredParameters.length, positionalArgumentCount]);
-    } else if (positionalArgumentCount > unnamedParameterCount &&
-        noBlankArguments) {
-      ErrorCode errorCode = (reportError
-          ? CompileTimeErrorCode.EXTRA_POSITIONAL_ARGUMENTS
-          : StaticWarningCode.EXTRA_POSITIONAL_ARGUMENTS);
-      _resolver.reportErrorForNode(errorCode, argumentList,
-          [unnamedParameterCount, positionalArgumentCount]);
-    }
-    return resolvedParameters;
+    return ResolverVisitor.resolveArgumentsToParameters(
+        argumentList, parameters, _resolver.reportErrorForNode,
+        reportError: reportError);
   }
 
   void _resolveBinaryExpression(BinaryExpression node, String methodName) {