From 3c34c8fa2fbca93bf2f3dc4451be60f03e1fb14d Mon Sep 17 00:00:00 2001
From: Paul Berry <paulberry@google.com>
Date: Fri, 24 May 2019 15:43:45 +0000
Subject: [PATCH] Fix type substitution of "star" types.

When performing type substitution, there are three types involved, the
substitution site (`this` in TypeImpl.substitute2), the type we are
replacing (`parameterType` in TypeImpl.substitute2), and the
replacement type (`argumentType` in TypeImpl.substitute2).

Previously, we only handled the nullabilities of `this` and
`argumentType`.  This CL adds support for the possibility that
`parameterType` might have nullability "*".  This happens because
TypeParameterElement.type returns a star type, so for instance when
subsituting `int` for `T` in `List<T>` to form `List<int>`, we are
actually substituting `int` for `T*` in `List<T*>`.

Change-Id: I0a61fdc47ec8aa205dc0c539c49ea7799ed4ac05
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103542
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
---
 pkg/analyzer/lib/src/dart/element/type.dart   | 31 ++++++++++++++-----
 ...efault_list_constructor_mismatch_test.dart | 13 --------
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/pkg/analyzer/lib/src/dart/element/type.dart b/pkg/analyzer/lib/src/dart/element/type.dart
index 12550dca383..f8e0113095c 100644
--- a/pkg/analyzer/lib/src/dart/element/type.dart
+++ b/pkg/analyzer/lib/src/dart/element/type.dart
@@ -3244,7 +3244,8 @@ class TypeParameterTypeImpl extends TypeImpl implements TypeParameterType {
       [List<FunctionTypeAliasElement> prune]) {
     int length = parameterTypes.length;
     for (int i = 0; i < length; i++) {
-      if (parameterTypes[i] == this) {
+      var parameterType = parameterTypes[i];
+      if (parameterType is TypeParameterTypeImpl && parameterType == this) {
         TypeImpl argumentType = argumentTypes[i];
 
         // TODO(scheglov) It should not happen, but sometimes arguments are null.
@@ -3254,15 +3255,29 @@ class TypeParameterTypeImpl extends TypeImpl implements TypeParameterType {
 
         // TODO(scheglov) Proposed substitution rules for nullability.
         NullabilitySuffix resultNullability;
+        NullabilitySuffix parameterNullability =
+            parameterType.nullabilitySuffix;
         NullabilitySuffix argumentNullability = argumentType.nullabilitySuffix;
-        if (argumentNullability == NullabilitySuffix.question ||
-            nullabilitySuffix == NullabilitySuffix.question) {
-          resultNullability = NullabilitySuffix.question;
-        } else if (argumentNullability == NullabilitySuffix.star ||
-            nullabilitySuffix == NullabilitySuffix.star) {
-          resultNullability = NullabilitySuffix.star;
+        if (parameterNullability == NullabilitySuffix.none) {
+          if (argumentNullability == NullabilitySuffix.question ||
+              nullabilitySuffix == NullabilitySuffix.question) {
+            resultNullability = NullabilitySuffix.question;
+          } else if (argumentNullability == NullabilitySuffix.star ||
+              nullabilitySuffix == NullabilitySuffix.star) {
+            resultNullability = NullabilitySuffix.star;
+          } else {
+            resultNullability = NullabilitySuffix.none;
+          }
+        } else if (parameterNullability == NullabilitySuffix.star) {
+          if (argumentNullability == NullabilitySuffix.question ||
+              nullabilitySuffix == NullabilitySuffix.question) {
+            resultNullability = NullabilitySuffix.question;
+          } else {
+            resultNullability = argumentNullability;
+          }
         } else {
-          resultNullability = NullabilitySuffix.none;
+          // We should never be substituting for `T?`.
+          throw new StateError('Tried to substitute for T?');
         }
 
         return argumentType.withNullability(resultNullability);
diff --git a/pkg/analyzer/test/src/diagnostics/default_list_constructor_mismatch_test.dart b/pkg/analyzer/test/src/diagnostics/default_list_constructor_mismatch_test.dart
index 17691707dfd..30e92679b76 100644
--- a/pkg/analyzer/test/src/diagnostics/default_list_constructor_mismatch_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/default_list_constructor_mismatch_test.dart
@@ -36,10 +36,7 @@ var l = new List<String?>(3);
 ''');
   }
 
-  @failingTest
   test_inferredType() async {
-    // This test is failing because summary support is incomplete, which results
-    // in the constructor having a type of 'List<C*>*'.
     await assertErrorsInCode('''
 class C {}
 List<C> v = List(5);
@@ -48,16 +45,6 @@ List<C> v = List(5);
     ]);
   }
 
-  test_starType() async {
-    // TODO(brianwilkerson) This test is currently taking advantage of the fact
-    //  that the SDK is not opted in, which makes the use of `int` below a
-    //  reference to 'int*'. When it's possible to opt-out in a test this needs
-    //  to be updated to use an explicitly opted out type.
-    await assertNoErrorsInCode('''
-List<int> v = List(5);
-''');
-  }
-
   test_typeParameter() async {
     await assertErrorsInCode('''
 class C<T> {