[nnbd_migration] Restrict downcasts; make all type parameters nullable.

This makes trial_migration.dart report two new exceptions:

_EdgeBuilder&GeneralizingAstVisitor&_AssignmentChecker._checkAssignment (package:nnbd_migration/src/edge_builder.dart:2169:14) (x30)
_EdgeBuilder&GeneralizingAstVisitor&_AssignmentChecker._checkDowncast (package:nnbd_migration/src/edge_builder.dart:2351:14) (x7)

30 & 7 is not great but not terrible. These also do not break path, logging, or
charcode.

Failing tests show why we cannot correctly cover remaining cases without
a larger CL.

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

Author: MichaelRFairhurst

pkg/analysis_server/test/domain_edit_dartfix_test.dart

@@ -190,7 +190,9 @@ f(Iterable<int> i) {
 ''');
   }
 
+  @failingTest
   test_dartfix_nonNullable() async {
+    // Failing because this contains a side-cast from Null to int.
     createAnalysisOptionsFile(experiments: ['non-nullable']);
     addTestFile('''
 int f(int i) => 0;

pkg/nnbd_migration/lib/src/edge_builder.dart

@@ -1610,6 +1610,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
         destinationType = destinationExpression.accept(this);
       }
     }
+
     if (questionAssignNode != null) {
       _guards.add(destinationType.node);
     }
@@ -2153,6 +2154,21 @@ mixin _AssignmentChecker {
       {@required DecoratedType source,
       @required DecoratedType destination,
       @required bool hard}) {
+    var sourceType = source.type;
+    var destinationType = destination.type;
+    if (!_typeSystem.isSubtypeOf(sourceType, destinationType)) {
+      // Not a proper upcast assignment.
+      if (_typeSystem.isSubtypeOf(destinationType, sourceType)) {
+        // But rather a downcast.
+        _checkDowncast(origin,
+            source: source, destination: destination, hard: hard);
+        return;
+      }
+      // Neither a proper upcast assignment nor an implicit downcast (some
+      // illegal code, or we did something wrong to get here).
+      assert(false, 'side cast not supported: $sourceType to $destinationType');
+      return;
+    }
     _connect(source.node, destination.node, origin, hard: hard);
     _checkAssignment_recursion(origin,
         source: source, destination: destination);
@@ -2165,35 +2181,7 @@ mixin _AssignmentChecker {
       {@required DecoratedType source, @required DecoratedType destination}) {
     var sourceType = source.type;
     var destinationType = destination.type;
-    if (!_typeSystem.isSubtypeOf(sourceType, destinationType)) {
-      // Not a proper upcast assignment.  It is either an implicit downcast or
-      // some illegal code.  It's handled on a "best effort" basis.
-      if (destinationType is TypeParameterType &&
-          sourceType is! TypeParameterType) {
-        // Assume an assignment to the type parameter's bound.
-        _checkAssignment(origin,
-            source: source,
-            destination:
-                _getTypeParameterTypeBound(destination).withNode(_graph.always),
-            hard: false);
-        return;
-      }
-      if (sourceType is InterfaceType && destinationType is InterfaceType) {
-        if (_typeSystem.isSubtypeOf(destinationType, sourceType)) {
-          var rewrittenDestination = _decoratedClassHierarchy.asInstanceOf(
-              destination, sourceType.element);
-          assert(rewrittenDestination.typeArguments.length ==
-              source.typeArguments.length);
-          for (int i = 0; i < rewrittenDestination.typeArguments.length; i++) {
-            _checkAssignment(origin,
-                source: source.typeArguments[i],
-                destination: rewrittenDestination.typeArguments[i],
-                hard: false);
-          }
-        }
-      }
-      return;
-    }
+    assert(_typeSystem.isSubtypeOf(sourceType, destinationType));
     if (destinationType.isDartAsyncFutureOr) {
       var s1 = destination.typeArguments[0];
       if (sourceType.isDartAsyncFutureOr) {
@@ -2327,6 +2315,63 @@ mixin _AssignmentChecker {
     }
   }
 
+  void _checkDowncast(EdgeOrigin origin,
+      {@required DecoratedType source,
+      @required DecoratedType destination,
+      @required bool hard}) {
+    assert(_typeSystem.isSubtypeOf(destination.type, source.type));
+    // Nullability should narrow to maintain subtype relationship.
+    _connect(source.node, destination.node, origin, hard: hard);
+    if (source.type.isDynamic) {
+      assert(destination.typeFormals?.isEmpty ?? true,
+          'downcast to something with type parameters not yet supported.');
+      assert(destination is! FunctionType,
+          'downcast to function type not yet supported.');
+      if (destination.type is ParameterizedType) {
+        for (final param
+            in (destination.type as ParameterizedType).typeParameters) {
+          assert(param.type.bound.isDynamic,
+              'downcast to type parameters with bounds not supported');
+        }
+      }
+
+      for (final arg in destination.typeArguments) {
+        // We cannot assume we're downcasting to C<T!>. Downcast to C<T?>.
+        _checkDowncast(origin, source: source, destination: arg, hard: false);
+      }
+    } else if (destination.type is TypeParameterType &&
+        source.type is! TypeParameterType) {
+      // Assume an assignment to the type parameter's bound.
+      _checkAssignment(origin,
+          source: source,
+          destination:
+              _getTypeParameterTypeBound(destination).withNode(_graph.always),
+          hard: false);
+    } else if (destination.type is InterfaceTypeImpl) {
+      assert(source.typeArguments.isEmpty,
+          'downcast from interface type with type args not supported.');
+      if (destination.type is ParameterizedType) {
+        for (final param
+            in (destination.type as ParameterizedType).typeParameters) {
+          assert(param.type.bound.isDynamic,
+              'downcast to type parameters with bounds not supported');
+        }
+      }
+      for (final arg in destination.typeArguments) {
+        // We cannot assume we're downcasting to C<T!>. Downcast to C<T?>.
+        _checkDowncast(origin,
+            source: DecoratedType(_typeProvider.dynamicType, _graph.always),
+            destination: arg,
+            hard: false);
+      }
+    } else {
+      assert(
+          false,
+          'downcasting from ${source.type.runtimeType} to '
+          '${destination.type.runtimeType} not supported.');
+    }
+  }
+
   void _connect(
       NullabilityNode source, NullabilityNode destination, EdgeOrigin origin,
       {bool hard = false});

pkg/nnbd_migration/test/api_test.dart

@@ -981,6 +981,82 @@ int f(int i) {
     await _checkSingleFileChanges(content, expected);
   }
 
+  @failingTest
+  test_downcast_not_widest_type_type_parameters() async {
+    // Fails because a hard assignment from List<int/*1*/> to List<int/*2*/>
+    // doesn't create a hard edge from 1 to 2. Perhaps this is correct. In this
+    // example it seems incorrect.
+    var content = '''
+void f(dynamic a) {
+  List<int> hardToNonNullNonNull = a;
+  List<int> hardToNullNonNull = a;
+  List<int> hardToNonNullNull = a;
+  List<int/*!*/>/*!*/ nonNullNonNull;
+  List<int/*?*/>/*!*/ nullNonNull;
+  List<int/*!*/>/*?*/ nonNullNull;
+  nonNullNonNull = hardToNonNullNonNull
+  nonNullNull = hardToNonNullNull
+  nullNonNull = hardToNullNonNull
+}
+''';
+
+    // TODO(paulberry): remove the /*!*/, /*?*/ comments on migration.
+    var expected = '''
+void f(dynamic a) {
+  List<int> hardToNonNullNonNull = a;
+  List<int?> hardToNullNonNull = a;
+  List<int>? hardToNonNullNull = a;
+  List<int/*!*/>/*!*/ nonNullNonNull;
+  List<int/*?*/>/*!*/ nullNonNull;
+  List<int/*!*/>/*?*/ nonNullNull;
+  nonNullNonNull = hardToNonNullNonNull
+  nonNullNull = hardToNonNullNull
+  nullNonNull = hardToNullNonNull
+}
+''';
+    await _checkSingleFileChanges(content, expected);
+  }
+
+  @failingTest
+  test_downcast_widest_type_from_related_type_parameters() async {
+    var content = '''
+List<int> f(Iterable<int/*?*/> a) => a;
+''';
+
+    // TODO(paulberry): remove the /*!*/, /*?*/ comments on migration.
+    var expected = '''
+List<int?> f(Iterable<int/*?*/> a) => a;
+''';
+    await _checkSingleFileChanges(content, expected);
+  }
+
+  test_downcast_widest_type_from_top_type_parameters() async {
+    var content = '''
+List<int> f1(dynamic a) => a;
+List<int> f2(Object b) => b;
+''';
+    var expected = '''
+List<int?>? f1(dynamic a) => a;
+List<int?> f2(Object b) => b;
+''';
+    await _checkSingleFileChanges(content, expected);
+  }
+
+  @failingTest
+  test_downcast_widest_type_from_unrelated_type_parameters() async {
+    var content = '''
+abstract class C<A, B> implements List<A> {}
+C<int, num> f(List<int> a) => a;
+''';
+
+    // TODO(paulberry): remove the /*!*/, /*?*/ comments on migration.
+    var expected = '''
+abstract class C<A, B> implements List<A> {}
+C<int, num?> f(List<int> a) => a;
+''';
+    await _checkSingleFileChanges(content, expected);
+  }
+
   test_dynamic_method_call() async {
     var content = '''
 class C {

pkg/nnbd_migration/test/edge_builder_test.dart

@@ -189,6 +189,7 @@ class AssignmentCheckerTest extends Object
     assertNoEdge(t.typeArguments[0].node, anyNode);
   }
 
+  @failingTest
   test_generic_to_generic_downcast() {
     var t1 = list(list(object()));
     var t2 = myListOfList(object());
@@ -452,6 +453,7 @@ class C<T extends List<int>> {
     var tType = decoratedTypeAnnotation('T f');
     assertEdge(parameterType.node, tType.node, hard: true);
     assertNoEdge(parameterType.node, boundType.node);
+    // TODO(mfairhurst): Confirm we want this edge.
     assertEdge(
         parameterType.typeArguments[0].node, boundType.typeArguments[0].node,
         hard: false);
@@ -639,7 +641,9 @@ C f(C y, C z) => (y += z);
     assertNullCheck(checkExpression('(y += z)'), fReturnEdge);
   }
 
+  @failingTest
   test_assignmentExpression_compound_withSubstitution() async {
+    // Failing due to a side-cast from incorrectly instantiating the operator.
     var code = '''
 abstract class C<T> {
   C<T> operator+(C<T> x);