Stop using NullabilitySuffix in fe/analyzer shared code.

The getter `SharedType.nullabilitySuffix` is replaced by
`SharedType.isQuestionType`, which returns a boolean.

The method `TypeAnalyzerOperations.withNullabilitySuffixInternal` is
replaced by `SharedType.setNullabilitySuffix`, which accepts a
boolean.

Support for `*` types has been removed from `mini_types.dart`.

A few test cases in `flow_analysis_test.dart` previously used `*`
types as a way of exercising corner cases involving types that were
mutual subtypes of each other. These tests have been changed to take
advantage of the fact that `dynamic` and `Object?` are mutual
subtypes.

Change-Id: Id9904f9570fc738b388192db8536848204af03e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/414581
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Paul Berry <[email protected]>

Author: stereotype441

pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer_operations.dart

@@ -4,7 +4,6 @@
 
 import '../flow_analysis/flow_analysis_operations.dart';
 import '../types/shared_type.dart';
-import 'nullability_suffix.dart';
 import 'type_constraint.dart';
 
 /// Callback API used by the shared type analyzer to query and manipulate the
@@ -692,10 +691,6 @@ abstract interface class TypeAnalyzerOperations<Variable extends Object,
 
   /// Converts a type into a corresponding type schema.
   SharedTypeSchemaView typeToSchema(SharedTypeView type);
-
-  /// Returns [type] suffixed with the [suffix].
-  SharedType withNullabilitySuffixInternal(
-      covariant SharedType type, NullabilitySuffix suffix);
 }
 
 mixin TypeAnalyzerOperationsMixin<Variable extends Object,
@@ -965,7 +960,7 @@ abstract class TypeConstraintGenerator<
   /// be rolled back to a state via [restoreState].
   TypeConstraintGeneratorState get currentState;
 
-  /// True if FutureOr types are required to have the empty [NullabilitySuffix]
+  /// True if FutureOr types are required to have `isQuestionType == false`
   /// when they are matched.
   ///
   /// For more information about the discrepancy between the Analyzer and the
@@ -1073,9 +1068,8 @@ abstract class TypeConstraintGenerator<
       SharedType p, SharedType q,
       {required bool leftSchema, required AstNode? astNodeForTesting}) {
     // If `P` is `FutureOr<P0>` the match holds under constraint set `C1 + C2`:
-    NullabilitySuffix pNullability = p.nullabilitySuffix;
     if (typeAnalyzerOperations.matchFutureOrInternal(p) case var p0?
-        when pNullability == NullabilitySuffix.none) {
+        when !p.isQuestionType) {
       final TypeConstraintGeneratorState state = currentState;
 
       // If `Future<P0>` is a subtype match for `Q` under constraint set `C1`.
@@ -1110,10 +1104,8 @@ abstract class TypeConstraintGenerator<
       SharedType p, SharedType q,
       {required bool leftSchema, required AstNode? astNodeForTesting}) {
     // If `P` is `P0?` the match holds under constraint set `C1 + C2`:
-    NullabilitySuffix pNullability = p.nullabilitySuffix;
-    if (pNullability == NullabilitySuffix.question) {
-      SharedType p0 = typeAnalyzerOperations.withNullabilitySuffixInternal(
-          p, NullabilitySuffix.none);
+    if (p.isQuestionType) {
+      SharedType p0 = p.asQuestionType(false);
       final TypeConstraintGeneratorState state = currentState;
 
       // If `P0` is a subtype match for `Q` under constraint set `C1`.
@@ -1204,14 +1196,14 @@ abstract class TypeConstraintGenerator<
     // If `Q` is `FutureOr<Q0>` the match holds under constraint set `C`:
     if (typeAnalyzerOperations.matchFutureOrInternal(q) case SharedType q0?
         when enableDiscrepantObliviousnessOfNullabilitySuffixOfFutureOr ||
-            q.nullabilitySuffix == NullabilitySuffix.none) {
+            !q.isQuestionType) {
       final TypeConstraintGeneratorState state = currentState;
 
       // If `P` is `FutureOr<P0>` and `P0` is a subtype match for `Q0` under
       // constraint set `C`.
       if (typeAnalyzerOperations.matchFutureOrInternal(p) case SharedType p0?
           when enableDiscrepantObliviousnessOfNullabilitySuffixOfFutureOr ||
-              p.nullabilitySuffix == NullabilitySuffix.none) {
+              !p.isQuestionType) {
         if (performSubtypeConstraintGenerationInternal(p0, q0,
             leftSchema: leftSchema, astNodeForTesting: astNodeForTesting)) {
           return true;
@@ -1260,18 +1252,14 @@ abstract class TypeConstraintGenerator<
       SharedType p, SharedType q,
       {required bool leftSchema, required AstNode? astNodeForTesting}) {
     // If `Q` is `Q0?` the match holds under constraint set `C`:
-    NullabilitySuffix qNullability = q.nullabilitySuffix;
-    if (qNullability == NullabilitySuffix.question) {
-      SharedType q0 = typeAnalyzerOperations.withNullabilitySuffixInternal(
-          q, NullabilitySuffix.none);
+    if (q.isQuestionType) {
+      SharedType q0 = q.asQuestionType(false);
       final TypeConstraintGeneratorState state = currentState;
 
       // If `P` is `P0?` and `P0` is a subtype match for `Q0` under
       // constraint set `C`.
-      NullabilitySuffix pNullability = p.nullabilitySuffix;
-      if (pNullability == NullabilitySuffix.question) {
-        SharedType p0 = typeAnalyzerOperations.withNullabilitySuffixInternal(
-            p, NullabilitySuffix.none);
+      if (p.isQuestionType) {
+        SharedType p0 = p.asQuestionType(false);
         if (performSubtypeConstraintGenerationInternal(p0, q0,
             leftSchema: leftSchema, astNodeForTesting: astNodeForTesting)) {
           return true;
@@ -1409,10 +1397,9 @@ abstract class TypeConstraintGenerator<
 
     // If `P` is a type variable `X` in `L`, then the match holds:
     //   Under constraint `_ <: X <: Q`.
-    NullabilitySuffix pNullability = p.nullabilitySuffix;
     if (typeAnalyzerOperations.matchInferableParameterInternal(p)
         case var pParameter?
-        when pNullability == NullabilitySuffix.none &&
+        when !p.isQuestionType &&
             typeParametersToConstrain.contains(pParameter)) {
       addUpperConstraintForParameter(pParameter, q,
           astNodeForTesting: astNodeForTesting);
@@ -1421,10 +1408,9 @@ abstract class TypeConstraintGenerator<
 
     // If `Q` is a type variable `X` in `L`, then the match holds:
     //   Under constraint `P <: X <: _`.
-    NullabilitySuffix qNullability = q.nullabilitySuffix;
     if (typeAnalyzerOperations.matchInferableParameterInternal(q)
         case var qParameter?
-        when qNullability == NullabilitySuffix.none &&
+        when !q.isQuestionType &&
             typeParametersToConstrain.contains(qParameter) &&
             (!inferenceUsingBoundsIsEnabled ||
                 (qParameter.boundShared == null ||
@@ -1832,11 +1818,11 @@ abstract class TypeConstraintGenerator<
   /// and `false` is returned.
   bool _interfaceTypes(SharedType p, SharedType q, bool leftSchema,
       {required AstNode? astNodeForTesting}) {
-    if (p.nullabilitySuffix != NullabilitySuffix.none) {
+    if (p.isQuestionType) {
       return false;
     }
 
-    if (q.nullabilitySuffix != NullabilitySuffix.none) {
+    if (q.isQuestionType) {
       return false;
     }
 

pkg/_fe_analyzer_shared/lib/src/types/shared_type.dart

@@ -2,8 +2,6 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import '../type_inference/nullability_suffix.dart';
-
 /// Common interface for data structures used by the implementations to
 /// represent the type `dynamic`.
 abstract interface class SharedDynamicType implements SharedType {}
@@ -73,9 +71,19 @@ abstract interface class SharedRecordType implements SharedType {
 /// Common interface for data structures used by the implementations to
 /// represent a type.
 abstract interface class SharedType {
-  /// If this type ends in a suffix (`?` or `*`), the suffix it ends with;
-  /// otherwise [NullabilitySuffix.none].
-  NullabilitySuffix get nullabilitySuffix;
+  /// Whether this type ends in a `?` suffix.
+  ///
+  /// Note that some types are nullable even though they do not end in a `?`
+  /// suffix (for example, `Null`, `dynamic`, and `FutureOr<int?>`). These types
+  /// all respond to this query with `false`.
+  bool get isQuestionType;
+
+  /// Returns a modified version of this type, with the nullability suffix
+  /// changed to [isQuestionType].
+  ///
+  /// For types that don't accept a nullability suffix (`dynamic`, InvalidType,
+  /// `Null`, `_`, and `void`), the type is returned unchanged.
+  SharedType asQuestionType(bool isQuestionType);
 
   /// Return the presentation of this type as it should appear when presented
   /// to users in contexts such as error messages.
@@ -163,7 +171,7 @@ extension type SharedTypeParameterView(SharedTypeParameter _typeParameter)
 
 extension type SharedTypeSchemaView(SharedType _typeStructure)
     implements Object {
-  NullabilitySuffix get nullabilitySuffix => _typeStructure.nullabilitySuffix;
+  bool get isQuestionType => _typeStructure.isQuestionType;
 
   String getDisplayString() => _typeStructure.getDisplayString();
 
@@ -175,7 +183,7 @@ extension type SharedTypeSchemaView(SharedType _typeStructure)
 }
 
 extension type SharedTypeView(SharedType _typeStructure) implements Object {
-  NullabilitySuffix get nullabilitySuffix => _typeStructure.nullabilitySuffix;
+  bool get isQuestionType => _typeStructure.isQuestionType;
 
   String getDisplayString() => _typeStructure.getDisplayString();
 

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

@@ -4207,27 +4207,29 @@ main() {
           test('; no promotion', () {
             var s1 = FlowModel<SharedTypeView>(Reachability.initial)
                 ._declare(h, objectQVar, true)
-                ._tryPromoteForTypeCheck(h, objectQVar, 'num?')
+                ._tryPromoteForTypeCheck(h, objectQVar, 'List<Object?>')
                 .ifFalse
-                ._tryPromoteForTypeCheck(h, objectQVar, 'num*')
+                ._tryPromoteForTypeCheck(h, objectQVar, 'List<dynamic>')
                 .ifFalse;
             expect(s1.promotionInfo.unwrap(h), {
               h.promotionKeyStore.keyForVariable(objectQVar):
                   _matchVariableModel(
-                chain: ['Object'],
-                ofInterest: ['num?', 'num*'],
+                ofInterest: ['List<Object?>', 'List<dynamic>'],
               ),
             });
-            var s2 = s1._write(h, null, objectQVar, SharedTypeView(Type('int')),
+            var s2 = s1._write(
+                h,
+                null,
+                objectQVar,
+                SharedTypeView(Type('List<int>')),
                 new SsaNode<SharedTypeView>(null));
-            // It's ambiguous whether to promote to num? or num*, so we don't
-            // promote.
+            // It's ambiguous whether to promote to List<Object?> or
+            // List<dynamic>, so we don't promote.
             expect(s2, isNot(same(s1)));
             expect(s2.promotionInfo.unwrap(h), {
               h.promotionKeyStore.keyForVariable(objectQVar):
                   _matchVariableModel(
-                chain: ['Object'],
-                ofInterest: ['num?', 'num*'],
+                ofInterest: ['List<Object?>', 'List<dynamic>'],
               ),
             });
           });
@@ -4236,24 +4238,28 @@ main() {
         test('exact match', () {
           var s1 = FlowModel<SharedTypeView>(Reachability.initial)
               ._declare(h, objectQVar, true)
-              ._tryPromoteForTypeCheck(h, objectQVar, 'num?')
+              ._tryPromoteForTypeCheck(h, objectQVar, 'List<Object?>')
               .ifFalse
-              ._tryPromoteForTypeCheck(h, objectQVar, 'num*')
+              ._tryPromoteForTypeCheck(h, objectQVar, 'List<dynamic>')
               .ifFalse;
           expect(s1.promotionInfo.unwrap(h), {
             h.promotionKeyStore.keyForVariable(objectQVar): _matchVariableModel(
-              chain: ['Object'],
-              ofInterest: ['num?', 'num*'],
+              ofInterest: ['List<Object?>', 'List<dynamic>'],
             ),
           });
-          var s2 = s1._write(h, _MockNonPromotionReason(), objectQVar,
-              SharedTypeView(Type('num?')), new SsaNode<SharedTypeView>(null));
-          // It's ambiguous whether to promote to num? or num*, but since the
-          // written type is exactly num?, we use that.
+          var s2 = s1._write(
+              h,
+              _MockNonPromotionReason(),
+              objectQVar,
+              SharedTypeView(Type('List<Object?>')),
+              new SsaNode<SharedTypeView>(null));
+          // It's ambiguous whether to promote to List<Object?> or
+          // List<dynamic>, but since the written type is exactly List<Object?>,
+          // we use that.
           expect(s2.promotionInfo.unwrap(h), {
             h.promotionKeyStore.keyForVariable(objectQVar): _matchVariableModel(
-              chain: ['num?'],
-              ofInterest: ['num?', 'num*'],
+              chain: ['List<Object?>'],
+              ofInterest: ['List<Object?>', 'List<dynamic>'],
             ),
           });
         });