Changes for map pattern: report an error for rest elements, empty map pattern.

Bug: dart-lang/language#2861
Change-Id: I00ccb3ea03aa476f96c2ecf3e3a9e13bd4926193
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291940
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: Marya Belanger <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

Author: scheglov

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -11176,26 +11176,12 @@ const MessageCode messageRequiredParameterWithDefault = const MessageCode(
         r"""Try removing the default value or making the parameter optional.""");
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Code<Null> codeRestElementWithSubpatternInMapPattern =
-    messageRestElementWithSubpatternInMapPattern;
+const Code<Null> codeRestPatternInMapPattern = messageRestPatternInMapPattern;
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const MessageCode messageRestElementWithSubpatternInMapPattern =
-    const MessageCode("RestElementWithSubpatternInMapPattern",
-        analyzerCodes: <String>["REST_ELEMENT_WITH_SUBPATTERN_IN_MAP_PATTERN"],
-        problemMessage:
-            r"""A rest element in a map pattern can't have a subpattern.""",
-        correctionMessage: r"""Try removing the subpattern.""");
-
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Code<Null> codeRestPatternNotLastInMapPattern =
-    messageRestPatternNotLastInMapPattern;
-
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const MessageCode messageRestPatternNotLastInMapPattern = const MessageCode(
-    "RestPatternNotLastInMapPattern",
-    problemMessage:
-        r"""The '...' pattern can appear only at the end in map patterns.""");
+const MessageCode messageRestPatternInMapPattern = const MessageCode(
+    "RestPatternInMapPattern",
+    problemMessage: r"""The '...' pattern can't appear in map patterns.""");
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeRethrowNotCatch = messageRethrowNotCatch;

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

@@ -143,20 +143,12 @@ class MapPatternResult<Type extends Object, Error> {
   /// The required type of the map pattern.
   final Type requiredType;
 
-  /// Errors for when multiple rest patterns occurred within the map pattern.
-  ///
-  /// The key is the index of the pattern within the map pattern.
-  ///
-  /// This is `null` if no such errors where found.
-  final Map<int, Error>? duplicateRestPatternErrors;
-
   /// Error for when the matched value type is not assignable to the required
   /// type in an irrefutable context.
   final Error? patternTypeMismatchInIrrefutableContextError;
 
   MapPatternResult(
       {required this.requiredType,
-      required this.duplicateRestPatternErrors,
       required this.patternTypeMismatchInIrrefutableContextError});
 }
 

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

@@ -1068,21 +1068,10 @@ mixin TypeAnalyzer<
         matchMayFailEvenIfCorrectType: true);
     // Stack: ()
 
-    bool hasDuplicateRestPatternReported = false;
-    Node? previousRestPattern;
-    Map<int, Error>? duplicateRestPatternErrors;
     for (int i = 0; i < elements.length; i++) {
       Node element = elements[i];
       if (isRestPatternElement(element)) {
-        if (previousRestPattern != null) {
-          (duplicateRestPatternErrors ??= {})[i] = errors.duplicateRestPattern(
-            mapOrListPattern: node,
-            original: previousRestPattern,
-            duplicate: element,
-          );
-          hasDuplicateRestPatternReported = true;
-        }
-        previousRestPattern = element;
+        errors.restPatternInMap(node: node, element: element);
       }
     }
 
@@ -1100,14 +1089,8 @@ mixin TypeAnalyzer<
         flow.popSubpattern();
       } else {
         assert(isRestPatternElement(element));
-        if (!hasDuplicateRestPatternReported) {
-          if (i != elements.length - 1) {
-            errors.restPatternNotLastInMap(node: node, element: element);
-          }
-        }
         Pattern? subPattern = getRestPatternElementPattern(element);
         if (subPattern != null) {
-          errors.restPatternWithSubPatternInMap(node: node, element: element);
           flow.pushSubpattern(dynamicType);
           dispatchPattern(
             context.withUnnecessaryWildcardKind(null),
@@ -1131,9 +1114,11 @@ mixin TypeAnalyzer<
         requiredType: requiredType,
       );
     }
+    if (elements.isEmpty) {
+      errors.emptyMapPattern(pattern: node);
+    }
     return new MapPatternResult(
         requiredType: requiredType,
-        duplicateRestPatternErrors: duplicateRestPatternErrors,
         patternTypeMismatchInIrrefutableContextError:
             patternTypeMismatchInIrrefutableContextError);
   }
@@ -2543,6 +2528,13 @@ abstract class TypeAnalyzerErrors<
     required Node duplicate,
   });
 
+  /// Called if a map pattern does not have elements.
+  ///
+  /// [pattern] is the map pattern.
+  void emptyMapPattern({
+    required Pattern pattern,
+  });
+
   /// Called when both branches have variables with the same name, but these
   /// variables either don't have the same finality, or their `NORM` types
   /// are not structurally equal.
@@ -2624,16 +2616,10 @@ abstract class TypeAnalyzerErrors<
     required Type returnType,
   });
 
-  /// Called if a rest pattern inside a map pattern is not the last element.
-  ///
-  /// [node] is the map pattern.  [element] is the rest pattern.
-  void restPatternNotLastInMap({required Pattern node, required Node element});
-
-  /// Called if a rest pattern inside a map pattern has a subpattern.
+  /// Called if a rest pattern found inside a map pattern.
   ///
   /// [node] is the map pattern.  [element] is the rest pattern.
-  void restPatternWithSubPatternInMap(
-      {required Pattern node, required Node element});
+  void restPatternInMap({required Pattern node, required Node element});
 
   /// Called if one of the case bodies of a switch statement completes normally
   /// (other than the last case body), and the "patterns" feature is not

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

@@ -7,11 +7,10 @@ untypedMap(Map map) {
     {} /*space={}*/ => 0,
     {1: _} /*space={1: ()}*/ => 1,
     {1: _, 2: _} /*space={1: (), 2: ()}*/ => 2,
-    {1: _, 2: _, ...} /*space={1: (), 2: (), ...}*/ => 3,
-    {...} /*space={...}*/ => 4,
+    Map() /*space={...}*/ => 3,
   };
   var b = /*type=Map<dynamic, dynamic>*/ switch (map) {
-    {...} /*space={...}*/ => 0,
+    Map() /*space={...}*/ => 0,
   };
 }
 
@@ -31,8 +30,8 @@ typedMap(Map<int, A> map) {
     {0: B b} /*space={0: B}*/ => 1,
     {0: C c} /*space={0: C}*/ => 2,
     {0: _, 1: _} /*space={0: A, 1: A}*/ => 3,
-    {0: B b, ...} /*space={0: B, ...}*/ => 4,
-    {0: C c, ..._} /*space={0: C, ...}*/ => 5,
+    {0: B b} /*space={0: B, ...}*/ => 4,
+    {0: C c} /*space={0: C, ...}*/ => 5,
   };
 
   var b = /*type=Map<int, A>*/ switch (map) {
@@ -43,17 +42,17 @@ typedMap(Map<int, A> map) {
    type=Map<int, A>
   */
       switch (map) {
-    <int, B>{...} /*space=<int, B>{...}*/ => 0,
+    Map<int, B>() /*space=<int, B>{...}*/ => 0,
   };
   var d = /*type=Map<int, B>*/ switch (map) {
-    {...} /*space={...}*/ => 0,
+    Map() /*space={...}*/ => 0,
     {1: _} /*
      error=unreachable,
      space={1: B}
     */
       =>
       1,
-    {2: _, ...} /*
+    {2: _} /*
      error=unreachable,
      space={2: B, ...}
     */
@@ -64,13 +63,13 @@ typedMap(Map<int, A> map) {
 
 exhaustiveRestOnly(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    {...} /*space={...}*/ => 0,
+    Map() /*space={...}*/ => 0,
   };
 }
 
 unreachableAfterRestOnly(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    {...} /*space={...}*/ => 0,
+    Map() /*space={...}*/ => 0,
     {0: _} /*
      error=unreachable,
      space={0: ()}
@@ -82,7 +81,7 @@ unreachableAfterRestOnly(Map o) {
 
 unreachableAfterRestOnlyTyped(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    {...} /*space={...}*/ => 0,
+    Map() /*space={...}*/ => 0,
     <int, String>{
       0: _
     } /*
@@ -96,7 +95,7 @@ unreachableAfterRestOnlyTyped(Map o) {
 
 unreachableAfterRestOnlyEmpty(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    {...} /*space={...}*/ => 0,
+    Map() /*space={...}*/ => 0,
     {} /*
      error=unreachable,
      space={}
@@ -108,14 +107,14 @@ unreachableAfterRestOnlyEmpty(Map o) {
 
 unreachableAfterRestSameKeys(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    {0: _, ...} /*space={0: (), ...}*/ => 0,
+    {0: _} /*space={0: (), ...}*/ => 0,
     {0: _} /*
      error=unreachable,
      space={0: ()}
     */
       =>
       1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
@@ -125,7 +124,7 @@ nonExhaustiveAfterRestSameKeys(Map o) {
    type=Map<dynamic, dynamic>
   */
       switch (o) {
-    {0: _, ...} /*space={0: (), ...}*/ => 0,
+    {0: _} /*space={0: (), ...}*/ => 0,
     {0: _} /*
      error=unreachable,
      space={0: ()}
@@ -137,7 +136,7 @@ nonExhaustiveAfterRestSameKeys(Map o) {
 
 unreachableAfterRestMoreKeys(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    {0: _, ...} /*space={0: (), ...}*/ => 0,
+    {0: _} /*space={0: (), ...}*/ => 0,
     {
       0: _,
       1: _
@@ -147,7 +146,7 @@ unreachableAfterRestMoreKeys(Map o) {
       */
       =>
       1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
@@ -157,7 +156,7 @@ nonExhaustiveAfterRestMoreKeys(Map o) {
    type=Map<dynamic, dynamic>
   */
       switch (o) {
-    {0: _, ...} /*space={0: (), ...}*/ => 0,
+    {0: _} /*space={0: (), ...}*/ => 0,
     {0: _, 1: _} /*
      error=unreachable,
      space={0: (), 1: ()}
@@ -176,7 +175,7 @@ unreachableAfterSameKeys(Map o) {
     */
       =>
       1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
@@ -200,7 +199,7 @@ reachableAfterRestOnlyDifferentTypes(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
     <int, String>{...} /*space=<int, String>{...}*/ => 0,
     <int, bool>{0: _} /*space=<int, bool>{0: bool}*/ => 1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
@@ -210,7 +209,7 @@ nonExhaustiveAfterRestOnlyDifferentTypes(Map o) {
    type=Map<dynamic, dynamic>
   */
       switch (o) {
-    <int, String>{...} /*space=<int, String>{...}*/ => 0,
+    Map<int, String>() /*space=<int, String>{...}*/ => 0,
     <int, bool>{0: _} /*space=<int, bool>{0: bool}*/ => 1,
   };
 }
@@ -219,7 +218,7 @@ reachableAfterRestOnlyEmptyDifferentTypes(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
     <int, String>{...} /*space=<int, String>{...}*/ => 0,
     <int, bool>{} /*space=<int, bool>{}*/ => 1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
@@ -229,32 +228,32 @@ nonExhaustiveAfterRestOnlyEmptyDifferentTypes(Map o) {
    type=Map<dynamic, dynamic>
   */
       switch (o) {
-    <int, String>{...} /*space=<int, String>{...}*/ => 0,
+    Map<int, String>() /*space=<int, String>{...}*/ => 0,
     <int, bool>{} /*space=<int, bool>{}*/ => 1,
   };
 }
 
 reachableAfterRestDifferentTypes(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    <int, String>{0: _, ...} /*space=<int, String>{0: String, ...}*/ => 0,
+    <int, String>{0: _} /*space=<int, String>{0: String, ...}*/ => 0,
     <int, bool>{0: _} /*space=<int, bool>{0: bool}*/ => 1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
 nonExhaustiveAfterRestDifferentTypes(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    <int, String>{0: _, ...} /*space=<int, String>{0: String, ...}*/ => 0,
+    <int, String>{0: _} /*space=<int, String>{0: String, ...}*/ => 0,
     <int, bool>{0: _} /*space=<int, bool>{0: bool}*/ => 1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
 reachableAfterRestDifferentKeys(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
-    {0: _, ...} /*space={0: (), ...}*/ => 0,
+    {0: _} /*space={0: (), ...}*/ => 0,
     {1: _} /*space={1: ()}*/ => 1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
@@ -264,7 +263,7 @@ nonExhaustiveAfterRestDifferentKeys(Map o) {
    type=Map<dynamic, dynamic>
   */
       switch (o) {
-    {0: _, ...} /*space={0: (), ...}*/ => 0,
+    {0: _} /*space={0: (), ...}*/ => 0,
     {1: _} /*space={1: ()}*/ => 1,
   };
 }
@@ -273,7 +272,7 @@ reachableAfterDifferentKeys(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
     {0: _} /*space={0: ()}*/ => 0,
     {1: _} /*space={1: ()}*/ => 1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }
 
@@ -292,7 +291,7 @@ reachableAfterDifferentTypes(Map o) {
   return /*type=Map<dynamic, dynamic>*/ switch (o) {
     <int, String>{0: _} /*space=<int, String>{0: String}*/ => 0,
     <int, bool>{0: _} /*space=<int, bool>{0: bool}*/ => 1,
-    {...} /*space={...}*/ => 2,
+    Map() /*space={...}*/ => 2,
   };
 }