Parser: Add assertions to check skipOuterPattern behaviour

The behaviour `skipOuterPattern` needs to be kept in sync with that of
`parsePrimaryPattern`.  To reduce the risk of these to going out of
sync, I've added logic to `parsePrimaryPattern` so that after parsing
an `outerPattern`, it asserts that `skipOuterPattern` would have
behaved appropriately.

This required adding some logic to `parser_test_parser.dart` so that
these calls to `skipOuterPattern` don't show up in front_end parser
expectations files.  (If they did show up, they would lead to
failures, since the bots run the front_end parser tests with
assertions both enabled and disabled).

Bug: #50035
Change-Id: Ida3a37532bb9a86837e70b5ec1aa2e557e3cb769
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271163
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: stereotype441

pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart

@@ -335,6 +335,17 @@ class Parser {
       {this.useImplicitCreationExpression = true, this.allowPatterns = false})
       : assert(listener != null); // ignore:unnecessary_null_comparison
 
+  /// Executes [callback]; however if `this` is the `TestParser` (from
+  /// `pkg/front_end/test/parser_test_parser.dart`) then no output is printed
+  /// during its execution.
+  ///
+  /// This is sometimes necessary inside `assert` statements, to ensure that the
+  /// output of `TestParser` is the same regardless of whether assertions are
+  /// enabled.
+  T inhibitPrinting<T>(T Function() callback) {
+    return callback();
+  }
+
   bool get inGenerator {
     return asyncState == AsyncModifier.AsyncStar ||
         asyncState == AsyncModifier.SyncStar;
@@ -9271,6 +9282,7 @@ class Parser {
   ///                   | 'const' '(' expression ')'
   /// objectPattern ::= typeName typeArguments? '(' patternFields? ')'
   Token parsePrimaryPattern(Token token, {required bool isRefutableContext}) {
+    Token start = token;
     TypeParamOrArgInfo typeArg =
         computeTypeParamOrArg(token, /* inDeclaration = */ true);
     Token next = typeArg.skip(token).next!;
@@ -9279,15 +9291,25 @@ class Parser {
       case '[':
         // listPattern ::= typeArguments? '[' patterns? ']'
         token = typeArg.parseArguments(token, this);
-        return parseListPatternSuffix(token,
+        token = parseListPatternSuffix(token,
             isRefutableContext: isRefutableContext);
+        // A list pattern is a valid form of outerPattern, so verify that
+        // skipOuterPattern would have skipped this pattern properly.
+        assert(
+            identical(inhibitPrinting(() => skipOuterPattern(start)), token));
+        return token;
       case '{':
         // mapPattern        ::= typeArguments? '{' mapPatternEntries? '}'
         // mapPatternEntries ::= mapPatternEntry ( ',' mapPatternEntry )* ','?
         // mapPatternEntry   ::= expression ':' pattern
         token = typeArg.parseArguments(token, this);
-        return parseMapPatternSuffix(token,
+        token = parseMapPatternSuffix(token,
             isRefutableContext: isRefutableContext);
+        // A map pattern is a valid form of outerPattern, so verify that
+        // skipOuterPattern would have skipped this pattern properly.
+        assert(
+            identical(inhibitPrinting(() => skipOuterPattern(start)), token));
+        return token;
     }
     // Whatever was after the optional type arguments didn't parse as a pattern
     // that can start with type arguments, so back up and reparse assuming that
@@ -9306,11 +9328,17 @@ class Parser {
         Token nextNext = next.next!;
         if (optional(')', nextNext)) {
           listener.handleRecordPattern(next, /* count = */ 0);
-          return nextNext;
+          token = nextNext;
         } else {
-          return parseParenthesizedPatternOrRecordPattern(token,
+          token = parseParenthesizedPatternOrRecordPattern(token,
               isRefutableContext: isRefutableContext);
         }
+        // A record or parenthesized pattern is a valid form of outerPattern, so
+        // verify that skipOuterPattern would have skipped this pattern
+        // properly.
+        assert(
+            identical(inhibitPrinting(() => skipOuterPattern(start)), token));
+        return token;
       case 'const':
         // constantPattern ::= booleanLiteral
         //                   | nullLiteral
@@ -9380,6 +9408,10 @@ class Parser {
         token = parseObjectPatternRest(token,
             isRefutableContext: isRefutableContext);
         listener.handleObjectPattern(firstIdentifier, dot, secondIdentifier);
+        // An object pattern is a valid form of outerPattern, so verify that
+        // skipOuterPattern would have skipped this pattern properly.
+        assert(
+            identical(inhibitPrinting(() => skipOuterPattern(start)), token));
         return token;
       } else if (dot == null) {
         // It's a single identifier.  If it's a wildcard pattern or we're in an

pkg/front_end/test/parser_test_parser.dart

@@ -30,6 +30,7 @@ class TestParser extends Parser {
   int indent = 0;
   StringBuffer sb = new StringBuffer();
   final bool trace;
+  bool _inhibitPrinting = false;
 
   TestParser(Listener listener, this.trace, {required bool allowPatterns})
       : super(listener,
@@ -51,11 +52,23 @@ class TestParser extends Parser {
   }
 
   void doPrint(String s) {
+    if (_inhibitPrinting) return;
     String traceString = "";
     if (trace) traceString = " (${createTrace()})";
     sb.writeln(("  " * indent) + s + traceString);
   }
 
+  @override
+  T inhibitPrinting<T>(T Function() callback) {
+    bool previousInhibitPrinting = _inhibitPrinting;
+    _inhibitPrinting = true;
+    try {
+      return callback();
+    } finally {
+      _inhibitPrinting = previousInhibitPrinting;
+    }
+  }
+
   @override
   Uri? get uri {
     doPrint('uri()');

pkg/front_end/test/parser_test_parser_creator.dart

@@ -67,6 +67,7 @@ class TestParser extends Parser {
   int indent = 0;
   StringBuffer sb = new StringBuffer();
   final bool trace;
+  bool _inhibitPrinting = false;
 
   TestParser(Listener listener, this.trace, {required bool allowPatterns})
       : super(listener,
@@ -88,11 +89,22 @@ class TestParser extends Parser {
   }
 
   void doPrint(String s) {
+    if (_inhibitPrinting) return;
     String traceString = "";
     if (trace) traceString = " (${createTrace()})";
     sb.writeln(("  " * indent) + s + traceString);
   }
 
+  @override
+  T inhibitPrinting<T>(T Function() callback) {
+    bool previousInhibitPrinting = _inhibitPrinting;
+    _inhibitPrinting = true;
+    try {
+      return callback();
+    } finally {
+      _inhibitPrinting = previousInhibitPrinting;
+    }
+  }
 """);
 
   ParserCreatorListener listener = new ParserCreatorListener(out);
@@ -154,7 +166,9 @@ class ParserCreatorListener extends Listener {
   @override
   void endClassMethod(Token? getOrSet, Token beginToken, Token beginParam,
       Token? beginInitializers, Token endToken) {
-    if (insideParserClass && !currentMethodName!.startsWith("_")) {
+    if (insideParserClass &&
+        !currentMethodName!.startsWith("_") &&
+        currentMethodName != 'inhibitPrinting') {
       Token token = beginToken;
       Token? latestToken;
       out.writeln("  @override");

pkg/front_end/test/spell_checking_list_code.txt

@@ -687,6 +687,7 @@ inferable
 influence
 informative
 infos
+inhibit
 init
 initializations
 initializer's

pkg/front_end/test/spell_checking_list_tests.txt

@@ -358,6 +358,7 @@ incrementally
 increments
 indents
 ing
+inhibit
 inlinable
 inlineable
 insights