Patterns parsing: recover when switch statement syntax used for switc…

…h expressions. This change ensure that if the user makes an erroneous switch expression by imitating the syntax of case statements (i.e. using `case`, `default`, `:` instead of `=>`, or `;` instead of `,`), error recovery will understand the user's intent, avoiding follow-on errors. Fixes #51886. Bug: #51886, #51943 Change-Id: Icd405d4fd4ddfb1aadcf0867e5a51ba898d4cdbc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293200 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
dart-lang · Apr 4, 2023 · a3f5cfd · a3f5cfd
1 parent 832e8fa
commit a3f5cfd
Show file tree

Hide file tree

Showing 35 changed files with 1,103 additions and 14 deletions.
diff --git a/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart b/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
@@ -2312,6 +2312,18 @@ const MessageCode messageDeclaredMemberConflictsWithOverriddenMembersCause =
         severity: Severity.context,
         problemMessage: r"""This is one of the overridden members.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Null> codeDefaultInSwitchExpression =
+    messageDefaultInSwitchExpression;
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const MessageCode messageDefaultInSwitchExpression = const MessageCode(
+    "DefaultInSwitchExpression",
+    index: 153,
+    problemMessage:
+        r"""A switch expression may not use the `default` keyword.""",
+    correctionMessage: r"""Try replacing `default` with `_`.""");
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<Message Function(String name)>
     templateDefaultValueInRedirectingFactoryConstructor =

diff --git a/pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart b/pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart
@@ -10371,7 +10371,20 @@ class Parser {
       mayParseFunctionExpressions = false;
       while (true) {
         listener.beginSwitchExpressionCase();
-        token = parsePattern(token, PatternContext.matching);
+        next = token.next!;
+        if (optional('default', next)) {
+          reportRecoverableError(next, codes.messageDefaultInSwitchExpression);
+          listener.handleNoType(next);
+          listener.handleWildcardPattern(null, next);
+          token = next;
+        } else {
+          if (optional('case', next)) {
+            reportRecoverableError(
+                next, codes.templateUnexpectedToken.withArguments(next));
+            token = next;
+          }
+          token = parsePattern(token, PatternContext.matching);
+        }
         listener.handleSwitchExpressionCasePattern(token);
         Token? when;
         next = token.next!;
@@ -10400,6 +10413,12 @@ class Parser {
         if (optional(',', next)) {
           comma = token = next;
           next = token.next!;
+        } else if (optional(';', next)) {
+          // User accidentally used `;` instead of `,`
+          reportRecoverableError(
+              next, codes.templateExpectedButGot.withArguments(','));
+          comma = token = next;
+          next = token.next!;
         }
         if (optional('}', next)) {
           break;
@@ -10418,13 +10437,16 @@ class Parser {
           } else {
             // Scanner guarantees a closing curly bracket
             Token closingBracket = beginSwitch.endGroup!;
-            comma = findNextComma(next, closingBracket);
+            comma = findNextCommaOrSemicolon(next, closingBracket);
             if (comma == null) {
               reportRecoverableError(
                   next, codes.templateExpectedButGot.withArguments('}'));
               next = closingBracket;
               break;
             } else {
+              // Note: `findNextCommaOrSemicolon` might have found a `;` instead
+              // of a `,`, but if it did, there's need to report an additional
+              // error.
               reportRecoverableError(
                   next, codes.templateExpectedButGot.withArguments(','));
               token = comma;
@@ -10442,14 +10464,14 @@ class Parser {
     return token;
   }
 
-  /// Finds and returns the next `,` token, starting at [token], but not
+  /// Finds and returns the next `,` or `;` token, starting at [token], but not
   /// searching beyond [limit].  If a begin token is encountered, the search
   /// proceeds after its matching end token, so the returned token (if any) will
   /// not be any more deeply nested than the starting point.
-  Token? findNextComma(Token token, Token limit) {
+  Token? findNextCommaOrSemicolon(Token token, Token limit) {
     while (true) {
       if (token.isEof || identical(token, limit)) return null;
-      if (optional(',', token)) return token;
+      if (optional(',', token) || optional(';', token)) return token;
       if (token is BeginToken) {
         token = token.endGroup!;
       } else {

diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -2320,6 +2320,8 @@ ParserErrorCode.COVARIANT_MEMBER:
   status: needsEvaluation
 ParserErrorCode.COVARIANT_TOP_LEVEL_DECLARATION:
   status: needsEvaluation
+ParserErrorCode.DEFAULT_IN_SWITCH_EXPRESSION:
+  status: needsEvaluation
 ParserErrorCode.DEFAULT_VALUE_IN_FUNCTION_TYPE:
   status: needsEvaluation
 ParserErrorCode.DEFERRED_AFTER_PREFIX:

diff --git a/pkg/analyzer/lib/src/dart/error/syntactic_errors.g.dart b/pkg/analyzer/lib/src/dart/error/syntactic_errors.g.dart
@@ -167,6 +167,7 @@ final fastaAnalyzerErrorCodes = <ErrorCode?>[
   ParserErrorCode.INVALID_INSIDE_UNARY_PATTERN,
   ParserErrorCode.LATE_PATTERN_VARIABLE_DECLARATION,
   ParserErrorCode.PATTERN_VARIABLE_DECLARATION_OUTSIDE_FUNCTION_OR_METHOD,
+  ParserErrorCode.DEFAULT_IN_SWITCH_EXPRESSION,
 ];
 
 class ParserErrorCode extends ErrorCode {
@@ -415,6 +416,13 @@ class ParserErrorCode extends ErrorCode {
     correctionMessage: "Try removing the keyword 'covariant'.",
   );
 
+  ///  No parameters.
+  static const ParserErrorCode DEFAULT_IN_SWITCH_EXPRESSION = ParserErrorCode(
+    'DEFAULT_IN_SWITCH_EXPRESSION',
+    "A switch expression may not use the `default` keyword.",
+    correctionMessage: "Try replacing `default` with `_`.",
+  );
+
   ///  No parameters.
   static const ParserErrorCode DEFAULT_VALUE_IN_FUNCTION_TYPE = ParserErrorCode(
     'DEFAULT_VALUE_IN_FUNCTION_TYPE',

diff --git a/pkg/analyzer/lib/src/error/error_code_values.g.dart b/pkg/analyzer/lib/src/error/error_code_values.g.dart
@@ -652,6 +652,7 @@ const List<ErrorCode> errorCodeValues = [
   ParserErrorCode.COVARIANT_CONSTRUCTOR,
   ParserErrorCode.COVARIANT_MEMBER,
   ParserErrorCode.COVARIANT_TOP_LEVEL_DECLARATION,
+  ParserErrorCode.DEFAULT_IN_SWITCH_EXPRESSION,
   ParserErrorCode.DEFAULT_VALUE_IN_FUNCTION_TYPE,
   ParserErrorCode.DEFERRED_AFTER_PREFIX,
   ParserErrorCode.DIRECTIVE_AFTER_DECLARATION,

diff --git a/pkg/analyzer/lib/src/fasta/ast_builder.dart b/pkg/analyzer/lib/src/fasta/ast_builder.dart
@@ -5505,7 +5505,9 @@ class AstBuilder extends StackListener {
   void handleWildcardPattern(Token? keyword, Token wildcard) {
     debugEvent('WildcardPattern');
     assert(_featureSet.isEnabled(Feature.patterns));
-    assert(wildcard.lexeme == '_');
+    // Note: if `default` appears in a switch expression, parser error recovery
+    // treats it as a wildcard pattern.
+    assert(wildcard.lexeme == '_' || wildcard.lexeme == 'default');
     var type = pop() as TypeAnnotationImpl?;
     push(
       WildcardPatternImpl(

diff --git a/pkg/analyzer/test/generated/patterns_parser_test.dart b/pkg/analyzer/test/generated/patterns_parser_test.dart
@@ -9393,6 +9393,46 @@ SwitchExpression
 ''');
   }
 
+  test_switchExpression_recovery_caseKeyword() {
+    _parse('''
+f(x) => switch (x) {
+  case 1 => 'one',
+  case 2 => 'two'
+};
+''', errors: [
+      error(ParserErrorCode.UNEXPECTED_TOKEN, 23, 4),
+      error(ParserErrorCode.UNEXPECTED_TOKEN, 42, 4),
+    ]);
+    var node = findNode.switchExpression('switch');
+    assertParsedNodeText(node, r'''
+SwitchExpression
+  switchKeyword: switch
+  leftParenthesis: (
+  expression: SimpleIdentifier
+    token: x
+  rightParenthesis: )
+  leftBracket: {
+  cases
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: ConstantPattern
+          expression: IntegerLiteral
+            literal: 1
+      arrow: =>
+      expression: SimpleStringLiteral
+        literal: 'one'
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: ConstantPattern
+          expression: IntegerLiteral
+            literal: 2
+      arrow: =>
+      expression: SimpleStringLiteral
+        literal: 'two'
+  rightBracket: }
+''');
+  }
+
   test_switchExpression_recovery_colonInsteadOfArrow() {
     _parse('''
 f(x) => switch (x) {
@@ -9433,6 +9473,44 @@ SwitchExpression
 ''');
   }
 
+  test_switchExpression_recovery_defaultKeyword() {
+    _parse('''
+f(x) => switch (x) {
+  1 => 'one',
+  default => 'other'
+};
+''', errors: [
+      error(ParserErrorCode.DEFAULT_IN_SWITCH_EXPRESSION, 37, 7),
+    ]);
+    var node = findNode.switchExpression('switch');
+    assertParsedNodeText(node, r'''
+SwitchExpression
+  switchKeyword: switch
+  leftParenthesis: (
+  expression: SimpleIdentifier
+    token: x
+  rightParenthesis: )
+  leftBracket: {
+  cases
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: ConstantPattern
+          expression: IntegerLiteral
+            literal: 1
+      arrow: =>
+      expression: SimpleStringLiteral
+        literal: 'one'
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: WildcardPattern
+          name: default
+      arrow: =>
+      expression: SimpleStringLiteral
+        literal: 'other'
+  rightBracket: }
+''');
+  }
+
   test_switchExpression_recovery_illegalFunctionExpressionInGuard() {
     // If a function expression occurs in a guard, parsing skips to the case
     // that follows.
@@ -9477,6 +9555,52 @@ SwitchExpression
 ''');
   }
 
+  test_switchExpression_recovery_illegalFunctionExpressionInGuard_semicolon() {
+    // If a function expression occurs in a guard, parsing skips to the case
+    // that follows.  The logic to skip to the next case understands that a
+    // naive user might have mistakenly used `;` instead of `,` to separate
+    // cases.
+    _parse('''
+f(x) => switch (x) {
+  _ when () => true => 1;
+  _ => 2
+};
+''', errors: [
+      error(ParserErrorCode.EXPECTED_TOKEN, 41, 2),
+    ]);
+    var node = findNode.switchExpression('switch');
+    assertParsedNodeText(node, r'''
+SwitchExpression
+  switchKeyword: switch
+  leftParenthesis: (
+  expression: SimpleIdentifier
+    token: x
+  rightParenthesis: )
+  leftBracket: {
+  cases
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: WildcardPattern
+          name: _
+        whenClause: WhenClause
+          whenKeyword: when
+          expression: RecordLiteral
+            leftParenthesis: (
+            rightParenthesis: )
+      arrow: =>
+      expression: BooleanLiteral
+        literal: true
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: WildcardPattern
+          name: _
+      arrow: =>
+      expression: IntegerLiteral
+        literal: 2
+  rightBracket: }
+''');
+  }
+
   test_switchExpression_recovery_missingComma() {
     // If the extra tokens after a switch case look like they could be a
     // pattern, the parser assumes there's a missing comma.
@@ -9524,6 +9648,45 @@ SwitchExpression
 ''');
   }
 
+  test_switchExpression_recovery_semicolonInsteadOfComma() {
+    _parse('''
+f(x) => switch (x) {
+  1 => 'one';
+  2 => 'two'
+};
+''', errors: [
+      error(ParserErrorCode.EXPECTED_TOKEN, 33, 1),
+    ]);
+    var node = findNode.switchExpression('switch');
+    assertParsedNodeText(node, r'''
+SwitchExpression
+  switchKeyword: switch
+  leftParenthesis: (
+  expression: SimpleIdentifier
+    token: x
+  rightParenthesis: )
+  leftBracket: {
+  cases
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: ConstantPattern
+          expression: IntegerLiteral
+            literal: 1
+      arrow: =>
+      expression: SimpleStringLiteral
+        literal: 'one'
+    SwitchExpressionCase
+      guardedPattern: GuardedPattern
+        pattern: ConstantPattern
+          expression: IntegerLiteral
+            literal: 2
+      arrow: =>
+      expression: SimpleStringLiteral
+        literal: 'two'
+  rightBracket: }
+''');
+  }
+
   test_switchExpression_twoPatterns() {
     _parse('''
 f(x) => switch(x) {

diff --git a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
@@ -9365,7 +9365,9 @@ class BodyBuilder extends StackListenerImpl
         libraryFeatures.patterns, wildcard.charOffset, wildcard.charCount);
     TypeBuilder? type = pop(NullValues.TypeBuilder) as TypeBuilder?;
     DartType? patternType = type?.build(libraryBuilder, TypeUse.variableType);
-    assert(wildcard.lexeme == '_');
+    // Note: if `default` appears in a switch expression, parser error recovery
+    // treats it as a wildcard pattern.
+    assert(wildcard.lexeme == '_' || wildcard.lexeme == 'default');
     push(forest.createWildcardPattern(wildcard.charOffset, patternType));
   }
 

diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml
@@ -6755,6 +6755,19 @@ PatternVariableDeclarationOutsideFunctionOrMethod:
       var (a, b) = (0, 1);
     }
 
+DefaultInSwitchExpression:
+  problemMessage: A switch expression may not use the `default` keyword.
+  correctionMessage: Try replacing `default` with `_`.
+  analyzerCode: ParserErrorCode.DEFAULT_IN_SWITCH_EXPRESSION
+  index: 153
+  experiments: patterns
+  comment: No parameters.
+  script: |
+    void f(x) => switch (x) {
+      1 => 'one',
+      default => 'other'
+    };
+
 WeakReferenceNotStatic:
   problemMessage: "Weak reference pragma can be used on a static method only."
 

diff --git a/...testcases/patterns/switchExpression_recovery_bogusTokensAfterCase.dart.intertwined.expect b/...testcases/patterns/switchExpression_recovery_bogusTokensAfterCase.dart.intertwined.expect
@@ -85,7 +85,7 @@ parseUnit(f)
                               parseLiteralInt(=>)
                                 listener: handleLiteralInt(0)
                       listener: endSwitchExpressionCase(null, =>, 0)
-                      findNextComma(:, })
+                      findNextCommaOrSemicolon(:, })
                       reportRecoverableError(:, Message[ExpectedButGot, Expected '}' before this., null, {string: }}])
                         listener: handleRecoverableError(Message[ExpectedButGot, Expected '}' before this., null, {string: }}], :, :)
                       listener: endSwitchExpressionBlock(1, {, })

diff --git a/...atterns/switchExpression_recovery_bogusTokensAfterCase_laterComma.dart.intertwined.expect b/...atterns/switchExpression_recovery_bogusTokensAfterCase_laterComma.dart.intertwined.expect
@@ -93,7 +93,7 @@ parseUnit(f)
                                         parseLiteralInt(=>)
                                           listener: handleLiteralInt(0)
                                 listener: endSwitchExpressionCase(null, =>, 0)
-                                findNextComma(:, })
+                                findNextCommaOrSemicolon(:, })
                                 reportRecoverableError(:, Message[ExpectedButGot, Expected '}' before this., null, {string: }}])
                                   listener: handleRecoverableError(Message[ExpectedButGot, Expected '}' before this., null, {string: }}], :, :)
                                 listener: endSwitchExpressionBlock(1, {, })

diff --git a/...tterns/switchExpression_recovery_bogusTokensAfterCase_nestedComma.dart.intertwined.expect b/...tterns/switchExpression_recovery_bogusTokensAfterCase_nestedComma.dart.intertwined.expect
@@ -85,7 +85,7 @@ parseUnit(f)
                               parseLiteralInt(=>)
                                 listener: handleLiteralInt(0)
                       listener: endSwitchExpressionCase(null, =>, 0)
-                      findNextComma(:, })
+                      findNextCommaOrSemicolon(:, })
                       reportRecoverableError(:, Message[ExpectedButGot, Expected '}' before this., null, {string: }}])
                         listener: handleRecoverableError(Message[ExpectedButGot, Expected '}' before this., null, {string: }}], :, :)
                       listener: endSwitchExpressionBlock(1, {, })

diff --git a/pkg/front_end/parser_testcases/patterns/switchExpression_recovery_caseKeyword.dart b/pkg/front_end/parser_testcases/patterns/switchExpression_recovery_caseKeyword.dart
@@ -0,0 +1,4 @@
+f(x) => switch (x) {
+  case 1 => 'one',
+  case 2 => 'two'
+};