[stable] Patterns parser: prohibit variable/identifier patterns named…

… when/as. In https://dart-review.googlesource.com/c/sdk/+/299400, the parser was adjusted so that it no longer accepts `when` and `as` as the names for variable patterns in cases where there is a possible ambiguity (e.g. `int when` is not accepted as a pattern because `int` is a legitimate pattern, therefore `when` could introduce a guard clause). This change further prohibits `when` and `as` from being the names of variable patterns or identifier patterns even in the case where there is no ambiguity. This is in line with the discussion at #52199 (comment), and the spec change at dart-lang/language#3033. Three new error codes are introduced, to cover the three circumstances in which `when` or `as` might be used illegally: in a declared variable pattern, in an assigned variable pattern, or in an identifier pattern. I've also added analyzer tests to ensure that the parser recovers from these errors nicely. Unfortunately, nice error recovery is only feasible in the non-ambiguous cases. I've also updated the language test expectations in `tests/language/patterns/version_2_32_changes_error_test.dart` to reflect the new error messages, and added a few more examples of uses of `when` and `as` that are still permitted. Fixes: #52311 Bug: #52260 Change-Id: I30cb3a1ca627c6db3d281740fb59de74c4118c2b Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/301482 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302100 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Jens Johansen <jensj@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
dart-lang · May 12, 2023 · 2f11564 · 2f11564
1 parent c6b0b74
commit 2f11564
Show file tree

Hide file tree

Showing 38 changed files with 1,359 additions and 1 deletion.
diff --git a/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart b/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
@@ -5684,6 +5684,84 @@ Message _withArgumentsIllegalMixinDueToConstructorsCause(String name) {
       arguments: {'name': name});
 }
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        Token
+            token)> templateIllegalPatternAssignmentVariableName = const Template<
+        Message Function(Token token)>(
+    problemMessageTemplate:
+        r"""A variable assigned by a pattern assignment can't be named '#lexeme'.""",
+    correctionMessageTemplate: r"""Choose a different name.""",
+    withArguments: _withArgumentsIllegalPatternAssignmentVariableName);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(Token token)>
+    codeIllegalPatternAssignmentVariableName =
+    const Code<Message Function(Token token)>(
+        "IllegalPatternAssignmentVariableName",
+        index: 155);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsIllegalPatternAssignmentVariableName(Token token) {
+  String lexeme = token.lexeme;
+  return new Message(codeIllegalPatternAssignmentVariableName,
+      problemMessage:
+          """A variable assigned by a pattern assignment can't be named '${lexeme}'.""",
+      correctionMessage: """Choose a different name.""",
+      arguments: {'lexeme': token});
+}
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<Message Function(Token token)>
+    templateIllegalPatternIdentifierName =
+    const Template<Message Function(Token token)>(
+        problemMessageTemplate:
+            r"""A pattern can't refer to an identifier named '#lexeme'.""",
+        correctionMessageTemplate: r"""Match the identifier using '==""",
+        withArguments: _withArgumentsIllegalPatternIdentifierName);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(Token token)> codeIllegalPatternIdentifierName =
+    const Code<Message Function(Token token)>("IllegalPatternIdentifierName",
+        index: 156);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsIllegalPatternIdentifierName(Token token) {
+  String lexeme = token.lexeme;
+  return new Message(codeIllegalPatternIdentifierName,
+      problemMessage:
+          """A pattern can't refer to an identifier named '${lexeme}'.""",
+      correctionMessage: """Match the identifier using '==""",
+      arguments: {'lexeme': token});
+}
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        Token
+            token)> templateIllegalPatternVariableName = const Template<
+        Message Function(Token token)>(
+    problemMessageTemplate:
+        r"""The variable declared by a variable pattern can't be named '#lexeme'.""",
+    correctionMessageTemplate: r"""Choose a different name.""",
+    withArguments: _withArgumentsIllegalPatternVariableName);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(Token token)> codeIllegalPatternVariableName =
+    const Code<Message Function(Token token)>("IllegalPatternVariableName",
+        index: 154);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsIllegalPatternVariableName(Token token) {
+  String lexeme = token.lexeme;
+  return new Message(codeIllegalPatternVariableName,
+      problemMessage:
+          """The variable declared by a variable pattern can't be named '${lexeme}'.""",
+      correctionMessage: """Choose a different name.""",
+      arguments: {'lexeme': token});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeIllegalSyncGeneratorReturnType =
     messageIllegalSyncGeneratorReturnType;

diff --git a/pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart b/pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart
@@ -117,6 +117,7 @@ import 'type_info.dart'
         computeType,
         computeTypeParamOrArg,
         computeVariablePatternType,
+        illegalPatternIdentifiers,
         isValidNonRecordTypeReference,
         noType,
         noTypeParamOrArg;
@@ -9874,12 +9875,18 @@ class Parser {
       } else if (dot == null) {
         // It's a single identifier.  If it's a wildcard pattern or we're in an
         // irrefutable context, parse it as a variable pattern.
-        if (!patternContext.isRefutable || firstIdentifier.lexeme == '_') {
+        String name = firstIdentifier.lexeme;
+        if (!patternContext.isRefutable || name == '_') {
           // It's a wildcard pattern with no preceding type, so parse it as a
           // variable pattern.
           isLastPatternAllowedInsideUnaryPattern = true;
           return parseVariablePattern(beforeFirstIdentifier, patternContext,
               typeInfo: typeInfo);
+        } else if (illegalPatternIdentifiers.contains(name)) {
+          reportRecoverableError(
+              firstIdentifier,
+              codes.templateIllegalPatternIdentifierName
+                  .withArguments(firstIdentifier));
         }
       }
       // It's not an object pattern so parse it as an expression.
@@ -9963,8 +9970,18 @@ class Parser {
       }
       listener.handleWildcardPattern(keyword, token);
     } else if (inAssignmentPattern && isBareIdentifier) {
+      if (illegalPatternIdentifiers.contains(variableName)) {
+        reportRecoverableError(
+            token,
+            codes.templateIllegalPatternAssignmentVariableName
+                .withArguments(token));
+      }
       listener.handleAssignedVariablePattern(token);
     } else {
+      if (illegalPatternIdentifiers.contains(variableName)) {
+        reportRecoverableError(token,
+            codes.templateIllegalPatternVariableName.withArguments(token));
+      }
       if (isBareIdentifier) {
         listener.handleNoType(token);
       }

diff --git a/pkg/_fe_analyzer_shared/lib/src/parser/type_info.dart b/pkg/_fe_analyzer_shared/lib/src/parser/type_info.dart
@@ -433,6 +433,11 @@ TypeParamOrArgInfo computeMethodTypeArguments(Token token) {
       : noTypeParamOrArg;
 }
 
+/// The set of identifiers that are illegal to use as the name of a variable in
+/// a variable pattern, or as the name of an identifier in an identifier
+/// pattern.
+const Set<String> illegalPatternIdentifiers = {'when', 'as'};
+
 /// Indicates whether the given [token] is allowed to follow a list of type
 /// arguments used as a selector after an expression.
 ///

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
@@ -2686,6 +2686,12 @@ ParserErrorCode.GETTER_WITH_PARAMETERS:
   status: hasFix
 ParserErrorCode.ILLEGAL_ASSIGNMENT_TO_NON_ASSIGNABLE:
   status: needsEvaluation
+ParserErrorCode.ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME:
+  status: needsEvaluation
+ParserErrorCode.ILLEGAL_PATTERN_IDENTIFIER_NAME:
+  status: needsEvaluation
+ParserErrorCode.ILLEGAL_PATTERN_VARIABLE_NAME:
+  status: needsEvaluation
 ParserErrorCode.IMPLEMENTS_BEFORE_EXTENDS:
   status: needsEvaluation
 ParserErrorCode.IMPLEMENTS_BEFORE_ON:

diff --git a/pkg/analyzer/lib/src/dart/error/syntactic_errors.g.dart b/pkg/analyzer/lib/src/dart/error/syntactic_errors.g.dart
@@ -168,6 +168,9 @@ final fastaAnalyzerErrorCodes = <ErrorCode?>[
   ParserErrorCode.LATE_PATTERN_VARIABLE_DECLARATION,
   ParserErrorCode.PATTERN_VARIABLE_DECLARATION_OUTSIDE_FUNCTION_OR_METHOD,
   ParserErrorCode.DEFAULT_IN_SWITCH_EXPRESSION,
+  ParserErrorCode.ILLEGAL_PATTERN_VARIABLE_NAME,
+  ParserErrorCode.ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME,
+  ParserErrorCode.ILLEGAL_PATTERN_IDENTIFIER_NAME,
 ];
 
 class ParserErrorCode extends ErrorCode {
@@ -896,6 +899,32 @@ class ParserErrorCode extends ErrorCode {
     "Illegal assignment to non-assignable expression.",
   );
 
+  ///  Parameters:
+  ///  0: the illegal name
+  static const ParserErrorCode ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME =
+      ParserErrorCode(
+    'ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME',
+    "A variable assigned by a pattern assignment can't be named '{0}'.",
+    correctionMessage: "Choose a different name.",
+  );
+
+  ///  Parameters:
+  ///  0: the illegal name
+  static const ParserErrorCode ILLEGAL_PATTERN_IDENTIFIER_NAME =
+      ParserErrorCode(
+    'ILLEGAL_PATTERN_IDENTIFIER_NAME',
+    "A pattern can't refer to an identifier named '{0}'.",
+    correctionMessage: "Match the identifier using '==",
+  );
+
+  ///  Parameters:
+  ///  0: the illegal name
+  static const ParserErrorCode ILLEGAL_PATTERN_VARIABLE_NAME = ParserErrorCode(
+    'ILLEGAL_PATTERN_VARIABLE_NAME',
+    "The variable declared by a variable pattern can't be named '{0}'.",
+    correctionMessage: "Choose a different name.",
+  );
+
   static const ParserErrorCode IMPLEMENTS_BEFORE_EXTENDS = ParserErrorCode(
     'IMPLEMENTS_BEFORE_EXTENDS',
     "The extends clause must be before the implements clause.",

diff --git a/pkg/analyzer/lib/src/error/error_code_values.g.dart b/pkg/analyzer/lib/src/error/error_code_values.g.dart
@@ -722,6 +722,9 @@ const List<ErrorCode> errorCodeValues = [
   ParserErrorCode.GETTER_IN_FUNCTION,
   ParserErrorCode.GETTER_WITH_PARAMETERS,
   ParserErrorCode.ILLEGAL_ASSIGNMENT_TO_NON_ASSIGNABLE,
+  ParserErrorCode.ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME,
+  ParserErrorCode.ILLEGAL_PATTERN_IDENTIFIER_NAME,
+  ParserErrorCode.ILLEGAL_PATTERN_VARIABLE_NAME,
   ParserErrorCode.IMPLEMENTS_BEFORE_EXTENDS,
   ParserErrorCode.IMPLEMENTS_BEFORE_ON,
   ParserErrorCode.IMPLEMENTS_BEFORE_WITH,

diff --git a/pkg/analyzer/test/generated/patterns_parser_test.dart b/pkg/analyzer/test/generated/patterns_parser_test.dart
@@ -21,6 +21,44 @@ main() {
 class PatternsTest extends ParserDiagnosticsTest {
   late FindNode findNode;
 
+  test_assignedVariable_namedAs() {
+    _parse('''
+void f(x) {
+  dynamic as;
+  (as) = x;
+}
+''', errors: [
+      error(ParserErrorCode.ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME, 29, 2)
+    ]);
+    var node = findNode.singlePatternAssignment.pattern;
+    assertParsedNodeText(node, r'''
+ParenthesizedPattern
+  leftParenthesis: (
+  pattern: AssignedVariablePattern
+    name: as
+  rightParenthesis: )
+''');
+  }
+
+  test_assignedVariable_namedWhen() {
+    _parse('''
+void f(x) {
+  dynamic when;
+  (when) = x;
+}
+''', errors: [
+      error(ParserErrorCode.ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME, 31, 4)
+    ]);
+    var node = findNode.singlePatternAssignment.pattern;
+    assertParsedNodeText(node, r'''
+ParenthesizedPattern
+  leftParenthesis: (
+  pattern: AssignedVariablePattern
+    name: when
+  rightParenthesis: )
+''');
+  }
+
   test_case_identifier_dot_incomplete() {
     // Based on the repro from
     // https://github.com/Dart-Code/Dart-Code/issues/4407.
@@ -1305,6 +1343,38 @@ ConstantPattern
 ''');
   }
 
+  test_constant_identifier_namedAs() {
+    _parse('''
+void f(x) {
+  switch (x) {
+    case as:
+  }
+}
+''', errors: [error(ParserErrorCode.ILLEGAL_PATTERN_IDENTIFIER_NAME, 36, 2)]);
+    var node = findNode.singleGuardedPattern.pattern;
+    assertParsedNodeText(node, r'''
+ConstantPattern
+  expression: SimpleIdentifier
+    token: as
+''');
+  }
+
+  test_constant_identifier_namedWhen() {
+    _parse('''
+void f(x) {
+  switch (x) {
+    case when:
+  }
+}
+''', errors: [error(ParserErrorCode.ILLEGAL_PATTERN_IDENTIFIER_NAME, 36, 4)]);
+    var node = findNode.singleGuardedPattern.pattern;
+    assertParsedNodeText(node, r'''
+ConstantPattern
+  expression: SimpleIdentifier
+    token: when
+''');
+  }
+
   test_constant_identifier_prefixed_builtin() {
     _parse('''
 void f(x) {
@@ -10022,6 +10092,38 @@ NullCheckPattern
 ''');
   }
 
+  test_variable_namedAs() {
+    _parse('''
+void f(x) {
+  switch (x) {
+    case var as:
+  }
+}
+''', errors: [error(ParserErrorCode.ILLEGAL_PATTERN_VARIABLE_NAME, 40, 2)]);
+    var node = findNode.singleGuardedPattern.pattern;
+    assertParsedNodeText(node, r'''
+DeclaredVariablePattern
+  keyword: var
+  name: as
+''');
+  }
+
+  test_variable_namedWhen() {
+    _parse('''
+void f(x) {
+  switch (x) {
+    case var when:
+  }
+}
+''', errors: [error(ParserErrorCode.ILLEGAL_PATTERN_VARIABLE_NAME, 40, 4)]);
+    var node = findNode.singleGuardedPattern.pattern;
+    assertParsedNodeText(node, r'''
+DeclaredVariablePattern
+  keyword: var
+  name: when
+''');
+  }
+
   test_variable_type_record_empty_inDeclarationContext() {
     _parse('''
 void f(x) {

diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml
@@ -6719,6 +6719,54 @@ VariablePatternKeywordInDeclarationContext:
     }
     ```
 
+IllegalPatternVariableName:
+  problemMessage: The variable declared by a variable pattern can't be named '#lexeme'.
+  correctionMessage: Choose a different name.
+  analyzerCode: ParserErrorCode.ILLEGAL_PATTERN_VARIABLE_NAME
+  index: 154
+  experiments: patterns
+  comment: |-
+    Parameters:
+    0: the illegal name
+  script: |
+    void f(x) {
+      switch (x) {
+        case var when:
+      }
+    }
+
+IllegalPatternAssignmentVariableName:
+  problemMessage: A variable assigned by a pattern assignment can't be named '#lexeme'.
+  correctionMessage: Choose a different name.
+  analyzerCode: ParserErrorCode.ILLEGAL_PATTERN_ASSIGNMENT_VARIABLE_NAME
+  index: 155
+  experiments: patterns
+  comment: |-
+    Parameters:
+    0: the illegal name
+  script: |
+    void f(x) {
+      dynamic when;
+      (when) = x;
+    }
+
+IllegalPatternIdentifierName:
+  problemMessage: A pattern can't refer to an identifier named '#lexeme'.
+  correctionMessage: Match the identifier using '== #lexeme'.
+  analyzerCode: ParserErrorCode.ILLEGAL_PATTERN_IDENTIFIER_NAME
+  index: 156
+  experiments: patterns
+  comment: |-
+    Parameters:
+    0: the illegal name
+  script: |
+    void f(x) {
+      const when = 0;
+      switch (x) {
+        case when:
+      }
+    }
+
 InvalidInsideUnaryPattern:
   problemMessage: This pattern cannot appear inside a unary pattern (cast pattern, null check pattern, or null assert pattern) without parentheses.
   correctionMessage: Try combining into a single pattern if possible, or enclose the inner pattern in parentheses.

diff --git a/pkg/front_end/parser_testcases/patterns/assignedVariable_namedAs.dart b/pkg/front_end/parser_testcases/patterns/assignedVariable_namedAs.dart
@@ -0,0 +1,4 @@
+void f(x) {
+  dynamic as;
+  (as) = x;
+}