Migration: store decorated type parameter bounds separately from other element decorated types.

stereotype441 · commit-bot@chromium.org · commit 1f7e6370b46b · 2019-08-12T17:56:45.000Z
We will need some special treatment for decorated type parameter bounds, because sometimes we will need to create type parameters (e.g. when comparing decorated types). So we store the bounds of non-ephemeral type parameters (the ones whose enclosing element is non-null) in the Variables data structure, and we store the bounds of ephemeral type parameters (the ones whose enclosing element is null) in an expando. Change-Id: I918c9baffedab9a8871ecea37c8101b5ef44a5d9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112743 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/nnbd_migration/lib/src/decorated_type.dart b/pkg/nnbd_migration/lib/src/decorated_type.dart
@@ -13,6 +13,14 @@ import 'package:nnbd_migration/src/potential_modification.dart';
 /// tracking the (unmigrated) [DartType], we track the [ConstraintVariable]s
 /// indicating whether the type, and the types that compose it, are nullable.
 class DecoratedType {
+  /// Mapping from type parameter elements to the decorated types of those type
+  /// parameters' bounds.
+  ///
+  /// This expando only applies to type parameters whose enclosing element is
+  /// `null`.  Type parameters whose enclosing element is not `null` should be
+  /// stored in [Variables._decoratedTypeParameterBounds].
+  static final _decoratedTypeParameterBounds = Expando<DecoratedType>();
+
   final DartType type;
 
   final NullabilityNode node;
@@ -302,6 +310,28 @@ class DecoratedType {
             returnType._substitute(substitution, undecoratedResult.returnType),
         positionalParameters: newPositionalParameters);
   }
+
+  /// Retrieves the decorated bound of the given [typeParameter].
+  ///
+  /// [typeParameter] must have an enclosing element of `null`.  Type parameters
+  /// whose enclosing element is not `null` are tracked by the [Variables]
+  /// class.
+  static DecoratedType decoratedTypeParameterBound(
+      TypeParameterElement typeParameter) {
+    assert(typeParameter.enclosingElement == null);
+    return _decoratedTypeParameterBounds[typeParameter];
+  }
+
+  /// Stores he decorated bound of the given [typeParameter].
+  ///
+  /// [typeParameter] must have an enclosing element of `null`.  Type parameters
+  /// whose enclosing element is not `null` are tracked by the [Variables]
+  /// class.
+  static void recordTypeParameterBound(
+      TypeParameterElement typeParameter, DecoratedType bound) {
+    assert(typeParameter.enclosingElement == null);
+    _decoratedTypeParameterBounds[typeParameter] = bound;
+  }
 }
 
 /// A [DecoratedType] based on a type annotation appearing explicitly in the
diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart
@@ -1005,13 +1005,15 @@ $stackTrace''');
         for (int i = 0; i < instantiatedType.typeArguments.length; i++) {
           _unionDecoratedTypes(
               instantiatedType.typeArguments[i],
-              _variables.decoratedElementType(element.typeParameters[i]),
+              _variables.decoratedTypeParameterBound(element.typeParameters[i]),
               origin);
         }
       } else {
         for (int i = 0; i < typeArguments.length; i++) {
           DecoratedType bound;
-          bound = _variables.decoratedElementType(element.typeParameters[i]);
+          bound =
+              _variables.decoratedTypeParameterBound(element.typeParameters[i]);
+          assert(bound != null);
           var argumentType =
               _variables.decoratedTypeAnnotation(_source, typeArguments[i]);
           if (argumentType == null) {
@@ -1188,7 +1190,7 @@ $stackTrace''');
     // TODO(paulberry): once we've wired up flow analysis, return promoted
     // bounds if applicable.
     return _variables
-        .decoratedElementType((type.type as TypeParameterType).element);
+        .decoratedTypeParameterBound((type.type as TypeParameterType).element);
   }
 
   /// Creates the necessary constraint(s) for an assignment of the given
diff --git a/pkg/nnbd_migration/lib/src/node_builder.dart b/pkg/nnbd_migration/lib/src/node_builder.dart
@@ -346,7 +346,7 @@ $stackTrace''');
           AlwaysNullableTypeOrigin(_source, node.offset));
       decoratedBound = DecoratedType(_typeProvider.objectType, nullabilityNode);
     }
-    _variables.recordDecoratedElementType(element, decoratedBound);
+    _variables.recordDecoratedTypeParameterBound(element, decoratedBound);
     return null;
   }
 
@@ -548,6 +548,10 @@ abstract class VariableRecorder {
       Source source, TypeAnnotation node, DecoratedTypeAnnotation type,
       {bool potentialModification: true});
 
+  /// Stores he decorated bound of the given [typeParameter].
+  void recordDecoratedTypeParameterBound(
+      TypeParameterElement typeParameter, DecoratedType bound);
+
   /// Records that [node] is associated with the question of whether the named
   /// [parameter] should be optional (should not have a `required`
   /// annotation added to it).
@@ -580,6 +584,9 @@ abstract class VariableRepository {
   DecoratedType decoratedTypeAnnotation(
       Source source, TypeAnnotation typeAnnotation);
 
+  /// Retrieves the decorated bound of the given [typeParameter].
+  DecoratedType decoratedTypeParameterBound(TypeParameterElement typeParameter);
+
   /// Records conditional discard information for the given AST node (which is
   /// an `if` statement or a conditional (`?:`) expression).
   void recordConditionalDiscard(
diff --git a/pkg/nnbd_migration/lib/src/variables.dart b/pkg/nnbd_migration/lib/src/variables.dart
@@ -20,6 +20,8 @@ class Variables implements VariableRecorder, VariableRepository {
 
   final _decoratedElementTypes = <Element, DecoratedType>{};
 
+  final _decoratedTypeParameterBounds = <Element, DecoratedType>{};
+
   final _decoratedDirectSupertypes =
       <ClassElement, Map<ClassElement, DecoratedType>>{};
 
@@ -42,8 +44,12 @@ class Variables implements VariableRecorder, VariableRepository {
   }
 
   @override
-  DecoratedType decoratedElementType(Element element) =>
-      _decoratedElementTypes[element] ??= _createDecoratedElementType(element);
+  DecoratedType decoratedElementType(Element element) {
+    assert(element is! TypeParameterElement,
+        'Use decoratedTypeParameterBound instead');
+    return _decoratedElementTypes[element] ??=
+        _createDecoratedElementType(element);
+  }
 
   @override
   DecoratedType decoratedTypeAnnotation(
@@ -62,6 +68,35 @@ class Variables implements VariableRecorder, VariableRepository {
     return decoratedTypeAnnotation;
   }
 
+  @override
+  DecoratedType decoratedTypeParameterBound(
+      TypeParameterElement typeParameter) {
+    if (typeParameter.enclosingElement == null) {
+      var decoratedType =
+          DecoratedType.decoratedTypeParameterBound(typeParameter);
+      if (decoratedType == null) {
+        throw StateError(
+            'A decorated type for the bound of $typeParameter should '
+            'have been stored by the NodeBuilder via recordTypeParameterBound');
+      }
+      return decoratedType;
+    } else {
+      var decoratedType = _decoratedTypeParameterBounds[typeParameter];
+      if (decoratedType == null) {
+        if (_graph.isBeingMigrated(typeParameter.library.source)) {
+          throw StateError(
+              'A decorated type for the bound of $typeParameter should '
+              'have been stored by the NodeBuilder via '
+              'recordTypeParameterBound');
+        }
+        decoratedType = _alreadyMigratedCodeDecorator
+            .decorate(typeParameter.bound ?? DynamicTypeImpl.instance);
+        _decoratedTypeParameterBounds[typeParameter] = decoratedType;
+      }
+      return decoratedType;
+    }
+  }
+
   Map<Source, List<PotentialModification>> getPotentialModifications() =>
       _potentialModifications;
 
@@ -87,6 +122,8 @@ class Variables implements VariableRecorder, VariableRepository {
 
   void recordDecoratedElementType(Element element, DecoratedType type) {
     assert(() {
+      assert(element is! TypeParameterElement,
+          'Use recordDecoratedTypeParameterBound instead');
       var library = element.library;
       if (library == null) {
         // No problem; the element is probably a parameter of a function type
@@ -109,6 +146,16 @@ class Variables implements VariableRecorder, VariableRepository {
         {})[_uniqueOffsetForTypeAnnotation(node)] = type;
   }
 
+  @override
+  void recordDecoratedTypeParameterBound(
+      TypeParameterElement typeParameter, DecoratedType bound) {
+    if (typeParameter.enclosingElement == null) {
+      DecoratedType.recordTypeParameterBound(typeParameter, bound);
+    } else {
+      _decoratedTypeParameterBounds[typeParameter] = bound;
+    }
+  }
+
   @override
   void recordExpressionChecks(
       Source source, Expression expression, ExpressionChecks checks) {
@@ -187,11 +234,6 @@ class Variables implements VariableRecorder, VariableRepository {
       decoratedType = _alreadyMigratedCodeDecorator.decorate(element.type);
     } else if (element is TopLevelVariableElement) {
       decoratedType = _alreadyMigratedCodeDecorator.decorate(element.type);
-    } else if (element is TypeParameterElement) {
-      // By convention, type parameter elements are decorated with the type of
-      // their bounds.
-      decoratedType = _alreadyMigratedCodeDecorator
-          .decorate(element.bound ?? DynamicTypeImpl.instance);
     } else {
       // TODO(paulberry)
       throw UnimplementedError('Decorating ${element.runtimeType}');
diff --git a/pkg/nnbd_migration/test/edge_builder_test.dart b/pkg/nnbd_migration/test/edge_builder_test.dart
@@ -3626,7 +3626,7 @@ void f(Point<int> x) {}
     var pointClass =
         findNode.typeName('Point').name.staticElement as ClassElement;
     var pointBound =
-        variables.decoratedElementType(pointClass.typeParameters[0]);
+        variables.decoratedTypeParameterBound(pointClass.typeParameters[0]);
     expect(pointBound.type.toString(), 'num');
     assertEdge(decoratedTypeAnnotation('int>').node, pointBound.node,
         hard: true);
@@ -3637,7 +3637,8 @@ void f(Point<int> x) {}
 void f(List<int> x) {}
 ''');
     var listClass = typeProvider.listType.element;
-    var listBound = variables.decoratedElementType(listClass.typeParameters[0]);
+    var listBound =
+        variables.decoratedTypeParameterBound(listClass.typeParameters[0]);
     expect(listBound.type.toString(), 'dynamic');
     assertEdge(decoratedTypeAnnotation('int>').node, listBound.node,
         hard: true);
diff --git a/pkg/nnbd_migration/test/node_builder_test.dart b/pkg/nnbd_migration/test/node_builder_test.dart
@@ -23,8 +23,9 @@ class NodeBuilderTest extends MigrationVisitorTestBase {
       variables.decoratedElementType(
           findNode.functionDeclaration(search).declaredElement);
 
-  DecoratedType decoratedTypeParameterBound(String search) => variables
-      .decoratedElementType(findNode.typeParameter(search).declaredElement);
+  DecoratedType decoratedTypeParameterBound(String search) =>
+      variables.decoratedTypeParameterBound(
+          findNode.typeParameter(search).declaredElement);
 
   test_class_alias_synthetic_constructors_no_parameters() async {
     await analyze('''
