clean up generic methods in resolution

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';
 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
  * the AST structure to the elements being referenced. The requirements for the
  * element resolver are:
  *
  * 1. Every [SimpleIdentifier] should be resolved to the element to which it
  *    refers. Specifically:
  *    * An identifier within the declaration of that name should resolve to the
  *      element being declared.
@@ skipping 602 matching lines @@
             target, staticType, methodName, isConditional);
         // If we have propagated type information use it (since it should
         // not be redundant with the staticType).  Otherwise, don't produce
         // a propagatedElement which duplicates the staticElement.
         if (propagatedType is InterfaceType) {
           propagatedElement = _resolveInvokedElementWithTarget(
               target, propagatedType, methodName, isConditional);
         }
       }
     }
-    //
-    // 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;
       }
     }
     //
     // Then check for error conditions.
     //
     ErrorCode errorCode = _checkForInvocationError(target, true, staticElement);
     bool generatedWithTypePropagation = false;
     if (_enableHints && errorCode == null && staticElement == null) {
@@ skipping 619 matching lines @@
     return false;
   }
 
   /**
    * Given an [argumentList] and the executable [element] that  will be invoked
    * using those arguments, compute the list of parameters that correspond to
    * the list of arguments. Return the parameters that correspond to the
    * 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);
-        }
+      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 (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);
-        }
-      }
+    } else if (type is FunctionType) {
+      return _resolveArgumentsToParameters(
+          false, argumentList, type.parameters);
     }
     return null;
   }
 
   /**
    * 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
@@ skipping 120 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): 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.
    */
   bool _isDeferredPrefix(Expression expression) {
     if (expression is! SimpleIdentifier) {
       return false;
     }
     Element element = (expression as SimpleIdentifier).staticElement;
     if (element is! PrefixElement) {
       return false;
@@ skipping 439 matching lines @@
    * Given an [argumentList] and the [parameters] related to the element that
    * will be invoked using those arguments, compute the list of parameters that
    * correspond to the list of arguments. An error will be reported if any of
    * the arguments cannot be matched to a parameter. The flag [reportError]
    * should be `true` if a compile-time error should be reported; or `false` if
    * a compile-time warning should be reported. Return the parameters that
    * correspond to the arguments.
    */
   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) {
     Expression leftOperand = node.leftOperand;
     if (leftOperand != null) {
       DartType staticType = _getStaticType(leftOperand);
       MethodElement staticMethod =
           _lookUpMethod(leftOperand, staticType, methodName);
       node.staticElement = staticMethod;
       DartType propagatedType = _getPropagatedType(leftOperand);
@@ skipping 587 matching lines @@
 
   @override
   Element get staticElement => null;
 
   @override
   accept(AstVisitor visitor) => null;
 
   @override
   void visitChildren(AstVisitor visitor) {}
 }