[_fe_shared_analyzer] Pass context type when creating spaces from patterns

This passes the [StaticType] for the context of a pattern to the
pattern converter. This is used to restrict the static type of the
created space. For instance when creating the space for an untyped
wildcard pattern, the context type will be used instead of the
nullable-object (top) type. For other patterns, if the context type
is a subtype of the type for the pattern itself, the context type
will be used instead.

This has the benefit that when checking for unreachable cases we will
not take a too broad case to mean that it has some that it could match.
For instance in `Foo(:var hashCode)` the space for `var hashCode` would
have been the nullable-object type, and if the preceeding case was
`Foo(:int hashCode)`, the reachable check would fail to see that all
cases are covered since nullable-object is not a subtype of int.

With this change, such cases are now handled.

Change-Id: I5b6248bf79c8cf0b365eee2f3ca78090c3f43512
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288902
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>

Author: johnniwinther

pkg/_fe_analyzer_shared/lib/src/exhaustiveness/shared.dart

@@ -316,16 +316,31 @@ class ExhaustivenessCache<
 mixin SpaceCreator<Pattern extends Object, Type extends Object> {
   TypeOperations<Type> get typeOperations;
 
+  ObjectFieldLookup get objectFieldLookup;
+
   /// Creates a [StaticType] for an unknown type.
   ///
   /// This is used when the type of the pattern is unknown or can't be
   /// represented as a [StaticType]. This type is unique and ensures that it
   /// is neither matches anything nor is matched by anything.
   StaticType createUnknownStaticType();
 
-  /// Creates the [StaticType] for [type]. If [nonNull] is `true`, the created
-  /// type is non-nullable.
-  StaticType createStaticType(Type type, {required bool nonNull});
+  /// Creates the [StaticType] for [type].
+  StaticType createStaticType(Type type);
+
+  /// Creates the [StaticType] for [type] restricted by the [contextType].
+  /// If [nonNull] is `true`, the created type is non-nullable.
+  StaticType _createStaticTypeWithContext(StaticType contextType, Type type,
+      {required bool nonNull}) {
+    StaticType staticType = createStaticType(type);
+    if (contextType.isSubtypeOf(staticType)) {
+      staticType = contextType;
+    }
+    if (nonNull && staticType is NullableStaticType) {
+      staticType = staticType.underlying;
+    }
+    return staticType;
+  }
 
   /// Creates the [StaticType] for the list [type] with the given [identity].
   StaticType createListType(Type type, ListTypeIdentity<Type> identity);
@@ -335,75 +350,100 @@ mixin SpaceCreator<Pattern extends Object, Type extends Object> {
 
   /// Creates the [Space] for [pattern] at the given [path].
   ///
+  /// The [contextType] is the [StaticType] in which the pattern match is
+  /// performed. This is used to the restrict type of the created [Space] to
+  /// the types allowed by the context. For instance `Object(:var hashCode)` is
+  /// in itself unrestricted and would yield the top space for matching
+  /// `var hashCode`. Using the [contextType] `int`, as given by the type of
+  /// the `Object.hashCode`, the created space is all `int` values rather than
+  /// all values.
+  ///
   /// If [nonNull] is `true`, the space is implicitly non-nullable.
-  Space dispatchPattern(Path path, Pattern pattern, {required bool nonNull});
+  Space dispatchPattern(Path path, StaticType contextType, Pattern pattern,
+      {required bool nonNull});
 
   /// Creates the root space for [pattern].
-  Space createRootSpace(Pattern pattern, {required bool hasGuard}) {
+  Space createRootSpace(StaticType contextType, Pattern pattern,
+      {required bool hasGuard}) {
     if (hasGuard) {
       return createUnknownSpace(const Path.root());
     } else {
-      return dispatchPattern(const Path.root(), pattern, nonNull: false);
+      return dispatchPattern(const Path.root(), contextType, pattern,
+          nonNull: false);
     }
   }
 
   /// Creates the [Space] at [path] for a variable pattern of the declared
   /// [type].
   ///
   /// If [nonNull] is `true`, the space is implicitly non-nullable.
-  Space createVariableSpace(Path path, Type type, {required bool nonNull}) {
-    return new Space(path, createStaticType(type, nonNull: nonNull));
+  Space createVariableSpace(Path path, StaticType contextType, Type type,
+      {required bool nonNull}) {
+    StaticType staticType =
+        _createStaticTypeWithContext(contextType, type, nonNull: nonNull);
+    return new Space(path, staticType);
   }
 
   /// Creates the [Space] at [path] for an object pattern of the required [type]
   /// and [fieldPatterns].
   ///
   /// If [nonNull] is `true`, the space is implicitly non-nullable.
-  Space createObjectSpace(
-      Path path, Type type, Map<String, Pattern> fieldPatterns,
+  Space createObjectSpace(Path path, StaticType contextType, Type type,
+      Map<String, Pattern> fieldPatterns,
       {required bool nonNull}) {
+    StaticType staticType =
+        _createStaticTypeWithContext(contextType, type, nonNull: nonNull);
     Map<String, Space> fields = <String, Space>{};
     for (MapEntry<String, Pattern> entry in fieldPatterns.entries) {
       String name = entry.key;
-      fields[name] =
-          dispatchPattern(path.add(name), entry.value, nonNull: false);
+      StaticType fieldType = staticType.getField(objectFieldLookup, name) ??
+          StaticType.nullableObject;
+      fields[name] = dispatchPattern(path.add(name), fieldType, entry.value,
+          nonNull: false);
     }
-    StaticType staticType = createStaticType(type, nonNull: nonNull);
     return new Space(path, staticType, fields: fields);
   }
 
   /// Creates the [Space] at [path] for a record pattern of the required [type],
   /// [positionalFields], and [namedFields].
-  Space createRecordSpace(Path path, Type recordType,
+  Space createRecordSpace(Path path, StaticType contextType, Type recordType,
       List<Pattern> positionalFields, Map<String, Pattern> namedFields) {
+    StaticType staticType =
+        _createStaticTypeWithContext(contextType, recordType, nonNull: true);
     Map<String, Space> fields = <String, Space>{};
     for (int index = 0; index < positionalFields.length; index++) {
       String name = '\$${index + 1}';
-      fields[name] = dispatchPattern(path.add(name), positionalFields[index],
+      StaticType fieldType = staticType.getField(objectFieldLookup, name) ??
+          StaticType.nullableObject;
+      fields[name] = dispatchPattern(
+          path.add(name), fieldType, positionalFields[index],
           nonNull: false);
     }
     for (MapEntry<String, Pattern> entry in namedFields.entries) {
       String name = entry.key;
-      fields[name] =
-          dispatchPattern(path.add(name), entry.value, nonNull: false);
+      StaticType fieldType = staticType.getField(objectFieldLookup, name) ??
+          StaticType.nullableObject;
+      fields[name] = dispatchPattern(path.add(name), fieldType, entry.value,
+          nonNull: false);
     }
-    return new Space(path, createStaticType(recordType, nonNull: true),
-        fields: fields);
+    return new Space(path, staticType, fields: fields);
   }
 
   /// Creates the [Space] at [path] for a wildcard pattern with the declared
   /// [type].
   ///
   /// If [nonNull] is `true`, the space is implicitly non-nullable.
-  Space createWildcardSpace(Path path, Type? type, {required bool nonNull}) {
+  Space createWildcardSpace(Path path, StaticType contextType, Type? type,
+      {required bool nonNull}) {
     if (type == null) {
-      if (nonNull) {
-        return new Space(path, StaticType.nonNullableObject);
-      } else {
-        return new Space(path, StaticType.nullableObject);
+      StaticType staticType = contextType;
+      if (nonNull && staticType is NullableStaticType) {
+        staticType = staticType.underlying;
       }
+      return new Space(path, staticType);
     } else {
-      StaticType staticType = createStaticType(type, nonNull: nonNull);
+      StaticType staticType =
+          _createStaticTypeWithContext(contextType, type, nonNull: nonNull);
       return new Space(path, staticType);
     }
   }
@@ -418,45 +458,49 @@ mixin SpaceCreator<Pattern extends Object, Type extends Object> {
   /// [subPattern].
   ///
   /// If [nonNull] is `true`, the space is implicitly non-nullable.
-  Space createCastSpace(Path path, Pattern subPattern,
+  Space createCastSpace(Path path, StaticType contextType, Pattern subPattern,
       {required bool nonNull}) {
     // TODO(johnniwinther): Handle types (sibling sealed types?) implicitly
     // handled by the throw of the invalid cast.
-    return dispatchPattern(path, subPattern, nonNull: nonNull);
+    return dispatchPattern(path, contextType, subPattern, nonNull: nonNull);
   }
 
   /// Creates the [Space] at [path] for a null check pattern with the given
   /// [subPattern].
-  Space createNullCheckSpace(Path path, Pattern subPattern) {
-    return dispatchPattern(path, subPattern, nonNull: true);
+  Space createNullCheckSpace(
+      Path path, StaticType contextType, Pattern subPattern) {
+    return dispatchPattern(path, contextType, subPattern, nonNull: true);
   }
 
   /// Creates the [Space] at [path] for a null assert pattern with the given
   /// [subPattern].
-  Space createNullAssertSpace(Path path, Pattern subPattern) {
-    Space space = dispatchPattern(path, subPattern, nonNull: true);
+  Space createNullAssertSpace(
+      Path path, StaticType contextType, Pattern subPattern) {
+    Space space = dispatchPattern(path, contextType, subPattern, nonNull: true);
     return space.union(new Space(path, StaticType.nullType));
   }
 
   /// Creates the [Space] at [path] for a logical or pattern with the given
   /// [left] and [right] subpatterns.
   ///
   /// If [nonNull] is `true`, the space is implicitly non-nullable.
-  Space createLogicalOrSpace(Path path, Pattern left, Pattern right,
+  Space createLogicalOrSpace(
+      Path path, StaticType contextType, Pattern left, Pattern right,
       {required bool nonNull}) {
-    Space aSpace = dispatchPattern(path, left, nonNull: nonNull);
-    Space bSpace = dispatchPattern(path, right, nonNull: nonNull);
+    Space aSpace = dispatchPattern(path, contextType, left, nonNull: nonNull);
+    Space bSpace = dispatchPattern(path, contextType, right, nonNull: nonNull);
     return aSpace.union(bSpace);
   }
 
   /// Creates the [Space] at [path] for a logical and pattern with the given
   /// [left] and [right] subpatterns.
   ///
   /// If [nonNull] is `true`, the space is implicitly non-nullable.
-  Space createLogicalAndSpace(Path path, Pattern left, Pattern right,
+  Space createLogicalAndSpace(
+      Path path, StaticType contextType, Pattern left, Pattern right,
       {required bool nonNull}) {
-    Space aSpace = dispatchPattern(path, left, nonNull: nonNull);
-    Space bSpace = dispatchPattern(path, right, nonNull: nonNull);
+    Space aSpace = dispatchPattern(path, contextType, left, nonNull: nonNull);
+    Space bSpace = dispatchPattern(path, contextType, right, nonNull: nonNull);
     return _createSpaceIntersection(path, aSpace, bSpace);
   }
 
@@ -469,47 +513,56 @@ mixin SpaceCreator<Pattern extends Object, Type extends Object> {
       required List<Pattern> tailElements,
       required bool hasRest,
       required bool hasExplicitTypeArgument}) {
-    Map<Key, Space> additionalFields = {};
     int headSize = headElements.length;
     int tailSize = tailElements.length;
+
+    String typeArgumentText;
+    if (hasExplicitTypeArgument) {
+      StringBuffer sb = new StringBuffer();
+      sb.write('<');
+      sb.write(typeOperations.typeToString(elementType));
+      sb.write('>');
+      typeArgumentText = sb.toString();
+    } else {
+      typeArgumentText = '';
+    }
+
+    ListTypeIdentity<Type> identity = new ListTypeIdentity(
+        elementType, typeArgumentText,
+        size: headSize + tailSize, hasRest: hasRest);
+
+    StaticType staticType = createListType(type, identity);
+
+    Map<Key, Space> additionalFields = {};
     for (int index = 0; index < headSize; index++) {
       Key key = new HeadKey(index);
+      StaticType fieldType =
+          staticType.getAdditionalField(key) ?? StaticType.nullableObject;
       additionalFields[key] = dispatchPattern(
-          path.add(key.name), headElements[index],
+          path.add(key.name), fieldType, headElements[index],
           nonNull: false);
     }
     if (hasRest) {
       Key key = new RestKey(headSize, tailSize);
+      StaticType fieldType =
+          staticType.getAdditionalField(key) ?? StaticType.nullableObject;
       if (restElement != null) {
-        additionalFields[key] =
-            dispatchPattern(path.add(key.name), restElement, nonNull: false);
+        additionalFields[key] = dispatchPattern(
+            path.add(key.name), fieldType, restElement,
+            nonNull: false);
       } else {
-        additionalFields[key] =
-            new Space(path.add(key.name), StaticType.nullableObject);
+        additionalFields[key] = new Space(path.add(key.name), fieldType);
       }
     }
     for (int index = 0; index < tailSize; index++) {
       Key key = new TailKey(index);
-      additionalFields[key] = dispatchPattern(
-          path.add(key.name), tailElements[tailElements.length - index - 1],
+      StaticType fieldType =
+          staticType.getAdditionalField(key) ?? StaticType.nullableObject;
+      additionalFields[key] = dispatchPattern(path.add(key.name), fieldType,
+          tailElements[tailElements.length - index - 1],
           nonNull: false);
     }
-    String typeArgumentText;
-    if (hasExplicitTypeArgument) {
-      StringBuffer sb = new StringBuffer();
-      sb.write('<');
-      sb.write(typeOperations.typeToString(elementType));
-      sb.write('>');
-      typeArgumentText = sb.toString();
-    } else {
-      typeArgumentText = '';
-    }
-
-    ListTypeIdentity<Type> identity = new ListTypeIdentity(
-        elementType, typeArgumentText,
-        size: headSize + tailSize, hasRest: hasRest);
-    return new Space(path, createListType(type, identity),
-        additionalFields: additionalFields);
+    return new Space(path, staticType, additionalFields: additionalFields);
   }
 
   /// Creates the [Space] at [path] for a map pattern.
@@ -520,12 +573,6 @@ mixin SpaceCreator<Pattern extends Object, Type extends Object> {
       required Map<MapKey, Pattern> entries,
       required bool hasRest,
       required bool hasExplicitTypeArguments}) {
-    Map<Key, Space> additionalFields = {};
-    for (MapEntry<Key, Pattern> entry in entries.entries) {
-      Key key = entry.key;
-      additionalFields[key] =
-          dispatchPattern(path.add(key.name), entry.value, nonNull: false);
-    }
     String typeArgumentsText;
     if (hasExplicitTypeArguments) {
       StringBuffer sb = new StringBuffer();
@@ -542,8 +589,18 @@ mixin SpaceCreator<Pattern extends Object, Type extends Object> {
     MapTypeIdentity<Type> identity = new MapTypeIdentity(
         keyType, valueType, entries.keys.toSet(), typeArgumentsText,
         hasRest: hasRest);
-    return new Space(path, createMapType(type, identity),
-        additionalFields: additionalFields);
+    StaticType staticType = createMapType(type, identity);
+
+    Map<Key, Space> additionalFields = {};
+    for (MapEntry<Key, Pattern> entry in entries.entries) {
+      Key key = entry.key;
+      StaticType fieldType =
+          staticType.getAdditionalField(key) ?? StaticType.nullableObject;
+      additionalFields[key] = dispatchPattern(
+          path.add(key.name), fieldType, entry.value,
+          nonNull: false);
+    }
+    return new Space(path, staticType, additionalFields: additionalFields);
   }
 
   /// Creates the [Space] at [path] for a pattern with unknown space.

pkg/_fe_analyzer_shared/test/exhaustiveness/data/and_pattern.dart

@@ -17,7 +17,10 @@ class E extends B {}
 and(A o1, A o2) {
   var a = /*type=A*/switch (o1) {
     A() && var a /*space=A*/=> 0,
-    _ /*space=()*/=> 1,
+    _ /*
+     error=unreachable,
+     space=A
+    */=> 1,
   };
 
   var b = /*type=A*/switch (o1) {

pkg/_fe_analyzer_shared/test/exhaustiveness/data/cast.dart

@@ -61,7 +61,7 @@ sealedCast(B b1, B b2) {
    type=B
   */switch (b1) {
     /*space=C*/case C():
-    /*space=()*/case _ as D:
+    /*space=B*/case _ as D:
   }
   /*
    error=non-exhaustive:E,