Reify implicit casts for FutureOr

pkg/analyzer/lib/src/task/strong/checker.dart

Index: pkg/analyzer/lib/src/task/strong/checker.dart
diff --git a/pkg/analyzer/lib/src/task/strong/checker.dart b/pkg/analyzer/lib/src/task/strong/checker.dart
index 91134c2f558bf9a5bd17219eb60ee654b390a389..84579e91e7923c9a462d02d8c5561010585031c9 100644
--- a/pkg/analyzer/lib/src/task/strong/checker.dart
+++ b/pkg/analyzer/lib/src/task/strong/checker.dart
@@ -742,7 +742,7 @@ class CodeChecker extends RecursiveAstVisitor {
       {DartType from, bool opAssign: false}) {
     from ??= _getDefiniteType(expr);
 
-    if (_needsImplicitCast(expr, to, from: from)) {
+    if (_needsImplicitCast(expr, to, from: from) == true) {
       _recordImplicitCast(expr, to, from: from, opAssign: opAssign);
     }
   }
@@ -841,9 +841,11 @@ class CodeChecker extends RecursiveAstVisitor {
         // Stream<T> -> T
         expectedType = typeProvider.streamType;
       } else {
-        // Don't validate return type of async methods.
-        // They're handled by the runtime implementation.
-        return null;
+        // Future<T> -> FutureOr<T>
+        var typeArg = (type.element == typeProvider.futureType.element)
+            ? type.typeArguments[0]
+            : typeProvider.dynamicType;
+        return typeProvider.futureOrType.instantiate([typeArg]);
       }
     } else {
       if (body.isGenerator) {
@@ -907,9 +909,10 @@ class CodeChecker extends RecursiveAstVisitor {
   }
 
   /// Returns true if we need an implicit cast of [expr] from [from] type to
-  /// [to] type, otherwise returns false.
+  /// [to] type, returns false if no cast is needed, and returns null if the
+  /// types are statically incompatible.
   ///
-  /// If [from] is omitted, uses the static type of [expr].
+  /// If [from] is omitted, uses the static type of [expr]
   bool _needsImplicitCast(Expression expr, DartType to, {DartType from}) {
     from ??= _getDefiniteType(expr);
 
@@ -933,10 +936,19 @@ class CodeChecker extends RecursiveAstVisitor {
       return true;
     }
 
+    // TODO(vsm): Resolve assignability / implicit cast around FutureOr.
+    // Unwrap FutureOr union to check for a cast.

[Leaf, 2017/04/05 20:47:27]

This will solve the async issue, but will also generate downcasts for other
cases that are sideways casts.  I don't think this is a problem, in fact I'm not
sure why we care about eliding sideways casts.

[vsm, 2017/04/07 22:23:08]

I've cleaned this up to just use assignability.

+    if (to.element == typeProvider.futureOrType.element) {
+      var to1 = to.typeArguments[0];
+      var to2 = typeProvider.futureType.instantiate([to1]);
+      return _needsImplicitCast(expr, to1, from: from) == true ||
+          _needsImplicitCast(expr, to2, from: from) == true;
+    }
+
     // Anything else is an illegal sideways cast.
     // However, these will have been reported already in error_verifier, so we
     // don't need to report them again.
-    return false;
+    return null;

[Leaf, 2017/04/05 20:47:27]

Is there any reason not to just generate the cast even when it's a sideways
cast?  Then you could just get rid of the new code above.

[vsm, 2017/04/07 22:23:07]

Injecting the cast here spams the error list with a ton of hints (and a big diff
in the tests).

   }
 
   void _recordDynamicInvoke(AstNode node, Expression target) {
@@ -953,7 +965,9 @@ class CodeChecker extends RecursiveAstVisitor {
   /// the AST node.
   void _recordImplicitCast(Expression expr, DartType to,
       {DartType from, bool opAssign: false}) {
-    assert(rules.isSubtypeOf(to, from));
+    // TODO(vsm): Resolve assignability / implicit cast around FutureOr.
+    assert(rules.isSubtypeOf(to, from) ||
+        to.element == typeProvider.futureOrType.element);
 
     // Inference "casts":
     if (expr is Literal) {