Add fasta parser formal parameter recovery

Change-Id: I1f7772634b583ba798ea933a7876f8a520b9f414
Reviewed-on: https://dart-review.googlesource.com/22680
Commit-Queue: Dan Rubel <danrubel@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analyzer/test/generated/parser_fasta_test.dart

@@ -15,6 +15,7 @@ import 'package:front_end/src/fasta/fasta_codes.dart'
     show LocatedMessage, Message;
 import 'package:front_end/src/fasta/kernel/kernel_builder.dart';
 import 'package:front_end/src/fasta/kernel/kernel_library_builder.dart';
+import 'package:front_end/src/fasta/scanner/error_token.dart' show ErrorToken;
 import 'package:front_end/src/fasta/scanner/string_scanner.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -414,33 +415,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_expectedTypeName_is_void();
   }
 
-  @override
-  @failingTest
-  void test_extraCommaInParameterList() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.MISSING_IDENTIFIER, found 0;
-    // 1 errors of type ParserErrorCode.EXPECTED_TOKEN, found 0
-    super.test_extraCommaInParameterList();
-  }
-
-  @override
-  @failingTest
-  void test_extraCommaTrailingNamedParameterGroup() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, found 0;
-    // 1 errors of type ParserErrorCode.MISSING_IDENTIFIER, found 0
-    super.test_extraCommaTrailingNamedParameterGroup();
-  }
-
-  @override
-  @failingTest
-  void test_extraCommaTrailingPositionalParameterGroup() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, found 0;
-    // 1 errors of type ParserErrorCode.MISSING_IDENTIFIER, found 0
-    super.test_extraCommaTrailingPositionalParameterGroup();
-  }
-
   @override
   @failingTest
   void test_factoryWithInitializers() {
@@ -511,15 +485,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_functionTypedParameter_var();
   }
 
-  @override
-  @failingTest
-  void test_genericFunctionType_extraLessThan() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.UNEXPECTED_TOKEN, found 0;
-    // 0 errors of type ParserErrorCode.EXTRANEOUS_MODIFIER, found 1 (52)
-    super.test_genericFunctionType_extraLessThan();
-  }
-
   @override
   @failingTest
   void test_getterInFunction_block_noReturnType() {
@@ -1191,14 +1156,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_missingAssignableSelector_superPropertyAccessAssigned();
   }
 
-  @override
-  @failingTest
-  void test_missingClosingParenthesis() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ScannerErrorCode.EXPECTED_TOKEN, found 0
-    super.test_missingClosingParenthesis();
-  }
-
   @override
   @failingTest
   void test_missingEnumBody() {
@@ -1381,22 +1338,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_missingVariableInForEach();
   }
 
-  @override
-  @failingTest
-  void test_mixedParameterGroups_namedPositional() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.MIXED_PARAMETER_GROUPS, found 0
-    super.test_mixedParameterGroups_namedPositional();
-  }
-
-  @override
-  @failingTest
-  void test_mixedParameterGroups_positionalNamed() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.MIXED_PARAMETER_GROUPS, found 0
-    super.test_mixedParameterGroups_positionalNamed();
-  }
-
   @override
   @failingTest
   void test_mixin_application_lacks_with_clause() {
@@ -1405,22 +1346,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_mixin_application_lacks_with_clause();
   }
 
-  @override
-  @failingTest
-  void test_multipleNamedParameterGroups() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.MULTIPLE_NAMED_PARAMETER_GROUPS, found 0
-    super.test_multipleNamedParameterGroups();
-  }
-
-  @override
-  @failingTest
-  void test_multiplePositionalParameterGroups() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.MULTIPLE_POSITIONAL_PARAMETER_GROUPS, found 0
-    super.test_multiplePositionalParameterGroups();
-  }
-
   @override
   @failingTest
   void test_multipleVariablesInForEach() {
@@ -1545,46 +1470,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     expectNotNullIfNoErrors(member);
   }
 
-  @override
-  @failingTest
-  void test_optionalAfterNormalParameters_named() {
-    // TODO(brianwilkerson) Does not recover.
-    //   type 'FormalParameterListImpl' is not a subtype of type 'TypeParameterList' of 'typeParameters' where
-    //   FormalParameterListImpl is from package:analyzer/src/dart/ast/ast.dart
-    //   TypeParameterList is from package:analyzer/dart/ast/ast.dart
-    //
-    //   package:analyzer/src/fasta/ast_builder.dart 1122:40                AstBuilder.endTopLevelMethod
-    //   package:front_end/src/fasta/parser/parser.dart 1741:14             Parser.parseTopLevelMethod
-    //   package:front_end/src/fasta/parser/parser.dart 1646:11             Parser.parseTopLevelMember
-    //   package:front_end/src/fasta/parser/parser.dart 298:14              Parser._parseTopLevelDeclaration
-    //   package:front_end/src/fasta/parser/parser.dart 263:13              Parser.parseTopLevelDeclaration
-    //   package:front_end/src/fasta/parser/parser.dart 252:15              Parser.parseUnit
-    //   package:analyzer/src/generated/parser_fasta.dart 77:33             _Parser2.parseCompilationUnit2
-    //   package:analyzer/src/generated/parser_fasta.dart 72:12             _Parser2.parseCompilationUnit
-    //   test/generated/parser_fasta_test.dart 3189:35                      FastaParserTestCase.parseCompilationUnit
-    super.test_optionalAfterNormalParameters_named();
-  }
-
-  @override
-  @failingTest
-  void test_optionalAfterNormalParameters_positional() {
-    // TODO(brianwilkerson) Does not recover.
-    //   type 'FormalParameterListImpl' is not a subtype of type 'TypeParameterList' of 'typeParameters' where
-    //   FormalParameterListImpl is from package:analyzer/src/dart/ast/ast.dart
-    //   TypeParameterList is from package:analyzer/dart/ast/ast.dart
-    //
-    //   package:analyzer/src/fasta/ast_builder.dart 1122:40                AstBuilder.endTopLevelMethod
-    //   package:front_end/src/fasta/parser/parser.dart 1741:14             Parser.parseTopLevelMethod
-    //   package:front_end/src/fasta/parser/parser.dart 1646:11             Parser.parseTopLevelMember
-    //   package:front_end/src/fasta/parser/parser.dart 298:14              Parser._parseTopLevelDeclaration
-    //   package:front_end/src/fasta/parser/parser.dart 263:13              Parser.parseTopLevelDeclaration
-    //   package:front_end/src/fasta/parser/parser.dart 252:15              Parser.parseUnit
-    //   package:analyzer/src/generated/parser_fasta.dart 77:33             _Parser2.parseCompilationUnit2
-    //   package:analyzer/src/generated/parser_fasta.dart 72:12             _Parser2.parseCompilationUnit
-    //   test/generated/parser_fasta_test.dart 3189:35                      FastaParserTestCase.parseCompilationUnit
-    super.test_optionalAfterNormalParameters_positional();
-  }
-
   @override
   @failingTest
   void test_parseCascadeSection_missingIdentifier() {
@@ -1743,18 +1628,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_switchHasMultipleDefaultCases_repeated();
   }
 
-  @override
-  @failingTest
-  void test_topLevelOperator_withoutType() {
-    super.test_topLevelOperator_withoutType();
-  }
-
-  @override
-  @failingTest
-  void test_topLevelOperator_withVoid() {
-    super.test_topLevelOperator_withVoid();
-  }
-
   @override
   @failingTest
   void test_topLevelVariable_withMetadata() {
@@ -1795,14 +1668,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_unexpectedTerminatorForParameterGroup_named();
   }
 
-  @override
-  @failingTest
-  void test_unexpectedTerminatorForParameterGroup_optional() {
-    // TODO(brianwilkerson) Wrong errors:
-    //Expected 1 errors of type ParserErrorCode.UNEXPECTED_TERMINATOR_FOR_PARAMETER_GROUP, found 0
-    super.test_unexpectedTerminatorForParameterGroup_optional();
-  }
-
   @override
   @failingTest
   void test_unexpectedToken_endOfFieldDeclarationStatement() {
@@ -2746,11 +2611,13 @@ class ParserProxy extends analyzer.ParserAdapter {
     _eventListener.begin(enclosingEvent);
     var result = f();
     _eventListener.end(enclosingEvent);
+    String lexeme = currentToken is ErrorToken
+        ? currentToken.runtimeType.toString()
+        : currentToken.lexeme;
     if (expectedEndOffset == null) {
-      expect(currentToken.isEof, isTrue, reason: currentToken.lexeme);
+      expect(currentToken.isEof, isTrue, reason: lexeme);
     } else {
-      expect(currentToken.offset, expectedEndOffset,
-          reason: currentToken.lexeme);
+      expect(currentToken.offset, expectedEndOffset, reason: lexeme);
     }
     expect(astBuilder.stack, hasLength(0));
     expect(astBuilder.directives, hasLength(0));

pkg/analyzer/test/generated/parser_test.dart

@@ -3166,30 +3166,41 @@ class Foo {
     createParser('(int a, , int b)');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors([
-      expectedError(ParserErrorCode.MISSING_IDENTIFIER, 8, 1),
-      expectedError(ParserErrorCode.EXPECTED_TOKEN, 8, 1)
-    ]);
+    listener.assertErrors(usingFastaParser
+        ? [
+            expectedError(ParserErrorCode.MISSING_IDENTIFIER, 8, 1),
+            expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 10, 3)
+          ]
+        : [
+            expectedError(ParserErrorCode.MISSING_IDENTIFIER, 8, 1),
+            expectedError(ParserErrorCode.EXPECTED_TOKEN, 8, 1)
+          ]);
   }
 
   void test_extraCommaTrailingNamedParameterGroup() {
     createParser('({int b},)');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors([
-      expectedError(ParserErrorCode.MISSING_IDENTIFIER, 9, 1),
-      expectedError(ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 9, 1)
-    ]);
+    listener.assertErrors(usingFastaParser
+        ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 8, 1)]
+        : [
+            expectedError(ParserErrorCode.MISSING_IDENTIFIER, 9, 1),
+            expectedError(
+                ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 9, 1)
+          ]);
   }
 
   void test_extraCommaTrailingPositionalParameterGroup() {
     createParser('([int b],)');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors([
-      expectedError(ParserErrorCode.MISSING_IDENTIFIER, 9, 1),
-      expectedError(ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 9, 1)
-    ]);
+    listener.assertErrors(usingFastaParser
+        ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 8, 1)]
+        : [
+            expectedError(ParserErrorCode.MISSING_IDENTIFIER, 9, 1),
+            expectedError(
+                ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 9, 1)
+          ]);
   }
 
   void test_extraTrailingCommaInParameterList() {
@@ -3866,17 +3877,20 @@ class Wrong<T> {
         errors: [expectedError(ParserErrorCode.MISSING_CLASS_BODY, 8, 5)]);
   }
 
-  @failingTest
   void test_missingClosingParenthesis() {
     // It is possible that it is not possible to generate this error (that it's
     // being reported in code that cannot actually be reached), but that hasn't
     // been proven yet.
-    createParser('(int a, int b ;');
+    createParser('(int a, int b ;',
+        expectedEndOffset: 0 /* ErrorToken at end of token stream */);
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    if (fe.Scanner.useFasta) {
+    if (usingFastaParser) {
       listener.assertErrors(
-          [expectedError(ScannerErrorCode.EXPECTED_TOKEN, 14, 1)]);
+          [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 14, 1)]);
+    } else if (fe.Scanner.useFasta) {
+      listener.errors
+          .contains(expectedError(ParserErrorCode.EXPECTED_TOKEN, 14, 1));
     } else {
       listener
           .assertErrorsWithCodes([ParserErrorCode.MISSING_CLOSING_PARENTHESIS]);
@@ -4237,16 +4251,18 @@ class Wrong<T> {
     createParser('(a, {b}, [c])');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors(
-        [expectedError(ParserErrorCode.MIXED_PARAMETER_GROUPS, 9, 3)]);
+    listener.assertErrors(usingFastaParser
+        ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 7, 1)]
+        : [expectedError(ParserErrorCode.MIXED_PARAMETER_GROUPS, 9, 3)]);
   }
 
   void test_mixedParameterGroups_positionalNamed() {
     createParser('(a, [b], {c})');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors(
-        [expectedError(ParserErrorCode.MIXED_PARAMETER_GROUPS, 9, 3)]);
+    listener.assertErrors(usingFastaParser
+        ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 7, 1)]
+        : [expectedError(ParserErrorCode.MIXED_PARAMETER_GROUPS, 9, 3)]);
   }
 
   void test_mixin_application_lacks_with_clause() {
@@ -4276,8 +4292,11 @@ class Wrong<T> {
     createParser('(a, {b}, {c})');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors(
-        [expectedError(ParserErrorCode.MULTIPLE_NAMED_PARAMETER_GROUPS, 9, 3)]);
+    listener.assertErrors(usingFastaParser
+        ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 7, 1)]
+        : [
+            expectedError(ParserErrorCode.MULTIPLE_NAMED_PARAMETER_GROUPS, 9, 3)
+          ]);
   }
 
   void test_multiplePartOfDirectives() {
@@ -4290,9 +4309,12 @@ class Wrong<T> {
     createParser('(a, [b], [c])');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors([
-      expectedError(ParserErrorCode.MULTIPLE_POSITIONAL_PARAMETER_GROUPS, 9, 3)
-    ]);
+    listener.assertErrors(usingFastaParser
+        ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 7, 1)]
+        : [
+            expectedError(
+                ParserErrorCode.MULTIPLE_POSITIONAL_PARAMETER_GROUPS, 9, 3)
+          ]);
   }
 
   void test_multipleVariablesInForEach() {
@@ -4381,15 +4403,23 @@ class Wrong<T> {
   }
 
   void test_optionalAfterNormalParameters_named() {
-    parseCompilationUnit("f({a}, b) {}", errors: [
-      expectedError(ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 7, 1)
-    ]);
+    parseCompilationUnit("f({a}, b) {}",
+        errors: usingFastaParser
+            ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 5, 1)]
+            : [
+                expectedError(
+                    ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 7, 1)
+              ]);
   }
 
   void test_optionalAfterNormalParameters_positional() {
-    parseCompilationUnit("f([a], b) {}", errors: [
-      expectedError(ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 7, 1)
-    ]);
+    parseCompilationUnit("f([a], b) {}",
+        errors: usingFastaParser
+            ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 5, 1)]
+            : [
+                expectedError(
+                    ParserErrorCode.NORMAL_BEFORE_OPTIONAL_PARAMETERS, 7, 1)
+              ]);
   }
 
   void test_parseCascadeSection_missingIdentifier() {
@@ -4665,10 +4695,12 @@ m() {
     CompilationUnitMember member = parseFullCompilationUnitMember();
     expectNotNullIfNoErrors(member);
     if (usingFastaParser) {
-      listener.assertErrors([
-        expectedError(ParserErrorCode.MISSING_IDENTIFIER, 0, 0),
-        expectedError(ParserErrorCode.TOP_LEVEL_OPERATOR, 0, 8)
-      ]);
+      listener.assertErrors(usingFastaParser
+          ? [expectedError(ParserErrorCode.TOP_LEVEL_OPERATOR, 0, 8)]
+          : [
+              expectedError(ParserErrorCode.MISSING_IDENTIFIER, 0, 0),
+              expectedError(ParserErrorCode.TOP_LEVEL_OPERATOR, 0, 8)
+            ]);
     } else {
       listener.assertErrorsWithCodes([ParserErrorCode.TOP_LEVEL_OPERATOR]);
     }
@@ -4693,11 +4725,16 @@ m() {
     CompilationUnitMember member = parseFullCompilationUnitMember();
     expectNotNullIfNoErrors(member);
     if (usingFastaParser) {
-      listener.assertErrors([
-        expectedError(ParserErrorCode.EXTRANEOUS_MODIFIER, 0, 0),
-        expectedError(ParserErrorCode.MISSING_IDENTIFIER, 0, 0),
-        expectedError(ParserErrorCode.TOP_LEVEL_OPERATOR, 5, 8)
-      ]);
+      listener.assertErrors(usingFastaParser
+          ? [
+              expectedError(ParserErrorCode.EXTRANEOUS_MODIFIER, 0, 4),
+              expectedError(ParserErrorCode.TOP_LEVEL_OPERATOR, 5, 8)
+            ]
+          : [
+              expectedError(ParserErrorCode.EXTRANEOUS_MODIFIER, 0, 0),
+              expectedError(ParserErrorCode.MISSING_IDENTIFIER, 0, 0),
+              expectedError(ParserErrorCode.TOP_LEVEL_OPERATOR, 5, 8)
+            ]);
     } else {
       listener.assertErrorsWithCodes([ParserErrorCode.TOP_LEVEL_OPERATOR]);
     }
@@ -4763,10 +4800,12 @@ main() {
     createParser('(a, b])');
     FormalParameterList list = parser.parseFormalParameterList();
     expectNotNullIfNoErrors(list);
-    listener.assertErrors([
-      expectedError(
-          ParserErrorCode.UNEXPECTED_TERMINATOR_FOR_PARAMETER_GROUP, 5, 1)
-    ]);
+    listener.assertErrors(usingFastaParser
+        ? [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 5, 1)]
+        : [
+            expectedError(
+                ParserErrorCode.UNEXPECTED_TERMINATOR_FOR_PARAMETER_GROUP, 5, 1)
+          ]);
   }
 
   void test_unexpectedToken_endOfFieldDeclarationStatement() {

pkg/front_end/lib/src/fasta/parser/parser.dart

@@ -1022,7 +1022,7 @@ class Parser {
     assert(optional('(', token));
     listener.beginFormalParameters(begin, kind);
     int parameterCount = 0;
-    do {
+    while (true) {
       Token next = token.next;
       if (optional(')', next)) {
         token = next;
@@ -1031,22 +1031,30 @@ class Parser {
       ++parameterCount;
       String value = next.stringValue;
       if (identical(value, '[')) {
-        token = parseOptionalPositionalParameters(token, kind).next;
+        token = parseOptionalPositionalParameters(token, kind);
+        token = ensureCloseParen(token, begin);
         break;
       } else if (identical(value, '{')) {
-        token = parseOptionalNamedParameters(token, kind).next;
+        token = parseOptionalNamedParameters(token, kind);
+        token = ensureCloseParen(token, begin);
         break;
       } else if (identical(value, '[]')) {
         --parameterCount;
         reportRecoverableError(next, fasta.messageEmptyOptionalParameterList);
-        token = next.next;
+        token = ensureCloseParen(next, begin);
         break;
       }
-      token =
-          parseFormalParameter(token, FormalParameterKind.mandatory, kind).next;
-    } while (optional(',', token));
+      token = parseFormalParameter(token, FormalParameterKind.mandatory, kind);
+      next = token.next;
+      if (optional(',', next)) {
+        token = next;
+        continue;
+      }
+      token = ensureCloseParen(token, begin);
+      break;
+    }
+    assert(optional(')', token));
     listener.endFormalParameters(parameterCount, begin, token, kind);
-    expect(')', token);
     return token;
   }
 
@@ -2987,6 +2995,25 @@ class Parser {
     return token;
   }
 
+  /// If the next token is a closing parenthesis, return it.
+  /// Otherwise, report an error and return the closing parenthesis
+  /// associated with the specified open parenthesis.
+  Token ensureCloseParen(Token token, Token openParen) {
+    Token next = token.next;
+    if (optional(')', next)) {
+      return next;
+    }
+
+    // TODO(danrubel): Pass in context for better error message.
+    reportRecoverableError(
+        next, fasta.templateExpectedButGot.withArguments(')'));
+
+    // Scanner guarantees a closing parenthesis
+    // TODO(danrubel): Improve recovery by having callers parse tokens
+    // between `token` and `openParen.endGroup`.
+    return openParen.endGroup;
+  }
+
   /// If the next token is a colon, return it. Otherwise, report an
   /// error, insert a synthetic colon, and return the inserted colon.
   Token ensureColon(Token token) {