Migration: do not use always or never in already-migrated types.

Previously, when the code being migrated referred to a nullable or a
non-nullable type in an already-migrated library, we would use the
`always` or `never` graph node to represent the nullability of the
type.  This was problematic, because it made it difficult for
instrumentation to pinpoint precisely which element in an
already-migrated library was the cause of an expresion being
null-checked, or a type being made non-nullable.

We now create a fresh nullability node for each non-nullable type
coming from already-migrated code, with an edge to `always` or `never`
to ensure that the fresh node has the correct nullability, so that
instrumentation can see precisely which element it came from.

Since the migration tool doesn't traverse the ASTs of already-migrated
code, we can't report an AST node that caused such an edge, so an
`element` getter has been added EdgeOriginInfo to allow us to report
the element that caused the edge.

As before, types associated with already-migrated code are reported to
instrumentation via InstrumentationListener.externalDecoratedType.  As
a new enhancement, bounds of generic parameters in already-migrated
code are reported to instrumentation via
InstrumentationListener.externalDecoratedTypeParameterBound.

Change-Id: Ided4e96e2920f8d9062688f5a6fda29b9b71dd12
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124000
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Mike Fairhurst <mfairhurst@google.com>

Author: stereotype441

pkg/analysis_server/lib/src/edit/nnbd_migration/instrumentation_listener.dart

@@ -29,6 +29,12 @@ class InstrumentationListener implements NullabilityMigrationInstrumentation {
     data.externalDecoratedType[element] = decoratedType;
   }
 
+  @override
+  void externalDecoratedTypeParameterBound(
+      TypeParameterElement typeParameter, DecoratedTypeInfo decoratedType) {
+    // TODO(paulberry): make use of this information.
+  }
+
   @override
   void fix(SingleNullabilityFix fix, Iterable<FixReasonInfo> reasons) {
     _sourceInfo(fix.source).fixes[fix] =

pkg/nnbd_migration/lib/instrumentation.dart

@@ -8,6 +8,7 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/generated/source.dart';
 import 'package:nnbd_migration/nnbd_migration.dart';
 import 'package:nnbd_migration/nullability_state.dart';
+import 'package:nnbd_migration/src/decorated_type.dart';
 
 /// Information exposed to the migration client about the set of nullability
 /// nodes decorating a type in the program being migrated.
@@ -99,20 +100,32 @@ abstract class EdgeInfo implements FixReasonInfo {
 /// Information exposed to the migration client about the location in source
 /// code that led an edge to be introduced into the nullability graph.
 abstract class EdgeOriginInfo {
+  /// If the proximate cause of the edge being introduced into the graph
+  /// corresponds to the type of an element in an already migrated-library, the
+  /// corresponding element; otherwise `null`.
+  ///
+  /// Note that either [node] or [element] will always be non-null.
+  Element get element;
+
   /// The kind of origin represented by this info.
   EdgeOriginKind get kind;
 
-  /// The AST node that led the edge to be introduced into the nullability
-  /// graph.
+  /// If the proximate cause of the edge being introduced into the graph
+  /// corresponds to an AST node in a source file that is being migrated, the
+  /// corresponding AST node; otherwise `null`.
+  ///
+  /// Note that either [node] or [element] will always be non-null.
   AstNode get node;
 
-  /// The source file that [node] appears in.
+  /// If [node] is non-null, the source file that it appears in.  Otherwise
+  /// `null`.
   Source get source;
 }
 
 /// An enumeration of the various kinds of edge origins created by the migration
 /// engine.
 enum EdgeOriginKind {
+  alreadyMigratedType,
   alwaysNullableType,
   compoundAssignment,
   defaultValue,
@@ -153,6 +166,12 @@ abstract class NullabilityMigrationInstrumentation {
   /// of the element.
   void externalDecoratedType(Element element, DecoratedTypeInfo decoratedType);
 
+  /// Called whenever reference is made to an [typeParameter] outside of the
+  /// code being migrated, to report the nullability nodes associated with the
+  /// bound of the type parameter.
+  void externalDecoratedTypeParameterBound(
+      TypeParameterElement typeParameter, DecoratedTypeInfo decoratedType);
+
   /// Called whenever a fix is decided upon, to report the reasons for the fix.
   void fix(SingleNullabilityFix fix, Iterable<FixReasonInfo> reasons);
 

pkg/nnbd_migration/lib/src/already_migrated_code_decorator.dart

@@ -8,6 +8,7 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:nnbd_migration/src/decorated_type.dart';
+import 'package:nnbd_migration/src/edge_origin.dart';
 import 'package:nnbd_migration/src/nullability_node.dart';
 
 /// This class transforms ordinary [DartType]s into their corresponding
@@ -22,53 +23,61 @@ class AlreadyMigratedCodeDecorator {
 
   /// Transforms [type], which should have come from code that has already been
   /// migrated to NNBD, into the corresponding [DecoratedType].
-  DecoratedType decorate(DartType type) {
+  DecoratedType decorate(DartType type, Element element) {
     if (type.isVoid || type.isDynamic) {
-      return DecoratedType(type, _graph.always);
+      var node = NullabilityNode.forAlreadyMigrated();
+      _graph.makeNullable(node, AlwaysNullableTypeOrigin.forElement(element));
+      return DecoratedType(type, node);
     }
     NullabilityNode node;
     var nullabilitySuffix = (type as TypeImpl).nullabilitySuffix;
     if (nullabilitySuffix == NullabilitySuffix.question) {
-      node = _graph.always;
+      node = NullabilityNode.forAlreadyMigrated();
+      _graph.makeNullable(node, AlreadyMigratedTypeOrigin.forElement(element));
     } else {
       // Currently, all types passed to this method have nullability suffix `star`
       // because (a) we don't yet have a migrated SDK, and (b) we haven't added
       // support to the migrator for analyzing packages that have already been
       // migrated with NNBD enabled.
       // TODO(paulberry): fix this assertion when things change.
       assert(nullabilitySuffix == NullabilitySuffix.star);
-      node = _graph.never;
+      node = NullabilityNode.forAlreadyMigrated();
+      _graph.makeNonNullable(
+          node, AlreadyMigratedTypeOrigin.forElement(element));
     }
     if (type is FunctionType) {
       var typeFormalBounds = type.typeFormals.map((e) {
         var bound = e.bound;
         if (bound == null) {
-          return decorate((_typeProvider.objectType as TypeImpl)
-              .withNullability(NullabilitySuffix.question));
+          return decorate(
+              (_typeProvider.objectType as TypeImpl)
+                  .withNullability(NullabilitySuffix.question),
+              element);
         } else {
-          return decorate(bound);
+          return decorate(bound, element);
         }
       }).toList();
       var positionalParameters = <DecoratedType>[];
       var namedParameters = <String, DecoratedType>{};
       for (var parameter in type.parameters) {
         if (parameter.isPositional) {
-          positionalParameters.add(decorate(parameter.type));
+          positionalParameters.add(decorate(parameter.type, element));
         } else {
-          namedParameters[parameter.name] = decorate(parameter.type);
+          namedParameters[parameter.name] = decorate(parameter.type, element);
         }
       }
       return DecoratedType(type, node,
           typeFormalBounds: typeFormalBounds,
-          returnType: decorate(type.returnType),
+          returnType: decorate(type.returnType, element),
           namedParameters: namedParameters,
           positionalParameters: positionalParameters);
     } else if (type is InterfaceType) {
       var typeParameters = type.element.typeParameters;
       if (typeParameters.isNotEmpty) {
         assert(type.typeArguments.length == typeParameters.length);
-        return DecoratedType(type, node,
-            typeArguments: type.typeArguments.map(decorate).toList());
+        return DecoratedType(type, node, typeArguments: [
+          for (var t in type.typeArguments) decorate(t, element)
+        ]);
       }
       return DecoratedType(type, node);
     } else if (type is TypeParameterType) {
@@ -96,6 +105,6 @@ class AlreadyMigratedCodeDecorator {
       // Add FutureOr<T> as a supertype of Future<T>.
       allSupertypes.add(_typeProvider.futureOrType2(type.typeArguments.single));
     }
-    return allSupertypes.map(decorate);
+    return [for (var t in allSupertypes) decorate(t, class_)];
   }
 }

pkg/nnbd_migration/lib/src/edge_origin.dart

@@ -3,9 +3,33 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/src/generated/source.dart';
 import 'package:nnbd_migration/instrumentation.dart';
 
+/// Edge origin resulting from a type in already-migrated code.
+///
+/// For example, in the Map class in dart:core:
+///   V? operator [](Object key);
+///
+/// this class is used for the edge connecting `always` to the return type of
+/// `operator []`, due to the fact that dart:core has already been migrated and
+/// the type is explicitly nullable.
+///
+/// Note that since a single element can have a complex type, it is likely that
+/// multiple edges will be created with an [AlreadyMigratedTypeOrigin] pointing
+/// to the same type.  To distinguish which edge corresponds to which part of
+/// the element's type, use the callbacks
+/// [NullabilityMigrationInstrumentation.externalDecoratedType] and
+/// [NullabilityMigrationInstrumentation.externalDecoratedTypeParameterBound].
+class AlreadyMigratedTypeOrigin extends EdgeOrigin {
+  AlreadyMigratedTypeOrigin.forElement(Element element)
+      : super.forElement(element);
+
+  @override
+  EdgeOriginKind get kind => EdgeOriginKind.alreadyMigratedType;
+}
+
 /// Edge origin resulting from the use of a type that is always nullable.
 ///
 /// For example, in the following code snippet:
@@ -17,6 +41,9 @@ import 'package:nnbd_migration/instrumentation.dart';
 class AlwaysNullableTypeOrigin extends EdgeOrigin {
   AlwaysNullableTypeOrigin(Source source, AstNode node) : super(source, node);
 
+  AlwaysNullableTypeOrigin.forElement(Element element)
+      : super.forElement(element);
+
   @override
   EdgeOriginKind get kind => EdgeOriginKind.alwaysNullableType;
 }
@@ -53,7 +80,14 @@ abstract class EdgeOrigin extends EdgeOriginInfo {
   @override
   final AstNode node;
 
-  EdgeOrigin(this.source, this.node);
+  @override
+  final Element element;
+
+  EdgeOrigin(this.source, this.node) : element = null;
+
+  EdgeOrigin.forElement(this.element)
+      : source = null,
+        node = null;
 }
 
 /// Edge origin resulting from the relationship between a field formal parameter

pkg/nnbd_migration/lib/src/nullability_node.dart

@@ -158,6 +158,19 @@ class NullabilityGraph {
     return _sourcesBeingMigrated.contains(source);
   }
 
+  /// Creates a graph edge that will try to force the given [node] to be
+  /// non-nullable.
+  void makeNonNullable(NullabilityNode node, EdgeOrigin origin) {
+    connect(node, never, origin, hard: true);
+  }
+
+  /// Creates a graph edge that will try to force the given [node] to be
+  /// nullable.
+  void makeNullable(NullabilityNode node, EdgeOrigin origin,
+      {List<NullabilityNode> guards: const []}) {
+    connect(always, node, origin, guards: guards);
+  }
+
   /// Record source as code that is being migrated.
   void migrating(Source source) {
     _sourcesBeingMigrated.add(source);
@@ -447,6 +460,11 @@ abstract class NullabilityNode implements NullabilityNodeInfo {
   /// List of edges that have this node as their destination.
   final _upstreamEdges = <NullabilityEdge>[];
 
+  /// Creates a [NullabilityNode] representing the nullability of a variable
+  /// whose type comes from an already-migrated library.
+  factory NullabilityNode.forAlreadyMigrated() =>
+      _NullabilityNodeSimple('migrated');
+
   /// Creates a [NullabilityNode] representing the nullability of an expression
   /// which is nullable iff two other nullability nodes are both nullable.
   ///
@@ -494,6 +512,8 @@ abstract class NullabilityNode implements NullabilityNodeInfo {
   /// annotate the nullability of that type.
   String get debugSuffix => '?($this)';
 
+  Iterable<EdgeInfo> get downstreamEdges => _downstreamEdges;
+
   /// After nullability propagation, this getter can be used to query whether
   /// the type associated with this node should be considered "exact nullable".
   @visibleForTesting

pkg/nnbd_migration/lib/src/variables.dart

@@ -92,8 +92,10 @@ class Variables implements VariableRecorder, VariableRepository {
               'have been stored by the NodeBuilder via '
               'recordTypeParameterBound');
         }
-        decoratedType = _alreadyMigratedCodeDecorator
-            .decorate(typeParameter.bound ?? DynamicTypeImpl.instance);
+        decoratedType = _alreadyMigratedCodeDecorator.decorate(
+            typeParameter.bound ?? DynamicTypeImpl.instance, typeParameter);
+        instrumentation?.externalDecoratedTypeParameterBound(
+            typeParameter, decoratedType);
         _decoratedTypeParameterBounds[typeParameter] = decoratedType;
       }
       return decoratedType;
@@ -181,9 +183,11 @@ class Variables implements VariableRecorder, VariableRepository {
 
     DecoratedType decoratedType;
     if (element is FunctionTypedElement) {
-      decoratedType = _alreadyMigratedCodeDecorator.decorate(element.type);
+      decoratedType =
+          _alreadyMigratedCodeDecorator.decorate(element.type, element);
     } else if (element is VariableElement) {
-      decoratedType = _alreadyMigratedCodeDecorator.decorate(element.type);
+      decoratedType =
+          _alreadyMigratedCodeDecorator.decorate(element.type, element);
     } else {
       // TODO(paulberry)
       throw UnimplementedError('Decorating ${element.runtimeType}');
@@ -199,7 +203,6 @@ class Variables implements VariableRecorder, VariableRepository {
     var result = <ClassElement, DecoratedType>{};
     for (var decoratedSupertype
         in _alreadyMigratedCodeDecorator.getImmediateSupertypes(class_)) {
-      assert(identical(decoratedSupertype.node, _graph.never));
       var class_ = (decoratedSupertype.type as InterfaceType).element;
       result[class_] = decoratedSupertype;
     }