Fixes #151

lib/src/checker/checker.dart

 // Copyright (c) 2015, 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 dev_compiler.src.checker.checker;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
 import 'package:analyzer/src/generated/scanner.dart' show Token, TokenType;
@@ skipping 319 matching lines @@
     if (info.level >= logger.Level.SEVERE) _failure = true;
     _reporter.log(info);
   }
 }
 
 /// Checks the body of functions and properties.
 class CodeChecker extends RecursiveAstVisitor {
   final TypeRules _rules;
   final CheckerReporter _reporter;
   final _OverrideChecker _overrideChecker;
-  bool _constantContext = false;
   bool _failure = false;
   bool get failure => _failure || _overrideChecker._failure;
 
   CodeChecker(
       TypeRules rules, CheckerReporter reporter, StrongModeOptions options)
       : _rules = rules,
         _reporter = reporter,
         _overrideChecker = new _OverrideChecker(rules, reporter, options);
 
   @override
   void visitCompilationUnit(CompilationUnit unit) {
     void report(Expression expr) {
       _reporter.log(new MissingTypeError(expr));
     }
     var callback = _rules.reportMissingType;
     _rules.reportMissingType = report;
     unit.visitChildren(this);
     _rules.reportMissingType = callback;
   }
 
-  _visitMaybeConst(AstNode n, visitNode(AstNode n)) {
-    var o = _constantContext;
-    if (!o) {
-      if (n is VariableDeclarationList) {
-        _constantContext = o || n.isConst;
-      } else if (n is VariableDeclaration) {
-        _constantContext = o || n.isConst;
-      } else if (n is DefaultFormalParameter) {
-        _constantContext = true;
-      } else if (n is FormalParameter) {
-        _constantContext = o || n.isConst;
-      } else if (n is InstanceCreationExpression) {
-        _constantContext = o || n.isConst;
-      } else if (n is ConstructorDeclaration) {
-        _constantContext = o || n.element.isConst;
-      }
-    }
-    visitNode(n);
-    _constantContext = o;
-  }
-
   @override
   void visitComment(Comment node) {
     // skip, no need to do typechecking inside comments (they may contain
     // comment references which would require resolution).
   }
 
   @override
   void visitClassDeclaration(ClassDeclaration node) {
     _overrideChecker.check(node);
     super.visitClassDeclaration(node);
   }
 
   @override
   void visitAssignmentExpression(AssignmentExpression node) {
     var token = node.operator;
     if (token.type != TokenType.EQ) {
       _checkCompoundAssignment(node);
     } else {
       DartType staticType = _rules.getStaticType(node.leftHandSide);
       checkAssignment(node.rightHandSide, staticType);
     }
     node.visitChildren(this);
   }
 
   /// Check constructor declaration to ensure correct super call placement.
   @override
   void visitConstructorDeclaration(ConstructorDeclaration node) {
-    _visitMaybeConst(node, (node) {
-      node.visitChildren(this);
+    node.visitChildren(this);
 
-      final init = node.initializers;
-      for (int i = 0, last = init.length - 1; i < last; i++) {
-        final node = init[i];
-        if (node is SuperConstructorInvocation) {
-          _recordMessage(new InvalidSuperInvocation(node));
-        }
+    final init = node.initializers;
+    for (int i = 0, last = init.length - 1; i < last; i++) {
+      final node = init[i];
+      if (node is SuperConstructorInvocation) {
+        _recordMessage(new InvalidSuperInvocation(node));
       }
-    });
+    }
   }
 
   @override
   void visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
     var field = node.fieldName;
     DartType staticType = _rules.elementType(field.staticElement);
     checkAssignment(node.expression, staticType);
     node.visitChildren(this);
   }
 
@@ skipping 166 matching lines @@
     var element = node.staticElement;
     if (element == null) {
       _recordMessage(new MissingTypeError(node));
     } else {
       var type = node.staticElement.type;
       checkArgumentList(node.argumentList, type);
     }
     node.visitChildren(this);
   }
 
-  AstNode _getOwnerFunction(AstNode node) {
-    var parent = node.parent;
-    while (parent is! FunctionExpression &&
-        parent is! MethodDeclaration &&
-        parent is! ConstructorDeclaration) {
-      parent = parent.parent;
+  DartType _getExpectedReturnType(FunctionBody body) {
+    FunctionType functionType;
+    var parent = body.parent;
+    if (parent is Declaration) {
+      functionType = _rules.elementType(parent.element);
+    } else {
+      assert(parent is FunctionExpression);
+      functionType = _rules.getStaticType(parent);
     }
-    return parent;
-  }
 
-  FunctionType _getFunctionType(AstNode node) {
-    if (node is Declaration) {
-      return _rules.elementType(node.element);
+    var type = functionType.returnType;
+    var provider = _rules.provider;
+    if (type.isDynamic) return provider.dynamicType;
+
+    InterfaceType expectedType = null;
+    if (body.isAsynchronous) {
+      if (body.isGenerator) {
+        // Stream<T> -> T
+        expectedType = provider.streamType;
+      } else {
+        // Future<T> -> T
+        // TODO(vsm): Revisit with issue #228.
+        expectedType = provider.futureType;
+      }
     } else {
-      assert(node is FunctionExpression);
-      return _rules.getStaticType(node);
+      if (body.isGenerator) {
+        // Iterable<T> -> T
+        expectedType = provider.iterableType;
+      } else {
+        // T -> T
+        return type;
+      }
+    }
+    if (type is InterfaceType && type.element == expectedType.element) {
+      return type.typeArguments[0];
+    } else {
+      return null;
     }
   }
 
-  void _checkReturn(Expression expression, AstNode node) {
-    var type = _getFunctionType(_getOwnerFunction(node)).returnType;
+  FunctionBody _getFunctionBody(AstNode node) {
+    while (node is! FunctionBody) {
+      node = node.parent;
+    }
+    return node as FunctionBody;
+  }
+
+  void _checkReturnOrYield(Expression expression, AstNode node) {
+    var body = _getFunctionBody(node);
+    var type = _getExpectedReturnType(body);
+    if (type == null) {
+      // We have a type mismatch: the async/async*/sync* modifier does
+      // not match the return or yield type.  We should have already gotten an
+      // analyzer error in this case.
+      return;
+    }
     // TODO(vsm): Enforce void or dynamic (to void?) when expression is null.
     if (expression != null) checkAssignment(expression, type);
   }
 
   @override
   void visitExpressionFunctionBody(ExpressionFunctionBody node) {
-    _checkReturn(node.expression, node);
+    _checkReturnOrYield(node.expression, node);
     node.visitChildren(this);
   }
 
   @override
   void visitReturnStatement(ReturnStatement node) {
-    _checkReturn(node.expression, node);
+    _checkReturnOrYield(node.expression, node);
     node.visitChildren(this);
   }
 
+  @override
+  void visitYieldStatement(YieldStatement node) {
+    _checkReturnOrYield(node.expression, node);

[Leaf, 2015/06/19 22:15:27]

I think this will do the wrong thing if this yield statement is a yield*
statement.  For yield* e you want to check the type of e against the return
type, but I think this will fall into one of the generator cases above and
project out the type argument.

+    node.visitChildren(this);
+  }
+
   @override
   void visitPropertyAccess(PropertyAccess node) {
     var target = node.realTarget;
     if (_rules.isDynamicTarget(target)) {
       _recordDynamicInvoke(node, target);
     }
     node.visitChildren(this);
   }
 
   @override
   void visitPrefixedIdentifier(PrefixedIdentifier node) {
     final target = node.prefix;
     if (_rules.isDynamicTarget(target)) {
       _recordDynamicInvoke(node, target);
     }
     node.visitChildren(this);
   }
 
   @override
   void visitDefaultFormalParameter(DefaultFormalParameter node) {
-    _visitMaybeConst(node, (node) {
-      // Check that defaults have the proper subtype.
-      var parameter = node.parameter;
-      var parameterType = _rules.elementType(parameter.element);
-      assert(parameterType != null);
-      var defaultValue = node.defaultValue;
-      if (defaultValue == null) {
-        if (_rules.maybeNonNullableType(parameterType)) {
-          var staticInfo = new InvalidVariableDeclaration(
-              _rules, node.identifier, parameterType);
-          _recordMessage(staticInfo);
-        }
-      } else {
-        checkAssignment(defaultValue, parameterType);
+    // Check that defaults have the proper subtype.
+    var parameter = node.parameter;
+    var parameterType = _rules.elementType(parameter.element);
+    assert(parameterType != null);
+    var defaultValue = node.defaultValue;
+    if (defaultValue == null) {
+      if (_rules.maybeNonNullableType(parameterType)) {
+        var staticInfo = new InvalidVariableDeclaration(
+            _rules, node.identifier, parameterType);
+        _recordMessage(staticInfo);
       }
+    } else {
+      checkAssignment(defaultValue, parameterType);
+    }
 
-      node.visitChildren(this);
-    });
+    node.visitChildren(this);
   }
 
   @override
   void visitFieldFormalParameter(FieldFormalParameter node) {
     var element = node.element;
     var typeName = node.type;
     if (typeName != null) {
       var type = _rules.elementType(element);
       var fieldElement =
           node.identifier.staticElement as FieldFormalParameterElement;
       var fieldType = _rules.elementType(fieldElement.field);
       if (!_rules.isSubTypeOf(type, fieldType)) {
         var staticInfo =
             new InvalidParameterDeclaration(_rules, node, fieldType);
         _recordMessage(staticInfo);
       }
     }
     node.visitChildren(this);
   }
 
   @override
   void visitInstanceCreationExpression(InstanceCreationExpression node) {
-    _visitMaybeConst(node, (node) {
-      var arguments = node.argumentList;
-      var element = node.staticElement;
-      if (element != null) {
-        var type = _rules.elementType(node.staticElement);
-        checkArgumentList(arguments, type);
-      } else {
-        _recordMessage(new MissingTypeError(node));
-      }
-      node.visitChildren(this);
-    });
+    var arguments = node.argumentList;
+    var element = node.staticElement;
+    if (element != null) {
+      var type = _rules.elementType(node.staticElement);
+      checkArgumentList(arguments, type);
+    } else {
+      _recordMessage(new MissingTypeError(node));
+    }
+    node.visitChildren(this);
   }
 
   @override
   void visitVariableDeclarationList(VariableDeclarationList node) {
-    _visitMaybeConst(node, (node) {
-      TypeName type = node.type;
-      if (type == null) {
-        // No checks are needed when the type is var. Although internally the
-        // typing rules may have inferred a more precise type for the variable
-        // based on the initializer.
-      } else {
-        var dartType = getType(type);
-        for (VariableDeclaration variable in node.variables) {
-          var initializer = variable.initializer;
-          if (initializer != null) {
-            checkAssignment(initializer, dartType);
-          } else if (_rules.maybeNonNullableType(dartType)) {
-            var element = variable.element;
-            if (element is FieldElement && !element.isStatic) {
-              // Initialized - possibly implicitly - during construction.
-              // Handle this via a runtime check during code generation.
+    TypeName type = node.type;
+    if (type == null) {
+      // No checks are needed when the type is var. Although internally the
+      // typing rules may have inferred a more precise type for the variable
+      // based on the initializer.
+    } else {
+      var dartType = getType(type);
+      for (VariableDeclaration variable in node.variables) {
+        var initializer = variable.initializer;
+        if (initializer != null) {
+          checkAssignment(initializer, dartType);
+        } else if (_rules.maybeNonNullableType(dartType)) {
+          var element = variable.element;
+          if (element is FieldElement && !element.isStatic) {
+            // Initialized - possibly implicitly - during construction.
+            // Handle this via a runtime check during code generation.
 
-              // TODO(vsm): Detect statically whether this can fail and
-              // report a static error (must fail) or warning (can fail).
-            } else {
-              var staticInfo =
-                  new InvalidVariableDeclaration(_rules, variable, dartType);
-              _recordMessage(staticInfo);
-            }
+            // TODO(vsm): Detect statically whether this can fail and
+            // report a static error (must fail) or warning (can fail).
+          } else {
+            var staticInfo =
+                new InvalidVariableDeclaration(_rules, variable, dartType);
+            _recordMessage(staticInfo);
           }
         }
       }
-      node.visitChildren(this);
-    });
-  }
-
-  @override
-  void visitVariableDeclaration(VariableDeclaration node) {
-    _visitMaybeConst(node, super.visitVariableDeclaration);
+    }
+    node.visitChildren(this);
   }
 
   void _checkRuntimeTypeCheck(AstNode node, TypeName typeName) {
     var type = getType(typeName);
     if (!_rules.isGroundType(type)) {
       _recordMessage(new InvalidRuntimeCheckError(node, type));
     }
   }
 
   @override
@@ skipping 115 matching lines @@
   /// Analyzer checks boolean conversions, but we need to check too, because
   /// it uses the default assignability rules that allow `dynamic` and `Object`
   /// to be assigned to bool with no message.
   void checkBoolean(Expression expr) =>
       checkAssignment(expr, _rules.provider.boolType);
 
   void checkAssignment(Expression expr, DartType type) {
     if (expr is ParenthesizedExpression) {
       checkAssignment(expr.expression, type);
     } else {
-      _recordMessage(_rules.checkAssignment(expr, type, _constantContext));
+      _recordMessage(_rules.checkAssignment(expr, type));
     }
   }
 
   DartType _specializedBinaryReturnType(
       TokenType op, DartType t1, DartType t2, DartType normalReturnType) {
     // This special cases binary return types as per 16.26 and 16.27 of the
     // Dart language spec.
     switch (op) {
       case TokenType.PLUS:
       case TokenType.MINUS:
@@ skipping 50 matching lines @@
         } else {
           // Static type error
           staticInfo = new StaticTypeError(_rules, expr, lhsType);
         }
         _recordMessage(staticInfo);
       }
 
       // Check the rhs type
       if (staticInfo is! CoercionInfo) {
         var paramType = paramTypes.first;
-        staticInfo = _rules.checkAssignment(
-            expr.rightHandSide, paramType, _constantContext);
+        staticInfo = _rules.checkAssignment(expr.rightHandSide, paramType);
         _recordMessage(staticInfo);
       }
     }
   }
 
   void _recordDynamicInvoke(AstNode node, AstNode target) {
     var dinvoke = new DynamicInvoke(_rules, node);
     _reporter.log(dinvoke);
     // TODO(jmesserly): we may eventually want to record if the whole operation
     // (node) was dynamic, rather than the target, but this is an easier fit
     // with what we used to do.
     DynamicInvoke.set(target, true);
   }
 
   void _recordMessage(StaticInfo info) {
     if (info == null) return;
     if (info.level >= logger.Level.SEVERE) _failure = true;
     _reporter.log(info);
     if (info is CoercionInfo) {
       assert(CoercionInfo.get(info.node) == null);
       CoercionInfo.set(info.node, info);
     }
   }
 }