Type check for-in statements.

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

Index: pkg/analyzer/lib/src/generated/error_verifier.dart
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index 70aa151cd7bd61ecdba7c55b6ccd368b9666745c..7d0c52be00f21130eeef324d522e9d8db516a6a7 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -689,6 +689,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   }
 
   @override
+  Object visitForEachStatement(ForEachStatement node) {
+    _checkForInIterable(node);
+    return super.visitForEachStatement(node);
+  }
+
+  @override
   Object visitFunctionDeclaration(FunctionDeclaration node) {
     ExecutableElement outerFunction = _enclosingFunction;
     try {
@@ -5663,16 +5669,65 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   }
 
   /**
+   * Check for a type mis-match between the iterable expression and the
+   * assigned variable in a for-in statement.
+   */
+  void _checkForInIterable(ForEachStatement node) {
+    // Ignore malformed for statements.
+    if (node.identifier == null && node.loopVariable == null) {
+      return;
+    }
+
+    DartType iterableType = getStaticType(node.iterable);
+    if (iterableType.isDynamic) {
+      return;
+    }
+
+    // The type of the loop variable.
+    SimpleIdentifier variable = node.identifier != null

[Jennifer Messerly, 2016/03/08 19:14:11]

probably not worth it, but this could be:

    node?.identifier ?? node.loopVariable.identifier

(aside: I've always found the AST structure of ForEach a bit confusing...)

+        ? node.identifier
+        : node.loopVariable.identifier;
+    DartType variableType = getStaticType(variable);
+
+    DartType loopType = node.awaitKeyword != null
+        ? _typeProvider.streamType
+        : _typeProvider.iterableType;
+
+    // Use an explicit string instead of [loopType] to remove the "<E>".
+    String loopTypeName = node.awaitKeyword != null

[Jennifer Messerly, 2016/03/08 19:14:11]

do you need this? Isn't this just: `loopType.name` ?

+        ? "Stream"
+        : "Iterable";
+
+    // The object being iterated has to implement Iterable<T> for some T that
+    // is assignable to the variable's type.
+    // TODO(rnystrom): Move this into mostSpecificTypeArgument()?

[Jennifer Messerly, 2016/03/08 19:14:11]

yeah, I think that makes sense.

+    iterableType = iterableType.resolveToBound(_typeProvider.objectType);
+    DartType bestIterableType = _typeSystem.mostSpecificTypeArgument(
+        iterableType, loopType);
+    if (bestIterableType == null) {
+      _errorReporter.reportTypeErrorForNode(
+          StaticTypeWarningCode.FOR_IN_OF_INVALID_TYPE,
+          node,
+          [iterableType, loopTypeName]);
+    } else if (!_typeSystem.isAssignableTo(bestIterableType, variableType)) {
+      _errorReporter.reportTypeErrorForNode(
+          StaticTypeWarningCode.FOR_IN_OF_INVALID_ELEMENT_TYPE,
+          node,
+          [iterableType, loopTypeName, variableType]);
+    }
+  }
+
+  /**
    * Check for a type mis-match between the yielded type and the declared
    * return type of a generator function.
    *
    * This method should only be called in generator functions.
    */
-  bool _checkForYieldOfInvalidType(
+  void _checkForYieldOfInvalidType(

[Brian Wilkerson, 2016/03/08 01:12:12]

fyi: I don't know that it matters, but the original pattern for the helper
methods in this class was to return `true` if they reported an error. It was
(and probably still it) used in a couple of cases to prevent extraneous
follow-on errors. I don't have a problem changing the pattern.

[Bob Nystrom, 2016/03/08 19:00:50]

I'm not sure about the other methods, but this one was only called in one place
and that place didn't use the return value so I figured I might as well clean up
a bit of unused code while I was in here.
SGTM. I didn't look at the other "_check___" methods closely, but my hunch is
they mostly report errors themselves now and don't need a return value either,
though I didn't want to mix a bunch of extra clean-up changes in this CL.

       Expression yieldExpression, bool isYieldEach) {
     assert(_inGenerator);
     if (_enclosingFunction == null) {
-      return false;
+      return;
     }
     DartType declaredReturnType = _enclosingFunction.returnType;
     DartType staticYieldedType = getStaticType(yieldExpression);
@@ -5691,7 +5746,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
           StaticTypeWarningCode.YIELD_OF_INVALID_TYPE,
           yieldExpression,
           [impliedReturnType, declaredReturnType]);
-      return true;
+      return;
     }
     if (isYieldEach) {
       // Since the declared return type might have been "dynamic", we need to
@@ -5708,10 +5763,9 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
             StaticTypeWarningCode.YIELD_OF_INVALID_TYPE,
             yieldExpression,
             [impliedReturnType, requiredReturnType]);
-        return true;
+        return;
       }
     }
-    return false;
   }
 
   /**