Type check for-in statements.

This does nominal checking on the iterable expression used in a for-in
statement:

1. It must implement Iterable.
2. The type argument to Iterable must be assignable to the variable's
   type.

R=brianwilkerson@google.com, jmesserly@google.com

Review URL: https://codereview.chromium.org/1771153002 .

pkg/analysis_server/lib/src/provisional/completion/dart/completion_plugin.dart

@@ -57,7 +57,7 @@ class DartCompletionPlugin implements Plugin {
   /**
    * Return a list containing all of the Dart specific completion contributors.
    */
-  Iterable<DartCompletionContributorFactory> get contributors =>
+  Iterable<DartCompletionContributor> get contributors =>
       _contributorExtensionPoint.extensions
           .map((DartCompletionContributorFactory factory) => factory());
 

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

@@ -2743,6 +2743,8 @@ abstract class ErrorCode {
     StaticTypeWarningCode.UNQUALIFIED_REFERENCE_TO_NON_LOCAL_STATIC_MEMBER,
     StaticTypeWarningCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS,
     StaticTypeWarningCode.YIELD_OF_INVALID_TYPE,
+    StaticTypeWarningCode.FOR_IN_OF_INVALID_TYPE,
+    StaticTypeWarningCode.FOR_IN_OF_INVALID_ELEMENT_TYPE,
     StaticWarningCode.AMBIGUOUS_IMPORT,
     StaticWarningCode.ARGUMENT_TYPE_NOT_ASSIGNABLE,
     StaticWarningCode.ASSIGNMENT_TO_CONST,
@@ -4395,6 +4397,32 @@ class StaticTypeWarningCode extends ErrorCode {
       const StaticTypeWarningCode('YIELD_OF_INVALID_TYPE',
           "The type '{0}' implied by the 'yield' expression must be assignable to '{1}'");
 
+  /**
+   * 17.6.2 For-in. If the iterable expression does not implement Iterable,
+   * this warning is reported.
+   *
+   * Parameters:
+   * 0: The type of the iterable expression.
+   * 1: The sequence type -- Iterable for `for` or Stream for `await for`.
+   */
+  static const StaticTypeWarningCode FOR_IN_OF_INVALID_TYPE =
+  const StaticTypeWarningCode('FOR_IN_OF_INVALID_TYPE',
+      "The type '{0}' used in the 'for' loop must implement {1}");
+
+  /**
+   * 17.6.2 For-in. It the iterable expression does not implement Iterable with
+   * a type argument that can be assigned to the for-in variable's type, this
+   * warning is reported.
+   *
+   * Parameters:
+   * 0: The type of the iterable expression.
+   * 1: The sequence type -- Iterable for `for` or Stream for `await for`.
+   * 2: The loop variable type.
+   */
+  static const StaticTypeWarningCode FOR_IN_OF_INVALID_ELEMENT_TYPE =
+      const StaticTypeWarningCode('FOR_IN_OF_INVALID_ELEMENT_TYPE',
+          "The type '{0}' used in the 'for' loop must implement {1} with a type argument that can be assigned to '{2}'");
+
   /**
    * Initialize a newly created error code to have the given [name]. The message
    * associated with the error will be created from the given [message]

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

@@ -688,6 +688,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     return super.visitForStatement(node);
   }
 
+  @override
+  Object visitForEachStatement(ForEachStatement node) {
+    _checkForInIterable(node);
+    return super.visitForEachStatement(node);
+  }
+
   @override
   Object visitFunctionDeclaration(FunctionDeclaration node) {
     ExecutableElement outerFunction = _enclosingFunction;
@@ -5662,17 +5668,66 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     return false;
   }
 
+  /**
+   * 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
+        ? 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
+        ? "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()?
+    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(
       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;
   }
 
   /**

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

@@ -8761,9 +8761,7 @@ class ResolverVisitor extends ScopedVisitor {
         InterfaceType wrapperType = _enclosingFunction.isSynchronous
             ? typeProvider.iterableType
             : typeProvider.streamType;
-        List<DartType> candidates =
-            _findImplementedTypeArgument(type, wrapperType);
-        type = InterfaceTypeImpl.findMostSpecificType(candidates, typeSystem);
+        type = typeSystem.mostSpecificTypeArgument(type, wrapperType);
       }
       if (type != null) {
         inferenceContext.addReturnOrYieldType(type);
@@ -8834,39 +8832,6 @@ class ResolverVisitor extends ScopedVisitor {
     return declaredType.flattenFutures(typeSystem);
   }
 
-  /**
-   * Starting from t1, search its class hierarchy for types of the form
-   * `t2<R>`, and return a list of the resulting R's.
-   *
-   * For example, given t1 = `List<int>` and t2 = `Iterable<T>`, this will
-   * return [int].
-   */
-  // TODO(jmesserly): this is very similar to code used for flattening futures.
-  // The only difference is, because of a lack of TypeProvider, the other method
-  // has to match the Future type by its name and library. Here was are passed
-  // in the correct type.
-  List<DartType> _findImplementedTypeArgument(DartType t1, InterfaceType t2) {
-    List<DartType> result = <DartType>[];
-    HashSet<ClassElement> visitedClasses = new HashSet<ClassElement>();
-    void recurse(InterfaceTypeImpl type) {
-      if (type.element == t2.element && type.typeArguments.isNotEmpty) {
-        result.add(type.typeArguments[0]);
-      }
-      if (visitedClasses.add(type.element)) {
-        if (type.superclass != null) {
-          recurse(type.superclass);
-        }
-        type.mixins.forEach(recurse);
-        type.interfaces.forEach(recurse);
-        visitedClasses.remove(type.element);
-      }
-    }
-    if (t1 is InterfaceType) {
-      recurse(t1);
-    }
-    return result;
-  }
-
   /**
    * The given expression is the expression used to compute the iterator for a
    * for-each statement. Attempt to compute the type of objects that will be

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

@@ -24,8 +24,6 @@ typedef bool _GuardedSubtypeChecker<T>(T t1, T t2, Set<Element> visited);
 class StrongTypeSystemImpl extends TypeSystem {
   final _specTypeSystem = new TypeSystemImpl();
 
-  StrongTypeSystemImpl();
-
   bool anyParameterType(FunctionType ft, bool predicate(DartType t)) {
     return ft.parameters.any((p) => predicate(p.type));
   }
@@ -508,6 +506,47 @@ abstract class TypeSystem {
    */
   bool isSubtypeOf(DartType leftType, DartType rightType);
 
+  /**
+   * Searches the superinterfaces of [type] for implementations of [genericType]
+   * and returns the most specific type argument used for that generic type.
+   *
+   * For example, given [type] `List<int>` and [genericType] `Iterable<T>`,
+   * returns [int].
+   *
+   * Returns `null` if [type] does not implement [genericType].
+   */
+  // TODO(jmesserly): this is very similar to code used for flattening futures.
+  // The only difference is, because of a lack of TypeProvider, the other method
+  // has to match the Future type by its name and library. Here was are passed
+  // in the correct type.
+  DartType mostSpecificTypeArgument(DartType type, DartType genericType) {
+    if (type is! InterfaceType) return null;
+
+    // Walk the superinterface hierarchy looking for [genericType].
+    List<DartType> candidates = <DartType>[];
+    HashSet<ClassElement> visitedClasses = new HashSet<ClassElement>();
+    void recurse(InterfaceTypeImpl interface) {
+      if (interface.element == genericType.element &&
+          interface.typeArguments.isNotEmpty) {
+        candidates.add(interface.typeArguments[0]);
+      }
+      if (visitedClasses.add(interface.element)) {
+        if (interface.superclass != null) {
+          recurse(interface.superclass);
+        }
+        interface.mixins.forEach(recurse);
+        interface.interfaces.forEach(recurse);
+        visitedClasses.remove(interface.element);
+      }
+    }
+
+    recurse(type);
+
+    // Since the interface may be implemented multiple times with different
+    // type arguments, choose the best one.
+    return InterfaceTypeImpl.findMostSpecificType(candidates, this);
+  }
+
   /**
    * Given a [DartType] type, return the [TypeParameterElement]s corresponding
    * to its formal type parameters (if any).

pkg/analyzer/test/generated/resolver_test.dart

@@ -8370,6 +8370,19 @@ class ResolverTestCase extends EngineTestCase {
     errorListener.assertErrorsWithCodes(expectedErrorCodes);
   }
 
+  /**
+   * Asserts that [code] has errors with the given error codes.
+   *
+   * Like [assertErrors], but takes a string of source code.
+   */
+  // TODO(rnystrom): Use this in more tests that have the same structure.
+  void assertErrorsInCode(String code, List<ErrorCode> errors) {
+    Source source = addSource(code);
+    computeLibrarySourceErrors(source);
+    assertErrors(source, errors);
+    verify([source]);
+  }
+
   /**
    * Assert that no errors have been reported against the given source.
    *
@@ -8381,6 +8394,17 @@ class ResolverTestCase extends EngineTestCase {
     assertErrors(source);
   }
 
+  /**
+   * Asserts that [code] has no errors or warnings.
+   */
+  // TODO(rnystrom): Use this in more tests that have the same structure.
+  void assertNoErrorsInCode(String code) {
+    Source source = addSource(code);
+    computeLibrarySourceErrors(source);
+    assertNoErrors(source);
+    verify([source]);
+  }
+
   /**
    * Cache the source file content in the source factory but don't add the source to the analysis
    * context. The file path should be absolute.

pkg/analyzer/test/generated/static_type_warning_code_test.dart

@@ -2039,6 +2039,208 @@ f(p) {
     verify([source]);
   }
 
+  void test_forIn_notIterable() {
+    assertErrorsInCode('''
+f() {
+  for (var i in true) {}
+}
+''', [StaticTypeWarningCode.FOR_IN_OF_INVALID_TYPE]);
+  }
+
+  void test_forIn_declaredVariableWrongType() {
+    assertErrorsInCode('''
+f() {
+  for (int i in <String>[]) {}
+}
+''', [StaticTypeWarningCode.FOR_IN_OF_INVALID_ELEMENT_TYPE]);
+  }
+
+  void test_forIn_existingVariableWrongType() {
+    assertErrorsInCode('''
+f() {
+  int i;
+  for (i in <String>[]) {}
+}
+''', [StaticTypeWarningCode.FOR_IN_OF_INVALID_ELEMENT_TYPE]);
+  }
+
+  void test_forIn_declaredVariableRightType() {
+    assertNoErrorsInCode('''
+f() {
+  for (int i in <int>[]) {}
+}
+''');
+  }
+
+  void test_forIn_existingVariableRightType() {
+    assertNoErrorsInCode('''
+f() {
+  int i;
+  for (i in <int>[]) {}
+}
+''');
+  }
+
+  void test_forIn_dynamicVariable() {
+    assertNoErrorsInCode('''
+f() {
+  for (var i in <int>[]) {}
+}
+''');
+  }
+
+  void test_forIn_iterableOfDynamic() {
+    assertNoErrorsInCode('''
+f() {
+  for (int i in []) {}
+}
+''');
+  }
+
+  void test_forIn_dynamicIterable() {
+    assertNoErrorsInCode('''
+f() {
+  dynamic iterable;
+  for (int i in iterable) {}
+}
+''');
+  }
+
+  void test_forIn_upcast() {
+    assertNoErrorsInCode('''
+f() {
+  for (num i in <int>[]) {}
+}
+''');
+  }
+
+  void test_forIn_downcast() {
+    assertNoErrorsInCode('''
+f() {
+  for (int i in <num>[]) {}
+}
+''');
+  }
+
+  void test_forIn_typeBoundBad() {
+    assertErrorsInCode('''
+class Foo<T extends Iterable<int>> {
+  void method(T iterable) {
+    for (String i in iterable) {}
+  }
+}
+''', [StaticTypeWarningCode.FOR_IN_OF_INVALID_ELEMENT_TYPE]);
+  }
+
+  void test_forIn_typeBoundGood() {
+    assertNoErrorsInCode('''
+class Foo<T extends Iterable<int>> {
+  void method(T iterable) {
+    for (var i in iterable) {}
+  }
+}
+''');
+  }
+
+  void test_awaitForIn_notStream() {
+    assertErrorsInCode('''
+f() async {
+  await for (var i in true) {}
+}
+''', [StaticTypeWarningCode.FOR_IN_OF_INVALID_TYPE]);
+  }
+
+  void test_awaitForIn_declaredVariableWrongType() {
+    assertErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream<String> stream;
+  await for (int i in stream) {}
+}
+''', [StaticTypeWarningCode.FOR_IN_OF_INVALID_ELEMENT_TYPE]);
+  }
+
+  void test_awaitForIn_existingVariableWrongType() {
+    assertErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream<String> stream;
+  int i;
+  await for (i in stream) {}
+}
+''', [StaticTypeWarningCode.FOR_IN_OF_INVALID_ELEMENT_TYPE]);
+  }
+
+  void test_awaitForIn_declaredVariableRightType() {
+    assertNoErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream<int> stream;
+  await for (int i in stream) {}
+}
+''');
+  }
+
+  void test_awaitForIn_existingVariableRightType() {
+    assertNoErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream<int> stream;
+  int i;
+  await for (i in stream) {}
+}
+''');
+  }
+
+  void test_awaitForIn_dynamicVariable() {
+    assertNoErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream<int> stream;
+  await for (var i in stream) {}
+}
+''');
+  }
+
+  void test_awaitForIn_streamOfDynamic() {
+    assertNoErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream stream;
+  await for (int i in stream) {}
+}
+''');
+  }
+
+  void test_awaitForIn_dynamicStream() {
+    assertNoErrorsInCode('''
+f() async {
+  dynamic stream;
+  await for (int i in stream) {}
+}
+''');
+  }
+
+  void test_awaitForIn_upcast() {
+    assertNoErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream<int> stream;
+  await for (num i in stream) {}
+}
+''');
+  }
+
+  void test_awaitForIn_downcast() {
+    assertNoErrorsInCode('''
+import 'dart:async';
+f() async {
+  Stream<num> stream;
+  await for (int i in stream) {}
+}
+''');
+  }
+
   void test_yield_async_to_basic_type() {
     Source source = addSource('''
 int f() async* {

runtime/observatory/tests/service/service.status

@@ -17,6 +17,10 @@ evaluate_activation_test/scope: RuntimeError # http://dartbug.com/20047
 # Tests with known analyzer issues
 [ $compiler == dart2analyzer ]
 developer_extension_test: SkipByDesign
+*: StaticWarning # https://github.com/dart-lang/observe/issues/85
+address_mapper_test: Pass # https://github.com/dart-lang/observe/issues/85
+command_test: Pass # https://github.com/dart-lang/observe/issues/85
+read_stream_test: Pass # https://github.com/dart-lang/observe/issues/85
 
 [ $arch == arm ]
 process_service_test: Pass, Fail # Issue 24344

tests/language/language_analyzer2.status

@@ -517,3 +517,6 @@ accessor_conflict_import2_test: StaticWarning # Issue 25624
 accessor_conflict_import_prefixed2_test: StaticWarning # Issue 25624
 accessor_conflict_import_prefixed_test: StaticWarning # Issue 25624
 accessor_conflict_import_test: StaticWarning # Issue 25624
+
+for_in3_test: StaticWarning, OK # Test should have warning by design.
+for_in_side_effects_test: StaticWarning, OK # Test uses custom class that does not implement Iterable in for-in.

tests/utils/utils.status

@@ -21,6 +21,12 @@ dummy_compiler_test: Slow, Pass
 [ $noopt || $compiler == precompiler || $mode == product ]
 source_mirrors_test: SkipByDesign # Imports dart:mirrors
 
+[ $compiler == dart2analyzer ]
+dart2js_test: StaticWarning # https://github.com/dart-lang/sdk/issues/25943
+recursive_import_test: StaticWarning # https://github.com/dart-lang/sdk/issues/25943
+dummy_compiler_test: StaticWarning # https://github.com/dart-lang/sdk/issues/25943
+source_mirrors_test: StaticWarning # https://github.com/dart-lang/sdk/issues/25943
+
 [ $compiler == dart2js && $cps_ir && $host_checked ]
 dummy_compiler_test: Crash # Issue 24485
 recursive_import_test: Crash # Issue 24485