Reify implicit casts for FutureOr

pkg/analyzer/lib/src/task/strong/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.
 
 // TODO(jmesserly): this was ported from package:dev_compiler, and needs to be
 // refactored to fit into analyzer.
 library analyzer.src.task.strong.checker;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/dart/ast/ast.dart';
@@ skipping 724 matching lines @@
   /// needed, and if so records it.
   ///
   /// If [from] is omitted, uses the static type of [expr].
   ///
   /// If [expr] does not require an implicit cast because it is not related to
   /// [to] or is already a subtype of it, does nothing.
   void _checkImplicitCast(Expression expr, DartType to,
       {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);
     }
   }
 
   /// Checks if the assignment is valid with respect to non-nullable types.
   /// Returns `false` if a nullable expression is assigned to a variable of
   /// non-nullable type and `true` otherwise.
   bool _checkNonNullAssignment(
       Expression expression, DartType to, DartType from) {
     if (rules.isNonNullableType(to) && rules.isNullableType(from)) {
@@ skipping 78 matching lines @@
     }
 
     var type = functionType.returnType;
 
     InterfaceType expectedType = null;
     if (body.isAsynchronous) {
       if (body.isGenerator) {
         // 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) {
         // Iterable<T> -> T
         expectedType = typeProvider.iterableType;
       } else {
         // T -> T
         return type;
       }
     }
@@ skipping 43 matching lines @@
     // a dynamic parameter type requires a dynamic call in general.
     // However, as an optimization, if we have an original definition, we know
     // dynamic is reified as Object - in this case a regular call is fine.
     if (hasStrictArrow(call.function)) {
       return false;
     }
     return rules.anyParameterType(ft, (pt) => pt.isDynamic);
   }
 
   /// 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);
 
     if (!_checkNonNullAssignment(expr, to, from)) return false;
 
     // We can use anything as void.
     if (to.isVoid) return false;
 
     // fromT <: toT, no coercion needed.
     if (rules.isSubtypeOf(from, to)) return false;
 
     // Note: a function type is never assignable to a class per the Dart
     // spec - even if it has a compatible call method.  We disallow as
     // well for consistency.
     if (from is FunctionType && rules.getCallMethodType(to) != null) {
       return false;
     }
 
     // Downcast if toT <: fromT
     if (rules.isSubtypeOf(to, from)) {
       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) {
     _recordMessage(node, StrongModeCode.DYNAMIC_INVOKE, [node]);
     // 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.
     if (target != null) setIsDynamicInvoke(target, true);
   }
 
   /// Records an implicit cast for the [expr] from [from] to [to].
   ///
   /// This will emit the appropriate error/warning/hint message as well as mark
   /// 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) {
       // fromT should be an exact type - this will almost certainly fail at
       // runtime.
       if (expr is ListLiteral) {
         _recordMessage(
             expr, StrongModeCode.INVALID_CAST_LITERAL_LIST, [from, to]);
       } else if (expr is MapLiteral) {
         _recordMessage(
@@ skipping 596 matching lines @@
     var visited = new Set<InterfaceType>();
     do {
       visited.add(current);
       current.mixins.reversed.forEach(
           (m) => _checkIndividualOverridesFromClass(node, m, seen, true));
       _checkIndividualOverridesFromClass(node, current.superclass, seen, true);
       current = current.superclass;
     } while (!current.isObject && !visited.contains(current));
   }
 }