Migration: Clean up substitution logic in getOrComputeElementType

stereotype441 · commit-bot@chromium.org · commit 66e349da9c8d · 2019-08-28T22:12:51.000Z
This should address ~267 exceptions whose stacktrace contains the line: EdgeBuilder.getOrComputeElementType (package:nnbd_migration/src/edge_builder.dart:197:16) Change-Id: If377190e0bdabe85f541af50cb5f896fde5481ee Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114741 Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart
@@ -186,25 +186,14 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
   DecoratedType getOrComputeElementType(Element element,
       {DecoratedType targetType}) {
     Map<TypeParameterElement, DecoratedType> substitution;
-    Element baseElement;
-    if (element is Member) {
-      assert(targetType != null);
-      baseElement = element.baseElement;
-      var targetTypeType = targetType.type;
-      if (targetTypeType is InterfaceType &&
-          baseElement is ClassMemberElement) {
-        var enclosingClass = baseElement.enclosingElement as ClassElement;
-        assert(targetTypeType.element == enclosingClass); // TODO(paulberry)
-        substitution = <TypeParameterElement, DecoratedType>{};
-        assert(enclosingClass.typeParameters.length ==
-            targetTypeType.typeArguments.length); // TODO(paulberry)
-        for (int i = 0; i < enclosingClass.typeParameters.length; i++) {
-          substitution[enclosingClass.typeParameters[i]] =
-              targetType.typeArguments[i];
-        }
+    Element baseElement = element is Member ? element.baseElement : element;
+    if (targetType != null) {
+      var classElement = baseElement.enclosingElement as ClassElement;
+      if (classElement.typeParameters.isNotEmpty) {
+        substitution = _decoratedClassHierarchy
+            .asInstanceOf(targetType, classElement)
+            .asSubstitution;
       }
-    } else {
-      baseElement = element;
     }
     DecoratedType decoratedBaseType;
     if (baseElement is PropertyAccessorElement &&
@@ -230,6 +219,8 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
         elementType = element.type;
       } else if (element is ConstructorElement) {
         elementType = element.type;
+      } else if (element is PropertyAccessorMember) {
+        elementType = element.type;
       } else {
         throw element.runtimeType; // TODO(paulberry)
       }
diff --git a/pkg/nnbd_migration/lib/src/nullability_node.dart b/pkg/nnbd_migration/lib/src/nullability_node.dart
@@ -346,6 +346,12 @@ class NullabilityGraph {
 class NullabilityGraphForTesting extends NullabilityGraph {
   final List<NullabilityEdge> _allEdges = [];
 
+  /// Prints out a representation of the graph nodes.  Useful in debugging
+  /// broken tests.
+  void debugDump() {
+    _debugDump();
+  }
+
   /// Iterates through all edges in the graph.
   @visibleForTesting
   Iterable<NullabilityEdge> getAllEdges() {
diff --git a/pkg/nnbd_migration/test/edge_builder_test.dart b/pkg/nnbd_migration/test/edge_builder_test.dart
@@ -2468,6 +2468,23 @@ void test(C c) {
     assertEdge(decoratedTypeAnnotation('C c').node, never, hard: true);
   }
 
+  test_methodInvocation_target_generic_in_base_class() async {
+    await analyze('''
+abstract class B<T> {
+  void m(T/*1*/ t);
+}
+abstract class C extends B<int/*2*/> {}
+void f(C c, int/*3*/ i) {
+  c.m(i);
+}
+''');
+    // nullable(3) -> substitute(nullable(2), nullable(1))
+    var nullable1 = decoratedTypeAnnotation('T/*1*/').node;
+    var nullable2 = decoratedTypeAnnotation('int/*2*/').node;
+    var nullable3 = decoratedTypeAnnotation('int/*3*/').node;
+    assertEdge(nullable3, substitutionNode(nullable2, nullable1), hard: true);
+  }
+
   test_methodInvocation_typeParameter_inferred() async {
     await analyze('''
 T f<T>(T t) => t;
@@ -3324,6 +3341,24 @@ bool f(C c) => c.b;
         hard: false);
   }
 
+  test_prefixedIdentifier_getter_type_in_generic() async {
+    await analyze('''
+class C<T> {
+  List<T> _x;
+  List<T> get x => _x;
+}
+List<int> f(C<int> c) => c.x;
+''');
+    assertEdge(decoratedTypeAnnotation('List<T> get').node,
+        decoratedTypeAnnotation('List<int> f').node,
+        hard: false);
+    assertEdge(
+        substitutionNode(decoratedTypeAnnotation('int> c').node,
+            decoratedTypeAnnotation('T> get').node),
+        decoratedTypeAnnotation('int> f').node,
+        hard: false);
+  }
+
   test_prefixedIdentifier_target_check() async {
     await analyze('''
 class C {
