fixes #25477, downward inference of generic methods

pkg/analyzer/lib/dart/ast/ast.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.
 
 /**
  * Defines the AST model. The AST (Abstract Syntax Tree) model describes the
  * syntactic (as opposed to semantic) structure of Dart code. The semantic
  * structure of the code is modeled by the
  * [element model](../element/element.dart).
  *
@@ skipping 518 matching lines @@
   /**
    * Return the first token included in this node's source range.
    */
   Token get beginToken;
 
   /**
    * Return an iterator that can be used to iterate through all the entities
    * (either AST nodes or tokens) that make up the contents of this node,
    * including doc comments but excluding other comments.
    */
-  Iterable /*<AstNode | Token>*/ get childEntities;
+  Iterable/*<AstNode | Token>*/ get childEntities;
 
   /**
    * Return the offset of the character immediately following the last character
    * of this node's source range. This is equivalent to
    * `node.getOffset() + node.getLength()`. For a compilation unit this will be
    * equal to the length of the unit's source. For synthetic nodes this will be
    * equivalent to the node's offset (because the length is zero (0) by
    * definition).
    */
   int get end;
@@ skipping 34 matching lines @@
    * Return the node at the root of this node's AST structure. Note that this
    * method's performance is linear with respect to the depth of the node in the
    * AST structure (O(depth)).
    */
   AstNode get root;
 
   /**
    * Use the given [visitor] to visit this node. Return the value returned by
    * the visitor as a result of visiting this node.
    */
-  dynamic /* =E */ accept /*<E>*/ (AstVisitor /*<E>*/ visitor);
+  dynamic /* =E */ accept/*<E>*/(AstVisitor/*<E>*/ visitor);
 
   /**
    * Return the most immediate ancestor of this node for which the [predicate]
    * returns `true`, or `null` if there is no such ancestor. Note that this node
    * will never be returned.
    */
   AstNode getAncestor(Predicate<AstNode> predicate);
 
   /**
    * Return the value of the property with the given [name], or `null` if this
@@ skipping 3382 matching lines @@
  * The invocation of a function resulting from evaluating an expression.
  * Invocations of methods and other forms of functions are represented by
  * [MethodInvocation] nodes. Invocations of getters and setters are represented
  * by either [PrefixedIdentifier] or [PropertyAccess] nodes.
  *
  * > functionExpressionInvocation ::=
  * >     [Expression] [TypeArgumentList]? [ArgumentList]
  *
  * Clients may not extend, implement or mix-in this class.
  */
-abstract class FunctionExpressionInvocation extends Expression {
+abstract class FunctionExpressionInvocation extends InvocationExpression {
   /**
    * Initialize a newly created function expression invocation.
    */
   factory FunctionExpressionInvocation(
       Expression function,
       TypeArgumentList typeArguments,
       ArgumentList argumentList) = FunctionExpressionInvocationImpl;
 
   /**
    * Return the list of arguments to the method.
    */
   ArgumentList get argumentList;

[Brian Wilkerson, 2016/02/22 22:13:25]

We should remove the members that are now defined by InvocationExpression (both
here and in MethodInvocation).

[Jennifer Messerly, 2016/02/22 22:45:55]

Done.

 
   /**
    * Set the list of arguments to the method to the given [argumentList].
    */
   void set argumentList(ArgumentList argumentList);
 
   /**
    * Return the best element available for the function being invoked. If
    * resolution was able to find a better element based on type propagation,
    * that element will be returned. Otherwise, the element found using the
@@ skipping 951 matching lines @@
    */
   String get value;
 
   /**
    * Set the value of the literal to the given [value].
    */
   void set value(String value);
 }
 
 /**
+ * Common interface for [FunctionExpressionInvocation] and [MethodInvocation].

[Brian Wilkerson, 2016/02/22 22:13:25]

nit: "Common interface for" --> "The invocation of a function or method; either
a"

non-nit: Please include the disclaimer:

    Clients may not extend, implement or mix-in this class.

[Jennifer Messerly, 2016/02/22 22:45:55]

Done.

+ */
+abstract class InvocationExpression extends Expression {
+  /**
+   * The function type of the method invocation, or `null` if the AST

[Brian Wilkerson, 2016/02/22 22:13:25]

nit: "The function type of the method invocation" --> "Return the function type
of the invocation based on the static type information"

[Jennifer Messerly, 2016/02/22 22:45:55]

Done. but, FYI, this comment already existed, code simply moved.

+   * structure has not been resolved, or if the invoke could not be resolved.
+   *
+   * This will usually be a [FunctionType], but it can also be an
+   * [InterfaceType] with a `call` method, `dynamic`, `Function`, or a `@proxy`
+   * interface type that implements `Function`.
+   */
+  DartType staticInvokeType;

[Brian Wilkerson, 2016/02/22 22:13:25]

I have used only getter and setters in the public API (in part to discourage
extending one of these classes). As per below, let's think about whether we
actually need the setter to be defined on this class, or whether we can leave it
on the subclasses.

+
+  /**
+   * Like [staticInvokeType], but reflects propagated type information.

[Brian Wilkerson, 2016/02/22 22:13:25]

nit: I'd prefer the full documentation here because most clients don't make it
easy to follow links in hover text.

[Jennifer Messerly, 2016/02/22 22:45:55]

Done.

But in general: I very strongly prefer to avoid copy+paste, *especially* in
documentation. It leads to stale content, and reduces incentives to ever improve
docs, because you have to maintain O(copies) instead of O(1).

I don't know what the answer is, though. You're right that how difficult it is
depends on the client. My theory when I originally wrote this (this isn't new
code -- just moved) was that it was an okay tradeoff for maintainability. 

+   */
+  DartType propagatedInvokeType;
+
+  /**
+   * Return the list of arguments to the method.
+   */
+  ArgumentList get argumentList;
+
+  /**
+   * Set the list of arguments to the method to the given [argumentList].
+   */
+  void set argumentList(ArgumentList argumentList);

[Brian Wilkerson, 2016/02/22 22:13:25]

Do we need this setter to be defined here, or are the subclasses good enough?
Over time I would prefer to have the public API be read-only and to remove all
of the setters, but I can't do that today without introducing breaking changes.
However, given that this is a new class, omitting setters wouldn't be a change.

[Jennifer Messerly, 2016/02/22 22:45:55]

Okay, moved setters back down.

+
+  /**
+   * The target of the invocation node. For example:
+   *
+   *     (o.m)<TArgs>(args); // target will be `o.m`
+   *     o.m<TArgs>(args);   // target will be `m`
+   *
+   * In either case, the [invocationTarget.staticType] will be the
+   * [staticInvokeType] before applying type arguments `TArgs`. Similarly,
+   * [invocationTarget.propagatedType] will be the [propagatedInvokeType]
+   * before applying type arguments `TArgs`.
+   */
+  Expression get invocationTarget;

[Brian Wilkerson, 2016/02/22 22:13:25]

I was a bit confused when I first read the comment because I interpreted
`target` mean the same thing as MethodInvocation.target. Perhaps replace "The
target of the invocation node." with something like "The expression that
identifies the function or method being invoked."

The overloaded terminology is a bit unfortunate. Could we use "function" (from
FunctionInvocationExpression), or would that be weird?

We could probably drop "target will be" in the comments in the examples.

[Jennifer Messerly, 2016/02/22 22:45:55]

Nice, yeah "function" is a good idea. Done. (& updated comment as well per your
suggestions)

+
+  /**
+   * Return the type arguments to be applied to the method being invoked, or
+   * `null` if no type arguments were provided.
+   */
+  TypeArgumentList get typeArguments;
+
+  /**
+   * Set the type arguments to be applied to the method being invoked to the
+   * given [typeArguments].
+   */
+  void set typeArguments(TypeArgumentList typeArguments);

[Brian Wilkerson, 2016/02/22 22:13:25]

Do we need this?

[Jennifer Messerly, 2016/02/22 22:45:55]

Done, moved down to subclasses.

+}
+
+/**
  * An is expression.
  *
  * > isExpression ::=
  * >     [Expression] 'is' '!'? [TypeName]
  *
  * Clients may not extend, implement or mix-in this class.
  */
 abstract class IsExpression extends Expression {
   /**
    * Initialize a newly created is expression. The [notOperator] can be `null`
@@ skipping 521 matching lines @@
  * The invocation of either a function or a method. Invocations of functions
  * resulting from evaluating an expression are represented by
  * [FunctionExpressionInvocation] nodes. Invocations of getters and setters are
  * represented by either [PrefixedIdentifier] or [PropertyAccess] nodes.
  *
  * > methodInvocation ::=
  * >     ([Expression] '.')? [SimpleIdentifier] [TypeArgumentList]? [ArgumentList]
  *
  * Clients may not extend, implement or mix-in this class.
  */
-abstract class MethodInvocation extends Expression {
+abstract class MethodInvocation extends InvocationExpression {
   /**
    * Initialize a newly created method invocation. The [target] and [operator]
    * can be `null` if there is no target.
    */
   factory MethodInvocation(
       Expression target,
       Token operator,
       SimpleIdentifier methodName,
       TypeArgumentList typeArguments,
       ArgumentList argumentList) = MethodInvocationImpl;
@@ skipping 2503 matching lines @@
   /**
    * Return the 'yield' keyword.
    */
   Token get yieldKeyword;
 
   /**
    * Return the 'yield' keyword to the given [token].
    */
   void set yieldKeyword(Token token);
 }