Migration: Rework handling of assignments to FutureOr.

This should address ~13 exceptions whose stack trace includes the line:

_AssignmentChecker._checkAssignment_recursion (package:nnbd_migration/src/edge_builder.dart:2089:7)

Change-Id: I74e78885c996b637d73f80ca2dabba0226fa1fff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116366
Reviewed-by: Mike Fairhurst <mfairhurst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

pkg/nnbd_migration/lib/src/edge_builder.dart

@@ -37,6 +37,9 @@ class AssignmentCheckerForTesting extends Object with _AssignmentChecker {
   @override
   final TypeSystem _typeSystem;
 
+  @override
+  final TypeProvider _typeProvider;
+
   final NullabilityGraph _graph;
 
   /// Tests should fill in this map with the bounds of any type parameters being
@@ -46,8 +49,8 @@ class AssignmentCheckerForTesting extends Object with _AssignmentChecker {
   @override
   final DecoratedClassHierarchy _decoratedClassHierarchy;
 
-  AssignmentCheckerForTesting(
-      this._typeSystem, this._graph, this._decoratedClassHierarchy);
+  AssignmentCheckerForTesting(this._typeSystem, this._typeProvider, this._graph,
+      this._decoratedClassHierarchy);
 
   void checkAssignment(EdgeOrigin origin,
       {@required DecoratedType source,
@@ -1966,6 +1969,8 @@ mixin _AssignmentChecker {
 
   NullabilityGraph get _graph;
 
+  TypeProvider get _typeProvider;
+
   TypeSystem get _typeSystem;
 
   /// Creates the necessary constraint(s) for an assignment from [source] to
@@ -1987,6 +1992,32 @@ mixin _AssignmentChecker {
       {@required DecoratedType source, @required DecoratedType destination}) {
     var sourceType = source.type;
     var destinationType = destination.type;
+    if (destinationType.isDartAsyncFutureOr) {
+      // (From the subtyping spec):
+      // if T1 is FutureOr<S1> then T0 <: T1 iff any of the following hold:
+      // - either T0 <: Future<S1>
+      var s1 = destination.typeArguments[0];
+      if (_typeSystem.isSubtypeOf(
+          sourceType, _typeProvider.futureType.instantiate([s1.type]))) {
+        // E.g. FutureOr<int> = (... as Future<int>)
+        // This is handled by the InterfaceType logic below, since we treat
+        // FutureOr as a supertype of Future.
+      }
+      // - or T0 <: S1
+      else if (_typeSystem.isSubtypeOf(sourceType, s1.type)) {
+        // E.g. FutureOr<int> = (... as int)
+        _checkAssignment_recursion(origin, source: source, destination: s1);
+        return;
+      }
+      // - or T0 is X0 and X0 has bound S0 and S0 <: T1
+      // - or T0 is X0 & S0 and S0 <: T1
+      else if (sourceType is TypeParameterType) {
+        throw UnimplementedError('TODO(paulberry)');
+      } else {
+        // Not a subtype; this must be a downcast.
+        throw UnimplementedError('TODO(paulberry)');
+      }
+    }
     if (sourceType.isBottom || sourceType.isDartCoreNull) {
       // No further edges need to be created, since all types are trivially
       // supertypes of bottom (and of Null, in the pre-migration world).
@@ -2019,15 +2050,6 @@ mixin _AssignmentChecker {
         destinationType is InterfaceType) {
       if (_typeSystem.isSubtypeOf(sourceType, destinationType)) {
         // Ordinary (upcast) assignment.  No cast necessary.
-        if (destinationType.isDartAsyncFutureOr) {
-          if (_typeSystem.isSubtypeOf(
-              sourceType, destinationType.typeArguments[0])) {
-            // We are looking at T <: FutureOr<U>.  So treat this as T <: U.
-            _checkAssignment_recursion(origin,
-                source: source, destination: destination.typeArguments[0]);
-            return;
-          }
-        }
         var rewrittenSource = _decoratedClassHierarchy.asInstanceOf(
             source, destinationType.element);
         assert(rewrittenSource.typeArguments.length ==

pkg/nnbd_migration/test/edge_builder_test.dart

@@ -48,8 +48,8 @@ class AssignmentCheckerTest extends Object
     var typeProvider = TestTypeProvider();
     var graph = NullabilityGraphForTesting();
     var decoratedClassHierarchy = _DecoratedClassHierarchyForTesting();
-    var checker = AssignmentCheckerForTesting(
-        Dart2TypeSystem(typeProvider), graph, decoratedClassHierarchy);
+    var checker = AssignmentCheckerForTesting(Dart2TypeSystem(typeProvider),
+        typeProvider, graph, decoratedClassHierarchy);
     var assignmentCheckerTest =
         AssignmentCheckerTest._(typeProvider, graph, checker);
     decoratedClassHierarchy.assignmentCheckerTest = assignmentCheckerTest;
@@ -165,6 +165,14 @@ class AssignmentCheckerTest extends Object
     assertEdge(t1.returnType.node, t2.returnType.node, hard: false);
   }
 
+  void test_future_int_to_future_or_int() {
+    var t1 = future(int_());
+    var t2 = futureOr(int_());
+    assign(t1, t2, hard: true);
+    assertEdge(t1.node, t2.node, hard: true);
+    assertEdge(t1.typeArguments[0].node, t2.typeArguments[0].node, hard: false);
+  }
+
   test_generic_to_dynamic() {
     var t = list(object());
     assign(t, dynamic_);
@@ -227,6 +235,21 @@ class AssignmentCheckerTest extends Object
     assertNoEdge(t.typeArguments[0].node, anyNode);
   }
 
+  void test_int_to_future_or_int() {
+    var t1 = int_();
+    var t2 = futureOr(int_());
+    assign(t1, t2, hard: true);
+    // Note: given code like:
+    //   int x = null;
+    //   FutureOr<int> y = x;
+    // There are two possible migrations for `FutureOr<int>`: we could change it
+    // to either `FutureOr<int?>` or `FutureOr<int>?`.  We choose to do
+    // `FutureOr<int>?` because it is a narrower type, so it is less likely to
+    // cause a proliferation of nullable types in the user's program.
+    assertEdge(t1.node, t2.node, hard: true);
+    assertNoEdge(t1.node, t2.typeArguments[0].node);
+  }
+
   void test_null_to_generic() {
     var t = list(object());
     assign(null_, t);
@@ -4650,6 +4673,11 @@ class _DecoratedClassHierarchyForTesting implements DecoratedClassHierarchy {
       return assignmentCheckerTest._myListOfListSupertype
           .substitute({class_.typeParameters[0]: type.typeArguments[0]});
     }
+    if (class_.name == 'Future' && superclass.name == 'FutureOr') {
+      return DecoratedType(
+          superclass.type.instantiate([type.typeArguments[0].type]), type.node,
+          typeArguments: [type.typeArguments[0]]);
+    }
     throw UnimplementedError(
         'TODO(paulberry): asInstanceOf($type, $superclass)');
   }

pkg/nnbd_migration/test/migration_visitor_test_base.dart

@@ -79,6 +79,19 @@ mixin DecoratedTypeTester implements DecoratedTypeTesterBase {
         namedParameters: named);
   }
 
+  DecoratedType future(DecoratedType parameter, {NullabilityNode node}) {
+    return DecoratedType(typeProvider.futureType.instantiate([parameter.type]),
+        node ?? newNode(),
+        typeArguments: [parameter]);
+  }
+
+  DecoratedType futureOr(DecoratedType parameter, {NullabilityNode node}) {
+    return DecoratedType(
+        typeProvider.futureOrType.instantiate([parameter.type]),
+        node ?? newNode(),
+        typeArguments: [parameter]);
+  }
+
   DecoratedType int_({NullabilityNode node}) =>
       DecoratedType(typeProvider.intType, node ?? newNode());