Downwards inference. This adds support to the resolver for downwards

pkg/analyzer/lib/src/generated/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 engine.resolver;
 
 import 'dart:collection';
 
+import '../task/strong/info.dart' show InferredType, StaticInfo;
+import '../task/strong/rules.dart' show TypeRules;
 import 'ast.dart';
 import 'constant.dart';
 import 'element.dart';
 import 'element_resolver.dart';
 import 'engine.dart';
 import 'error.dart';
 import 'error_verifier.dart';
 import 'html.dart' as ht;
 import 'java_core.dart';
 import 'java_engine.dart';
@@ skipping 5728 matching lines @@
       LibraryElement libraryElement, ImportDirective importDirective) {
     List<ImportDirective> importList = _libraryMap[libraryElement];
     if (importList == null) {
       importList = new List<ImportDirective>();
       _libraryMap[libraryElement] = importList;
     }
     importList.add(importDirective);
   }
 }
 
+/** Maintains and manages contextual type information used for

[Brian Wilkerson, 2015/11/21 16:12:22]

Not asking for a change, but I'm not a fan of the abbreviated block style.

[Jennifer Messerly, 2015/11/23 22:11:57]

Yeah same here. I've been trying to follow:

/**
 * The doc comment.
 */

... for consistency.

At some point, I guess we should run the auto conversion tool to get /// ?
That would match the Dart style guide:
https://www.dartlang.org/effective-dart/documentation/#do-use--doc-comments-t...

[Leaf, 2015/12/01 21:49:11]

Done.

[Brian Wilkerson, 2015/12/01 22:31:09]

Personally, I find the "///" style hard to read, so I'd prefer that we didn't.
(I'm not the only one on the team.)

[Bob Nystrom, 2015/12/01 23:33:27]

I'll point out two things:

1. The style guide requires "///" and has for quite some time.
(https://www.dartlang.org/effective-dart/documentation/#do-use--doc-comments-t...)

2. The style here is not even valid JavaDoc style according to Google's Java
style guide. It should be one line or three+, but never two.

But, of course, your code is your code.

[Leaf, 2015/12/01 23:39:24]

Per point 2: that's my fault (caught in the review), and is resolved in a later
patch set.  I almost made a feature request to dart_style asking for the
formatter to fix these for me... :)

+ * inferring types.
+ */
+class InferenceContext {

[Jennifer Messerly, 2015/11/23 22:11:57]

"Context" as a class suffix often just means "here's a bag of global variables"
... it's not all that descriptive. I seem to recall Vijay at some point steering
me away from a "context" class in DDC.

That said, it's definitely hard to name these things. I feel like every compiler
eventually ends up with some "Context" types :). And here, we are using
"contextual type information" so it's more sensible name than usual. But a lot
of the state is the same bag of stuff we track frequently (Type
provider/system/rules & error listener...)

One possible solution is to make this object take a bit more of an active role,
then it can be named after what it does. But that'd require moving methods to
it. I haven't gotten far enough in the CL yet to know if that would make sense.

Anyway this is probably an FYI comment for now, but something to noodle on.

[Leaf, 2015/12/01 21:49:11]

Acknowledged.

+  /** The key by which we hang information off of ast nodes.
+   */
+  static const String _propertyName =

[Jennifer Messerly, 2015/11/23 22:11:57]

might be worth giving this a more descriptive name, like "_contextTypeProperty".

[Leaf, 2015/12/01 21:49:11]

Done.

+      'analyzer.src.generated.InferenceContext.contextType';
+
+  /** The error listener on which to record inference information.
+   */
+  final AnalysisErrorListener _errorListener;
+
+  /** Type provider, needed for type matching.
+   */
+  final TypeProvider _typeProvider;
+
+  /** The type system in use.
+   */
+  final TypeSystem _typeSystem;
+
+  /** The DDC type rules, used to create the inference info nodes.
+   */
+  final TypeRules _rules;

[Jennifer Messerly, 2015/11/23 22:11:57]

Question: are we still on track to remove this object? I feel like it should be
folded into StrongTypeSystemImpl.

[Leaf, 2015/11/24 19:32:12]

Yes.  It's here only to produce the info nodes that we put into the error
stream, since I didn't want to muck around with changing that right now.  

+
+  /** A stack of return types for all of the enclosing
+   * functions and methods.
+   */
+  List<DartType> _returnStack = <DartType>[];
+
+  InferenceContext(

[Jennifer Messerly, 2015/11/23 22:11:58]

BTW, if either the type or constructor can be made private, that'd be ideal.

[Leaf, 2015/11/24 19:32:11]

The type has to be public since it's accessed by the static type analyzer
(though I guess I could instead provide accessors on the resolver itself that
forward to the context).

What's the convention for private constructors in Dart?  

[Jennifer Messerly, 2015/11/24 20:10:57]

InferenceContext._(...)

[Leaf, 2015/12/01 21:49:11]

Done.

+      this._errorListener, TypeProvider typeProvider, this._typeSystem)
+      : this._typeProvider = typeProvider,

[Jennifer Messerly, 2015/11/23 22:11:57]

nit: "this." is not necessary here

[Leaf, 2015/12/01 21:49:10]

Done.

+        this._rules = new TypeRules(typeProvider);
+
+  /**  Get the return type of the current enclusing function, if any.

[Brian Wilkerson, 2015/11/21 16:12:22]

"enclusing" --> "enclosing"

[Leaf, 2015/12/01 21:49:11]

Done.

+   */
+  DartType get returnContext =>
+      (_returnStack.isNotEmpty) ? _returnStack.last : null;
+
+  /** If t1 = I<dynamic, ..., dynamic>, then look for a supertype

[Jennifer Messerly, 2015/11/23 22:11:57]

BTW: you probably want to put backticks around generic types, otherwise dartdoc
might not render them well. I know by default many markdowns won't do the right
thing. In other words:

If `t1 = I<dynamic, ..., dynamic>`, then look for a supertype ...


BTW, my CL might have the same problem, I'll double check...

[Leaf, 2015/12/01 21:49:11]

Done.

+   * of t1 of the form K<S0, ..., Sm> where t2 = K<S0', ..., Sm'>

[Brian Wilkerson, 2015/11/21 16:12:22]

What is 't2'?

[Jennifer Messerly, 2015/11/23 22:11:57]

looks like it's the second argument to matchTypes

[Leaf, 2015/12/01 21:49:11]

Added a line to the comments to make this clearer.

+   * If the supertype exists, use the constraints S0 <: S0', ... Sm <: Sm'
+   * to derive a concrete instantation for I of the form <T0, ..., Tn>,
+   * such that I<T0, .., Tn> <: t2
+   */
+  List<DartType> matchTypes(DartType t1, DartType t2) =>
+      (t1 is InterfaceType && t2 is InterfaceType) ? _matchTypes(t1, t2) : null;
+
+  /** Pop a return type off of the return stack.
+   */
+  void popReturnContext() {
+    if (_returnStack.isNotEmpty) {

[Brian Wilkerson, 2015/11/21 16:12:22]

Add "assert(_returnStack.isNotEmpty);" before the if for safety during testing.

[Leaf, 2015/12/01 21:49:11]

Done.

+      _returnStack.removeLast();
+    }
+  }
+
+  /**  Push a [returnType] onto the return stack.
+   */
+  void pushReturnContext(DartType returnType) {
+    _returnStack.add(returnType);
+  }
+
+  /** Place an info node into the error stream indicating that a
+   * [type] has been inferred as the type of [node].
+   */
+  void recordInference(Expression node, DartType type) {
+    StaticInfo info = InferredType.create(_rules, node, type);
+    if (info == null) return;

[Brian Wilkerson, 2015/11/21 16:12:22]

Stylistic nit (for here and elsewhere): we always use blocks for control
structures and always include type annotations.

[Leaf, 2015/11/24 19:32:12]

Sorry - pulled over from DDC code and I missed the style conversion.  Will fix. 
:)

[Leaf, 2015/12/01 21:49:11]

Done.

+    var error = info.toAnalysisError();
+    _errorListener.onError(error);
+  }
+
+  List<DartType> _matchTypes(InterfaceType t1, InterfaceType t2) {
+    if (t1 == t2) {
+      return t2.typeArguments;
+    }
+    List<DartType> tArgs1 = t1.typeArguments;
+    List<DartType> tArgs2 = t2.typeArguments;
+    // If t1 isn't a raw type, bail out
+    if (tArgs1 != null && tArgs1.any((t) => !t.isDynamic)) {
+      return null;
+    }
+
+    // This is our inferred type argument list.  We start at all dynamic,
+    // and fill in with inferred types when we reach a match.
+    List<DartType> actuals =
+        new List<DartType>.filled(tArgs1.length, _typeProvider.dynamicType);
+
+    // When we find the supertype of t1 with the same
+    // classname as t2 (see below), we have the following:
+    // If t1 is an instantiation of a class T1<X0, ..., Xn>
+    // and t2 is an instantiation of a class T2<Y0, ...., Ym>
+    // of the form t2 = T2<S0, ..., Sm>
+    // then we want to choose instantiations for the Xi
+    // T0, ..., Tn such that T1<T0, ..., Tn> <: t2 .
+    // To find this, we simply instantate T1 with
+    // X0, ..., Xn, and then find its superclass
+    // T2<T0', ..., Tn'>.  We then solve the constraint
+    // set T0' <: S0, ..., Tn' <: Sn for the Xi.
+    // Currently, we only handle constraints where
+    // the Ti' is one of the Xi'.  If there are multiple
+    // constraints on some Xi, we choose the lower of the
+    // two (if it exists).
+    bool permute(List<DartType> permutedArgs) {
+      if (permutedArgs == null) {
+        return false;
+      }
+      List<TypeParameterElement> ps = t1.typeParameters;
+      List<DartType> ts = ps.map((p) => p.type).toList();
+      for (int i = 0; i < permutedArgs.length; i++) {
+        DartType tVar = permutedArgs[i];
+        DartType tActual = tArgs2[i];
+        int index = ts.indexOf(tVar);
+        if (index >= 0 && _typeSystem.isSubtypeOf(tActual, actuals[index])) {
+          actuals[index] = tActual;
+        }
+      }
+      return actuals.any((x) => !x.isDynamic);
+    }
+
+    // Look for the first supertype of t1 with the same class name as t2.
+    bool match(InterfaceType t1) {

[Brian Wilkerson, 2015/11/21 16:12:22]

This method will result in an infinite loop if the type hierarchy is not well
formed.

[Leaf, 2015/12/01 21:49:11]

Done.

+      if (t1.element == t2.element) {
+        return permute(t1.typeArguments);
+      }
+
+      if (t1 == _typeProvider.objectType) {
+        return false;
+      }
+
+      if (match(t1.superclass)) {
+        return true;
+      }
+
+      for (final parent in t1.interfaces) {
+        if (match(parent)) {
+          return true;
+        }
+      }
+
+      for (final parent in t1.mixins) {

[Brian Wilkerson, 2015/11/21 16:12:22]

Interesting that we do mixins after interfaces. I would have expected the
opposite. Perhaps a comment explaining why would help.

[Jennifer Messerly, 2015/11/23 22:11:58]

Yeah. I would normally think of mixins as coming before superclass as well.
Mixins create a new superclass. Given

    class C extends S with M0, Mn {}

what you really have is something like:

    class S { ... S methods ... }
    class M0' extends S { ... M0 methods ... }
    class Mn' extends M0' { ... Mn methods ... }
    class C extends Mn' { ... C methods ... }

... note also that the order is reversed (the last mixin instantiation, here
called Mn', is actually our immediate superclass, followed by M0, followed by S)

[Leaf, 2015/11/24 19:32:11]

I need to sit down and work through the details of this algorithm at some point.
 This code is still only an approximation of what it will should probably
eventually be.  The only way the order should matter is if there are multiple
solutions. If there are multiple solutions, there's no guarantee that any
particular order will give the best solution - it just depends on how the code
is written.  So eventually I have to decide whether just to fix an order and
take the first, or find all solutions and pick the "best" (according to some
heuristic).  If looking at the mixins first makes sense to you guys, I'm happy
working with that.  I'll try to make up a couple of tests where the order
matters.

+        if (match(parent)) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+    // We have that t1 = T1<dynamic, ..., dynamic>.
+    // To match t1 against t2, we use the uninstantiated version
+    // of t1, essentially treating it as an instantiation with
+    // fresh variables, and solve for the variables.
+    // t1.element.type will be of the form T1<X0, ..., Xn>
+    if (!match(t1.element.type)) return null;
+    DartType newT1 = t1.element.type.substitute4(actuals);
+    // If we found a solution, return it.
+    if (_typeSystem.isSubtypeOf(newT1, t2)) return actuals;
+    return null;
+  }
+
+  /**  Attach contextual type information [type] to [node] for use during
+   * inference.
+   */
+  static void annotateNode(AstNode node, DartType type) {

[Jennifer Messerly, 2015/11/23 22:11:58]

(note: the following may be obsolete in light of my comment below)

maybe this should be called "setContextType" or "setType" if we keep "Context"
in the name? That way it would read like:

InferenceContext.setType(node, type);

I would also change "get" to something that corresponds. Either "getAnnotation"
or "getType" or "typeOf" ... something along those lines...

[Leaf, 2015/12/01 21:49:10]

Done.

+    node?.setProperty(_propertyName, type);

[Jennifer Messerly, 2015/11/23 22:11:57]

hmmmm. Thinking on this more (having seen the use cases) ... could we get by
with just a Map<AstNode, DartType>, stored inside our InferenceContext object?
That would be a lot better than a permanent property on the AST node. The
property here is essentially a field on AstNode... Ideally, if it's just
temporary state, we can throw it away (let it be GC'd) once we're done with
Resolver. Otherwise, if we need to keep the state around forever, maybe it
should be an actual field on AstNode...

[Leaf, 2015/11/24 19:32:12]

Yeah, I think this would be fine.  I initially didn't like the map approach, but
I think I've warmed to it.  I don't think the GC issue is significant, since any
types annotated on the AST likely also appear in the element model, and the
element model is more persistent than the AST, but nonetheless, it's not ideal.

Anyone else have thoughts on this?

[Brian Wilkerson, 2015/11/24 21:17:57]

I don't have a problem with either approach.

The only consideration I can think of is the typical one for side-car data
structures: you typically need to pass the side-car along with the original data
structure, which can lead to longer arguments lists.

[Leaf, 2015/12/01 21:49:11]

I'm leaving it as it is for now, but added a todo to reconsider. 

+    return;

[Jennifer Messerly, 2015/11/23 22:11:57]

nit: return not needed here

[Leaf, 2015/12/01 21:49:10]

The visitor methods seem to always explicitly return null below - is the
principle that implicit returns are ok for void functions but not for Object
ones (even if the return value is never used)?

+  }
+
+  /** Clear the type information assocated with [node].
+   */
+  static void clear(AstNode node) {
+    node?.setProperty(_propertyName, null);
+    return;
+  }
+
+  // If t1 = I<dynamic, ..., dynamic>, then look for a supertype

[Jennifer Messerly, 2015/11/23 22:11:57]

Is this supposed to be here? Consider folding it into the doc comment?
(sometimes "sort members" will lose positions of non-doc comments, so I wanted
to ask)

[Leaf, 2015/12/01 21:49:10]

Done.

+  // of t1 of the form K<S0, ..., Sm> where t2 = K<S0', ..., Sm'>
+  // If the supertype exists, use the constraints S0 <: S0', ... Sm <: Sm'
+  // to derive a concrete instantation for I of the form <T0, ..., Tn>,
+  // such that I<T0, .., Tn> <: t2
+  /**  Look for contextual type information attached to [node].  Returns
+   * the type if found, otherwise null.
+   */
+  static DartType get(AstNode node) => node?.getProperty(_propertyName);
+}
+
 /**
  * Instances of the class `InheritanceManager` manage the knowledge of where class members
  * (methods, getters & setters) are inherited from.
  */
 class InheritanceManager {
   /**
    * The [LibraryElement] that is managed by this manager.
    */
   LibraryElement _library;
 
@@ skipping 4598 matching lines @@
    * The object used to resolve the element associated with the current node.
    */
   ElementResolver elementResolver;
 
   /**
    * The object used to compute the type associated with the current node.
    */
   StaticTypeAnalyzer typeAnalyzer;
 
   /*
-  * The type system in use during resolution.
-  */
+   * The type system in use during resolution.
+   */
   TypeSystem typeSystem;
 
   /**
    * The class element representing the class containing the current node,
    * or `null` if the current node is not contained in a class.
    */
   ClassElement enclosingClass = null;
 
   /**
    * The class declaration representing the class containing the current node, or `null` if
@@ skipping 12 matching lines @@
    * current node is not contained in a function.
    */
   ExecutableElement _enclosingFunction = null;
 
   /**
    * The [Comment] before a [FunctionDeclaration] or a [MethodDeclaration] that
    * cannot be resolved where we visited it, because it should be resolved in the scope of the body.
    */
   Comment _commentBeforeFunction = null;
 
+  InferenceContext inferenceContext = null;

[Jennifer Messerly, 2015/11/23 22:11:58]

hmmm, I guess this is for consistency, but `= null` is not required
https://www.dartlang.org/effective-dart/usage/#dont-explicitly-initialize-var...

[Leaf, 2015/12/01 21:49:11]

Acknowledged.

+
   /**
    * The object keeping track of which elements have had their types overridden.
    */
   TypeOverrideManager _overrideManager = new TypeOverrideManager();
 
   /**
    * The object keeping track of which elements have had their types promoted.
    */
   TypePromotionManager _promoteManager = new TypePromotionManager();
 
@@ skipping 31 matching lines @@
       StaticTypeAnalyzerFactory typeAnalyzerFactory})
       : super(definingLibrary, source, typeProvider, errorListener,
             nameScope: nameScope) {
     if (inheritanceManager == null) {
       this._inheritanceManager = new InheritanceManager(definingLibrary);
     } else {
       this._inheritanceManager = inheritanceManager;
     }
     this.elementResolver = new ElementResolver(this);
     this.typeSystem = definingLibrary.context.typeSystem;
+    this.inferenceContext =
+        new InferenceContext(errorListener, typeProvider, typeSystem);
     if (typeAnalyzerFactory == null) {
       this.typeAnalyzer = new StaticTypeAnalyzer(this);
     } else {
       this.typeAnalyzer = typeAnalyzerFactory(this);
     }
   }
 
   /**
    * Initialize a newly created visitor to resolve the nodes in a compilation unit.
    *
@@ skipping 252 matching lines @@
     expression.propagatedType = type;
   }
 
   @override
   Object visitAnnotation(Annotation node) {
     AstNode parent = node.parent;
     if (identical(parent, _enclosingClassDeclaration) ||
         identical(parent, _enclosingFunctionTypeAlias)) {
       return null;
     }
-    return super.visitAnnotation(node);
+    safelyVisit(node.name);
+    safelyVisit(node.constructorName);
+    Element element = node.element;
+    if (element is ExecutableElement) {
+      InferenceContext.annotateNode(node.arguments, element.type);
+    }
+    safelyVisit(node.arguments);
+    node.accept(elementResolver);
+    node.accept(typeAnalyzer);
+    return null;
+  }
+
+  @override
+  Object visitArgumentList(ArgumentList node) {
+    DartType callerType = InferenceContext.get(node);
+    if (callerType is FunctionType) {
+      Map<String, DartType> namedParameterTypes =
+          callerType.namedParameterTypes;
+      List<DartType> normalParameterTypes = callerType.normalParameterTypes;
+      List<DartType> optionalParameterTypes = callerType.optionalParameterTypes;
+      int normalCount = normalParameterTypes.length;
+      int optionalCount = optionalParameterTypes.length;
+      int namedCount = namedParameterTypes.length;
+
+      NodeList<Expression> arguments = node.arguments;
+      Iterable<Expression> positional =
+          arguments.takeWhile((l) => l is! NamedExpression);
+      Iterable<Expression> required = positional.take(normalCount);
+      Iterable<Expression> optional =
+          positional.skip(normalCount).take(optionalCount);
+      Iterable<Expression> named =
+          arguments.skipWhile((l) => l is! NamedExpression);
+
+      int index = 0;

[Jennifer Messerly, 2015/11/23 22:11:57]

trivial: I'd probably just write this as a C-style for loop if iterating over
two lists:

for (int i = 0; i < normalCount; i++) {
  InferenceContext.annotateNode(positional[i], normalParameterTypes[i]);
}

you wouldn't need "required" either.

Same thing below.

(/me misses Enumerable.zip)

[Leaf, 2015/11/24 19:32:12]

The for loop version above will throw an out of bounds exception if the code is
ill-typed and normalCount is larger than the number of positional arguments.  I
could take the min of the lengths...not sure if that would be better...I'll look
at it. This code went through a few evolutions so I might be missing a simpler
formulation, but I was trying to be very careful about overruns in ill-typed
code.

[Jennifer Messerly, 2015/11/24 20:10:58]

Ah, in that case, probably best to leave it as is!

+      for (Expression argument in required) {
+        InferenceContext.annotateNode(argument, normalParameterTypes[index++]);

[Jennifer Messerly, 2015/11/23 22:11:58]

you might be able to avoid some of the above logic using
argument.correspondingParameterElement.

(aside: correspondingParameterIndex might be more useful. I ran into this as
well and ended up doing .indexOf(...) to find the FunctionType's parameter)

[Leaf, 2015/12/01 21:49:11]

Maybe?  I'd have to think through the various implications and ordering - not
sure all the right resolution and inference has happened at this point to set up
the parameter elements correctly.  Added a TODO.

+      }
+      index = 0;
+      for (Expression argument in optional) {
+        InferenceContext.annotateNode(
+            argument, optionalParameterTypes[index++]);
+      }
+
+      for (Expression argument in named) {
+        if (argument is NamedExpression &&
+            namedParameterTypes.containsKey(argument.name.label.name)) {

[Jennifer Messerly, 2015/11/23 22:11:57]

not that it matters, but I'd be tempted to rewrite this to only do the map
lookup once:

if (argument is NamedExpression) {
  DartType type = namedParameterTypes[argument.name.label.name];
  if (type != null) { ... }
}

[Leaf, 2015/12/01 21:49:11]

Done.

+          InferenceContext.annotateNode(
+              argument, namedParameterTypes[argument.name.label.name]);
+        }
+      }
+    }
+    super.visitArgumentList(node);

[Brian Wilkerson, 2015/11/21 16:12:22]

Missing return

[Leaf, 2015/12/01 21:49:10]

Done.

   }
 
   @override
   Object visitAsExpression(AsExpression node) {
+    InferenceContext.annotateNode(node.expression, node.type.type);
     super.visitAsExpression(node);
     // Since an as-statement doesn't actually change the type, we don't
     // let it affect the propagated type when it would result in a loss
     // of precision.
     overrideExpression(node.expression, node.type.type, false, false);
     return null;
   }
 
   @override
   Object visitAssertStatement(AssertStatement node) {
     super.visitAssertStatement(node);
     _propagateTrueState(node.condition);
     return null;
   }
 
   @override
+  Object visitAssignmentExpression(AssignmentExpression node) {
+    safelyVisit(node.leftHandSide);
+    sc.TokenType operator = node.operator.type;
+    if (operator == sc.TokenType.EQ ||
+        operator == sc.TokenType.QUESTION_QUESTION_EQ) {
+      InferenceContext.annotateNode(
+          node.rightHandSide, node.leftHandSide.staticType);
+    }
+    safelyVisit(node.rightHandSide);
+    node.accept(elementResolver);
+    node.accept(typeAnalyzer);
+    return null;
+  }
+
+  @override
+  Object visitAwaitExpression(AwaitExpression node) {
+    //TODO(leafp): Handle the implicit union type here
+    DartType contextType = InferenceContext.get(node);
+    if (contextType != null) {
+      InterfaceType futureT =
+          typeProvider.futureType.substitute4([contextType]);
+      InferenceContext.annotateNode(node.expression, futureT);
+    }
+    super.visitAwaitExpression(node);
+    return null;

[Brian Wilkerson, 2015/11/21 16:12:22]

Or "return super.visitAwaitExpression(node);"

[Leaf, 2015/12/01 21:49:10]

Done.

+  }
+
+  @override
   Object visitBinaryExpression(BinaryExpression node) {
     sc.TokenType operatorType = node.operator.type;
     Expression leftOperand = node.leftOperand;
     Expression rightOperand = node.rightOperand;
     if (operatorType == sc.TokenType.AMPERSAND_AMPERSAND) {
       safelyVisit(leftOperand);
       if (rightOperand != null) {
         _overrideManager.enterScope();
         try {
           _promoteManager.enterScope();
@@ skipping 19 matching lines @@
       if (rightOperand != null) {
         _overrideManager.enterScope();
         try {
           _propagateFalseState(leftOperand);
           rightOperand.accept(this);
         } finally {
           _overrideManager.exitScope();
         }
       }
     } else {
+      // TODO(leafp): Handle this case

[Brian Wilkerson, 2015/11/21 16:12:22]

I won't remember what this means outside the context of this CL; more detail
would be good. Perhaps: Use InferenceContext to propagate types downward.

[Leaf, 2015/12/01 21:49:11]

Done.

       safelyVisit(leftOperand);
       safelyVisit(rightOperand);
     }
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitBlockFunctionBody(BlockFunctionBody node) {
     safelyVisit(_commentBeforeFunction);
     _overrideManager.enterScope();
     try {
+      inferenceContext.pushReturnContext(InferenceContext.get(node));
       super.visitBlockFunctionBody(node);
     } finally {
       _overrideManager.exitScope();
+      inferenceContext.popReturnContext();
     }
     return null;
   }
 
   @override
   Object visitBreakStatement(BreakStatement node) {
     //
     // We do not visit the label because it needs to be visited in the context
     // of the statement.
     //
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
+  Object visitCascadeExpression(CascadeExpression node) {
+    InferenceContext.annotateNode(node.target, InferenceContext.get(node));
+    super.visitCascadeExpression(node);
+  }
+
+  @override
   Object visitClassDeclaration(ClassDeclaration node) {
     //
     // Resolve the metadata in the library scope.
     //
     if (node.metadata != null) {
       node.metadata.accept(this);
     }
     _enclosingClassDeclaration = node;
     //
     // Continue the class resolution.
@@ skipping 112 matching lines @@
       try {
         _promoteManager.enterScope();
         try {
           _propagateTrueState(condition);
           // Type promotion.
           _promoteTypes(condition);
           _clearTypePromotionsIfPotentiallyMutatedIn(thenExpression);
           _clearTypePromotionsIfAccessedInClosureAndProtentiallyMutated(
               thenExpression);
           // Visit "then" expression.
+          InferenceContext.annotateNode(

[Jennifer Messerly, 2015/11/23 22:11:57]

might be worth a helper for this?

    InferenceContext.setTypeFromNode(thenExpression, node);

[Leaf, 2015/12/01 21:49:11]

Done.

+              thenExpression, InferenceContext.get(node));
           thenExpression.accept(this);
         } finally {
           _promoteManager.exitScope();
         }
       } finally {
         _overrideManager.exitScope();
       }
     }
     Expression elseExpression = node.elseExpression;
     if (elseExpression != null) {
       _overrideManager.enterScope();
       try {
         _propagateFalseState(condition);
+        InferenceContext.annotateNode(
+            elseExpression, InferenceContext.get(node));
         elseExpression.accept(this);
       } finally {
         _overrideManager.exitScope();
       }
     }
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     bool thenIsAbrupt = _isAbruptTerminationExpression(thenExpression);
     bool elseIsAbrupt = _isAbruptTerminationExpression(elseExpression);
     if (elseIsAbrupt && !thenIsAbrupt) {
       _propagateTrueState(condition);
       _propagateState(thenExpression);
     } else if (thenIsAbrupt && !elseIsAbrupt) {
       _propagateFalseState(condition);
       _propagateState(elseExpression);
     }
     return null;
   }
 
   @override
   Object visitConstructorDeclaration(ConstructorDeclaration node) {
     ExecutableElement outerFunction = _enclosingFunction;
     try {
       _enclosingFunction = node.element;
+      DartType type = _enclosingFunction.type;
+      if (type is FunctionType) {

[Jennifer Messerly, 2015/11/23 22:11:58]

I think this would always be the case for a constructor

[Leaf, 2015/12/01 21:49:11]

Done.

+        _inferFormalParameterList(node.parameters, type);

[Jennifer Messerly, 2015/11/23 22:11:57]

was just chatting about this--I'm not sure this could ever kick in for
ConstructorDecl or MethodDecls, since the FunctionType is derived from the
formal parameter types. I *think* they should exactly match. If not I would be
curious when it kicks in.

(it would work for local functions though)

[Leaf, 2015/12/01 21:49:11]

Done.

+        InferenceContext.annotateNode(node.body, type.returnType);
+      }
       super.visitConstructorDeclaration(node);
     } finally {
       _enclosingFunction = outerFunction;
     }
     ConstructorElementImpl constructor = node.element;
     constructor.constantInitializers =
         new ConstantAstCloner().cloneNodeList(node.initializers);
     return null;
   }
 
   @override
   Object visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
     //
     // We visit the expression, but do not visit the field name because it needs
     // to be visited in the context of the constructor field initializer node.
     //
+    FieldElement fieldElement = enclosingClass.getField(node.fieldName.name);

[Jennifer Messerly, 2015/11/23 22:11:57]

Does `fieldName.staticElement` give you the field element? That would save a
look up.

[Leaf, 2015/11/24 19:32:12]

Not set until the elementResolver runs below. I could try re-ordering the call
to the elementResolver, but in general I'm trying to avoid doing that whenever
possible since often the order matters.

[Jennifer Messerly, 2015/11/24 20:10:57]

ah, interesting. In this case I think re-ordering would probably be okay, since
this line essentially inlines visitConstructorFieldInitializer (all it does is
look up the field element & save it).

[Leaf, 2015/12/01 21:49:11]

Yes, it's fine here and now, but I feel like every non-standard use of these
visitors is a tax on future maintenance.   Mostly the element resolver can
assume that sub-expressions have been resolved - this wouldn't be the case here
if I move it.  It happens not to matter now.... dunno.  I'm inclined to leave it
as is despite the redundancy, but could be convinced otherwise.

+    InferenceContext.annotateNode(node.expression, fieldElement?.type);
     safelyVisit(node.expression);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitConstructorName(ConstructorName node) {
     //
     // We do not visit either the type name, because it won't be visited anyway,
@@ skipping 11 matching lines @@
     // We do not visit the label because it needs to be visited in the context
     // of the statement.
     //
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitDefaultFormalParameter(DefaultFormalParameter node) {
+    InferenceContext.annotateNode(
+        node.defaultValue, node.parameter.element?.type);
     super.visitDefaultFormalParameter(node);
     ParameterElement element = node.element;
     if (element.initializer != null && node.defaultValue != null) {
       (element.initializer as FunctionElementImpl).returnType =
           node.defaultValue.staticType;
     }
     FormalParameterList parent = node.parent;
     AstNode grandparent = parent.parent;
     if (grandparent is ConstructorDeclaration &&
         grandparent.constKeyword != null) {
@@ skipping 58 matching lines @@
   }
 
   @override
   Object visitExpressionFunctionBody(ExpressionFunctionBody node) {
     safelyVisit(_commentBeforeFunction);
     if (resolveOnlyCommentInFunctionBody) {
       return null;
     }
     _overrideManager.enterScope();
     try {
+      InferenceContext.annotateNode(
+          node.expression, InferenceContext.get(node));
       super.visitExpressionFunctionBody(node);
     } finally {
       _overrideManager.exitScope();
     }
     return null;
   }
 
   @override
   Object visitFieldDeclaration(FieldDeclaration node) {
     _overrideManager.enterScope();
@@ skipping 19 matching lines @@
     return null;
   }
 
   @override
   void visitForEachStatementInScope(ForEachStatement node) {
     //
     // We visit the iterator before the loop variable because the loop variable
     // cannot be in scope while visiting the iterator.
     //
     Expression iterable = node.iterable;
-    safelyVisit(iterable);
     DeclaredIdentifier loopVariable = node.loopVariable;
     SimpleIdentifier identifier = node.identifier;
+    if (loopVariable?.type?.type != null) {
+      InterfaceType targetType = (node.awaitKeyword == null)
+          ? typeProvider.iterableType
+          : typeProvider.streamType;
+      InferenceContext.annotateNode(
+          iterable, targetType.substitute4([loopVariable.type.type]));
+    }
+    safelyVisit(iterable);
     safelyVisit(loopVariable);
     safelyVisit(identifier);
     Statement body = node.body;
     if (body != null) {
       _overrideManager.enterScope();
       try {
         if (loopVariable != null && iterable != null) {
           LocalVariableElement loopElement = loopVariable.element;
           if (loopElement != null) {
             DartType propagatedType = null;
@@ skipping 52 matching lines @@
     // TODO(brianwilkerson) If the loop can only be exited because the condition
     // is false, then propagateFalseState(condition);
   }
 
   @override
   Object visitFunctionDeclaration(FunctionDeclaration node) {
     ExecutableElement outerFunction = _enclosingFunction;
     try {
       SimpleIdentifier functionName = node.name;
       _enclosingFunction = functionName.staticElement as ExecutableElement;
+      InferenceContext.annotateNode(
+          node.functionExpression, _enclosingFunction.type);
       super.visitFunctionDeclaration(node);
     } finally {
       _enclosingFunction = outerFunction;
     }
     return null;
   }
 
   @override
   Object visitFunctionExpression(FunctionExpression node) {
     ExecutableElement outerFunction = _enclosingFunction;
     try {
       _enclosingFunction = node.element;
       _overrideManager.enterScope();
       try {
+        DartType functionType = InferenceContext.get(node);
+        if (functionType is FunctionType) {
+          _inferFormalParameterList(node.parameters, functionType);
+          InferenceContext.annotateNode(node.body, functionType.returnType);
+        }
         super.visitFunctionExpression(node);
       } finally {
         _overrideManager.exitScope();
       }
     } finally {
       _enclosingFunction = outerFunction;
     }
     return null;
   }
 
   @override
   Object visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
     safelyVisit(node.function);
     node.accept(elementResolver);
     _inferFunctionExpressionsParametersTypes(node.argumentList);
+    InferenceContext.annotateNode(node.argumentList, node.function.staticType);
     safelyVisit(node.argumentList);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitFunctionTypeAlias(FunctionTypeAlias node) {
     // Resolve the metadata in the library scope.
     if (node.metadata != null) {
       node.metadata.accept(this);
@@ skipping 67 matching lines @@
       List<Map<VariableElement, DartType>> perBranchOverrides =
           new List<Map<VariableElement, DartType>>();
       perBranchOverrides.add(thenOverrides);
       perBranchOverrides.add(elseOverrides);
       _overrideManager.mergeOverrides(perBranchOverrides);
     }
     return null;
   }
 
   @override
+  Object visitInstanceCreationExpression(InstanceCreationExpression node) {
+    DartType contextType = InferenceContext.get(node);
+    if (contextType is InterfaceType &&
+        contextType.typeArguments != null &&
+        contextType.typeArguments.length > 0) {
+      TypeName classTypeName = node.constructorName.type;
+      if (classTypeName.typeArguments == null) {
+        List<DartType> targs =
+            inferenceContext.matchTypes(classTypeName.type, contextType);
+        if (targs != null && targs.any((t) => !t.isDynamic)) {
+          ClassElement classElement = classTypeName.type.element;
+          InterfaceType rawType = classElement.type;
+          InterfaceType fullType =
+              rawType.substitute2(targs, rawType.typeArguments);
+          // The element resolver uses the type on the constructor name, so
+          // infer it first
+          typeAnalyzer.inferConstructorName(node.constructorName, fullType);
+          safelyVisit(node.constructorName);
+          ConstructorElement invokedConstructor =
+              node.constructorName.staticElement;
+          FunctionType rawConstructorType = invokedConstructor.type;
+          FunctionType constructorType = rawConstructorType.substitute2(
+              targs, rawConstructorType.typeArguments);
+          InferenceContext.annotateNode(node.argumentList, constructorType);
+          safelyVisit(node.argumentList);
+          InferenceContext.annotateNode(node, fullType);
+          node.accept(elementResolver);
+          node.accept(typeAnalyzer);
+          return null;
+        }
+      } else {
+        InferenceContext.clear(node);
+      }
+    }
+    super.visitInstanceCreationExpression(node);
+    return null;
+  }
+
+  @override
   Object visitLabel(Label node) => null;
 
   @override
   Object visitLibraryIdentifier(LibraryIdentifier node) => null;
 
   @override
+  Object visitListLiteral(ListLiteral node) {
+    DartType contextType = InferenceContext.get(node);
+    if (node.typeArguments == null && contextType is InterfaceType) {
+      InterfaceType listD =
+          typeProvider.listType.substitute4([typeProvider.dynamicType]);
+      List<DartType> targs = inferenceContext.matchTypes(listD, contextType);
+      if (targs != null &&
+          targs.length == 1 &&
+          targs.any((t) => !t.isDynamic)) {
+        DartType eType = targs[0];
+        InterfaceType listT = typeProvider.listType.substitute4([eType]);
+        for (Expression child in node.elements) {
+          InferenceContext.annotateNode(child, eType);
+        }
+        InferenceContext.annotateNode(node, listT);
+      } else {
+        InferenceContext.clear(node);
+      }
+    }
+    super.visitListLiteral(node);
+    return null;
+  }
+
+  @override
+  Object visitMapLiteral(MapLiteral node) {
+    DartType contextType = InferenceContext.get(node);
+    if (node.typeArguments == null && contextType is InterfaceType) {
+      InterfaceType mapD = typeProvider.mapType
+          .substitute4([typeProvider.dynamicType, typeProvider.dynamicType]);
+      List<DartType> targs = inferenceContext.matchTypes(mapD, contextType);
+      if (targs != null &&
+          targs.length == 2 &&
+          targs.any((t) => !t.isDynamic)) {
+        DartType kType = targs[0];
+        DartType vType = targs[1];
+        InterfaceType mapT = typeProvider.mapType.substitute4([kType, vType]);
+        for (MapLiteralEntry entry in node.entries) {
+          InferenceContext.annotateNode(entry.key, kType);
+          InferenceContext.annotateNode(entry.value, vType);
+        }
+        InferenceContext.annotateNode(node, mapT);
+      } else {
+        InferenceContext.clear(node);
+      }
+    }
+    super.visitMapLiteral(node);
+    return null;
+  }
+
+  @override
   Object visitMethodDeclaration(MethodDeclaration node) {
     ExecutableElement outerFunction = _enclosingFunction;
     try {
       _enclosingFunction = node.element;
+      _inferFormalParameterList(node.parameters, node.element.type);
+      InferenceContext.annotateNode(node.body, node.element.type?.returnType);
       super.visitMethodDeclaration(node);
     } finally {
       _enclosingFunction = outerFunction;
     }
     return null;
   }
 
   @override
   Object visitMethodInvocation(MethodInvocation node) {
     //
     // We visit the target and argument list, but do not visit the method name
     // because it needs to be visited in the context of the invocation.
     //
     safelyVisit(node.target);
     node.accept(elementResolver);
     _inferFunctionExpressionsParametersTypes(node.argumentList);
+    if (node.methodName.staticElement is ExecutableElement) {

[Jennifer Messerly, 2015/11/23 22:11:57]

suggestion, if you save this into a variable you can get type propagation:

Element method = node.methodName.staticElement;
if (method is ExecutableElement) {
  InferenceContext.annotateNode(node.argumentList, method.type);
}

[Leaf, 2015/12/01 21:49:10]

Done.

+      DartType type = (node.methodName.staticElement as ExecutableElement).type;
+      InferenceContext.annotateNode(node.argumentList, type);
+    }
     safelyVisit(node.argumentList);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
+  Object visitNamedExpression(NamedExpression node) {
+    InferenceContext.annotateNode(node.expression, InferenceContext.get(node));
+    super.visitNamedExpression(node);

[Jennifer Messerly, 2015/11/23 22:11:57]

not sure if Brian got tired of pointing it out ... but, there's a few more
missing returns here & below.

[Leaf, 2015/12/01 21:49:10]

Done.

+  }
+
+  @override
   Object visitNode(AstNode node) {
     node.visitChildren(this);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
+  Object visitParenthesizedExpression(ParenthesizedExpression node) {
+    InferenceContext.annotateNode(node.expression, InferenceContext.get(node));
+    super.visitParenthesizedExpression(node);
+  }
+
+  @override
   Object visitPrefixedIdentifier(PrefixedIdentifier node) {
     //
     // We visit the prefix, but do not visit the identifier because it needs to
     // be visited in the context of the prefix.
     //
     safelyVisit(node.prefix);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
@@ skipping 11 matching lines @@
   }
 
   @override
   Object visitRedirectingConstructorInvocation(
       RedirectingConstructorInvocation node) {
     //
     // We visit the argument list, but do not visit the optional identifier
     // because it needs to be visited in the context of the constructor
     // invocation.
     //
+    InferenceContext.annotateNode(node.argumentList, node.staticElement?.type);
     safelyVisit(node.argumentList);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
+  Object visitReturnStatement(ReturnStatement node) {
+    InferenceContext.annotateNode(
+        node.expression, inferenceContext.returnContext);
+    super.visitReturnStatement(node);
+  }
+
+  @override
   Object visitShowCombinator(ShowCombinator node) => null;
 
   @override
   Object visitSuperConstructorInvocation(SuperConstructorInvocation node) {
     //
     // We visit the argument list, but do not visit the optional identifier
     // because it needs to be visited in the context of the constructor
     // invocation.
     //
+    InferenceContext.annotateNode(node.argumentList, node.staticElement?.type);
     safelyVisit(node.argumentList);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitSwitchCase(SwitchCase node) {
     _overrideManager.enterScope();
     try {
@@ skipping 27 matching lines @@
       _overrideManager.applyOverrides(overrides);
     }
     return null;
   }
 
   @override
   Object visitTypeName(TypeName node) => null;
 
   @override
   Object visitVariableDeclaration(VariableDeclaration node) {
+    InferenceContext.annotateNode(node.initializer, InferenceContext.get(node));
     super.visitVariableDeclaration(node);
     VariableElement element = node.element;
     if (element.initializer != null && node.initializer != null) {
       (element.initializer as FunctionElementImpl).returnType =
           node.initializer.staticType;
     }
     // Note: in addition to cloning the initializers for const variables, we
     // have to clone the initializers for non-static final fields (because if
     // they occur in a class with a const constructor, they will be needed to
     // evaluate the const constructor).
     if ((element.isConst ||
             (element is FieldElement &&
                 element.isFinal &&
                 !element.isStatic)) &&
         node.initializer != null) {
       (element as ConstVariableElement).constantInitializer =
           new ConstantAstCloner().cloneNode(node.initializer);
     }
     return null;
   }
 
+  @override visitVariableDeclarationList(VariableDeclarationList node) {
+    for (VariableDeclaration decl in node.variables) {
+      InferenceContext.annotateNode(decl, node.type?.type);
+    }
+    super.visitVariableDeclarationList(node);
+  }
+
   @override
   Object visitWhileStatement(WhileStatement node) {
     // Note: since we don't call the base class, we have to maintain
     // _implicitLabelScope ourselves.
     ImplicitLabelScope outerImplicitScope = _implicitLabelScope;
     try {
       _implicitLabelScope = _implicitLabelScope.nest(node);
       Expression condition = node.condition;
       safelyVisit(condition);
       Statement body = node.body;
       if (body != null) {
         _overrideManager.enterScope();
         try {
           _propagateTrueState(condition);
           visitStatementInScope(body);
         } finally {
           _overrideManager.exitScope();
         }
       }
     } finally {
       _implicitLabelScope = outerImplicitScope;
     }
     // TODO(brianwilkerson) If the loop can only be exited because the condition
     // is false, then propagateFalseState(condition);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
+  @override
+  Object visitYieldStatement(YieldStatement node) {
+    DartType returnType = inferenceContext.returnContext;
+    if (returnType != null && _enclosingFunction != null) {
+      // If we're not in a generator ([a]sync*, then we shouldn't have a yield.
+      // so don't infer
+      if (_enclosingFunction.isGenerator) {
+        // If this is a yield*, then we just propagate the return type downwards
+        DartType type = returnType;
+        // If this just a yield, then we need to get the element type
+        if (node.star == null) {
+          // If it's synchronous, we expect Iterable<T>, otherwise Stream<T>
+          InterfaceType wrapperD = (_enclosingFunction.isSynchronous)

[Jennifer Messerly, 2015/11/23 22:11:57]

nit: parens not needed 

[Leaf, 2015/12/01 21:49:11]

Done.

+              ? typeProvider.iterableDynamicType
+              : typeProvider.streamDynamicType;
+          // Match the types to instantiate the type arguments if possible
+          List<DartType> targs =
+              inferenceContext.matchTypes(wrapperD, returnType);
+          type = (targs?.length == 1) ? targs[0] : null;
+        }
+        InferenceContext.annotateNode(node.expression, type);
+      }
+    }
+    super.visitYieldStatement(node);

[Brian Wilkerson, 2015/11/21 16:12:22]

Missing return

[Leaf, 2015/11/24 19:32:12]

Just a meta-comment: this is one of those analyzer hints that I'd love to make
use of, but I can't actually make use of analyzer hints when working on the
analyzer because of the deluge of hints about deprecated APIs.  Is there any way
to suppress hints, or separate them by kind in the IDE, or somehow to address
this?  Seems like this would be a nice win for users who want to use things like
@deprecated.  As it is though, I always end up running the analyzer with
--no-hints, which is a shame.

[Brian Wilkerson, 2015/11/24 21:17:57]

That depends on the IDE. I don't know of any way to do that in IntelliJ, but
that might be planned work. I know there has also been some discussion about
being able to disable hints in the .analysis_options file, but I don't think
that work is completed.
Those should go away soon as part of our breaking change bundle.

[Leaf, 2015/12/01 21:49:10]

Done.

+  }
+
   /**
    * Checks each promoted variable in the current scope for compliance with the following
    * specification statement:
    *
    * If the variable <i>v</i> is accessed by a closure in <i>s<sub>1</sub></i> then the variable
    * <i>v</i> is not potentially mutated anywhere in the scope of <i>v</i>.
    */
   void _clearTypePromotionsIfAccessedInClosureAndProtentiallyMutated(
       AstNode target) {
     for (Element element in _promoteManager.promotedElements) {
@@ skipping 74 matching lines @@
         List<ParameterElement> onDataParameters = onDataType.parameters;
         if (onDataParameters == null || onDataParameters.isEmpty) {
           return null;
         }
         return onDataParameters[0].type;
       }
     }
     return null;
   }
 
+  void _inferFormalParameterList(FormalParameterList node, DartType type) {
+    if (typeAnalyzer.inferFormalParameterList(node, type)) {
+      // TODO(leafp): This gets dropped on the floor if we're in the field
+      // inference task.  We should probably keep these infos.
+      inferenceContext.recordInference(node.parent, type);
+    }
+  }
+
   /**
    * If given "mayBeClosure" is [FunctionExpression] without explicit parameters types and its
    * required type is [FunctionType], then infer parameters types from [FunctionType].
    */
   void _inferFunctionExpressionParametersTypes(
       Expression mayBeClosure, DartType mayByFunctionType) {
     // prepare closure
     if (mayBeClosure is! FunctionExpression) {
       return;
     }
@@ skipping 3913 matching lines @@
     nonFields.add(node);
     return null;
   }
 
   @override
   Object visitNode(AstNode node) => node.accept(TypeResolverVisitor_this);
 
   @override
   Object visitWithClause(WithClause node) => null;
 }