Update fasta parser to report ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR

In addition to adding missing errors, this CL inlines modifier parsing
as part of a multi step effort to parse modifiers once and provide
better error recovery for class member and top level declarations.

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

pkg/analyzer/lib/src/fasta/error_converter.dart

@@ -235,6 +235,10 @@ class FastaErrorReporter {
         errorReporter?.reportErrorForOffset(
             StaticWarningCode.FINAL_NOT_INITIALIZED, offset, length, [name]);
         return;
+      case "FUNCTION_TYPED_PARAMETER_VAR":
+        errorReporter?.reportErrorForOffset(
+            ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, offset, length);
+        return;
       case "GETTER_WITH_PARAMETERS":
         errorReporter?.reportErrorForOffset(
             ParserErrorCode.GETTER_WITH_PARAMETERS, offset, length);

pkg/analyzer/test/generated/parser_fasta_test.dart

@@ -387,14 +387,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_fieldInitializerOutsideConstructor();
   }
 
-  @override
-  @failingTest
-  void test_functionTypedParameter_final() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, found 0
-    super.test_functionTypedParameter_final();
-  }
-
   @override
   @failingTest
   void test_functionTypedParameter_incomplete1() {
@@ -415,14 +407,6 @@ class ErrorParserTest_Fasta extends FastaParserTestCase
     super.test_functionTypedParameter_incomplete1();
   }
 
-  @override
-  @failingTest
-  void test_functionTypedParameter_var() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 1 errors of type ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, found 0
-    super.test_functionTypedParameter_var();
-  }
-
   @override
   @failingTest
   void test_getterInFunction_block_noReturnType() {
@@ -2129,36 +2113,6 @@ class FastaParserTestCase extends Object
 @reflectiveTest
 class FormalParameterParserTest_Fasta extends FastaParserTestCase
     with FormalParameterParserTestMixin {
-  @override
-  @failingTest
-  void test_parseNormalFormalParameter_field_const_noType() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 0 errors of type ParserErrorCode.EXTRANEOUS_MODIFIER, found 1 (1)
-    super.test_parseNormalFormalParameter_field_const_noType();
-  }
-
-  @failingTest
-  void test_parseNormalFormalParameter_field_const_noType2() {
-    // TODO(danrubel): should not be generating an error
-    super.test_parseNormalFormalParameter_field_const_noType();
-    assertNoErrors();
-  }
-
-  @override
-  @failingTest
-  void test_parseNormalFormalParameter_field_const_type() {
-    // TODO(brianwilkerson) Wrong errors:
-    // Expected 0 errors of type ParserErrorCode.EXTRANEOUS_MODIFIER, found 1 (1)
-    super.test_parseNormalFormalParameter_field_const_type();
-  }
-
-  @failingTest
-  void test_parseNormalFormalParameter_field_const_type2() {
-    // TODO(danrubel): should not be generating an error
-    super.test_parseNormalFormalParameter_field_const_type();
-    assertNoErrors();
-  }
-
   @override
   void test_parseNormalFormalParameter_function_noType_typeParameterComments() {
     // Ignored: Fasta does not support the generic comment syntax.

pkg/analyzer/test/generated/parser_test.dart

@@ -3480,16 +3480,20 @@ class Foo {
   void test_functionTypedParameter_const() {
     parseCompilationUnit("void f(const x()) {}",
         errors: usingFastaParser
-            ? [expectedError(ParserErrorCode.EXTRANEOUS_MODIFIER, 7, 5)]
+            ? [
+                expectedError(ParserErrorCode.EXTRANEOUS_MODIFIER, 7, 5),
+                expectedError(
+                    ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, 7, 5)
+              ]
             : [
                 expectedError(
-                    ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, 7, 9)
+                    ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, 7, 5)
               ]);
   }
 
   void test_functionTypedParameter_final() {
     parseCompilationUnit("void f(final x()) {}", errors: [
-      expectedError(ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, 7, 9)
+      expectedError(ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, 7, 5)
     ]);
   }
 
@@ -3516,7 +3520,7 @@ class Foo {
 
   void test_functionTypedParameter_var() {
     parseCompilationUnit("void f(var x()) {}", errors: [
-      expectedError(ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, 7, 7)
+      expectedError(ParserErrorCode.FUNCTION_TYPED_PARAMETER_VAR, 7, 3)
     ]);
   }
 
@@ -8815,8 +8819,9 @@ abstract class FormalParameterParserTestMixin
   }
 
   void test_parseNormalFormalParameter_field_const_noType() {
-    NormalFormalParameter parameter =
+    NormalFormalParameter parameter = parseNormalFormalParameter('const this.a',
-        parseNormalFormalParameter('const this.a');
+        errorCodes:
+            usingFastaParser ? [ParserErrorCode.EXTRANEOUS_MODIFIER] : []);
     expect(parameter, isNotNull);
     expect(parameter, new isInstanceOf<FieldFormalParameter>());
     FieldFormalParameter fieldParameter = parameter;
@@ -8827,15 +8832,11 @@ abstract class FormalParameterParserTestMixin
   }
 
   void test_parseNormalFormalParameter_field_const_type() {
-    NormalFormalParameter parameter =
+    NormalFormalParameter parameter = parseNormalFormalParameter(
-        parseNormalFormalParameter('const A this.a');
+        'const A this.a',
+        errorCodes:
+            usingFastaParser ? [ParserErrorCode.EXTRANEOUS_MODIFIER] : []);
     expect(parameter, isNotNull);
-    if (usingFastaParser) {
-      // TODO(danrubel): should not be generating an error
-      assertErrorsWithCodes([ParserErrorCode.EXTRANEOUS_MODIFIER]);
-    } else {
-      assertNoErrors();
-    }
     expect(parameter, new isInstanceOf<FieldFormalParameter>());
     FieldFormalParameter fieldParameter = parameter;
     expect(fieldParameter.keyword, isNotNull);

pkg/compiler/lib/src/diagnostics/messages.dart

@@ -1486,40 +1486,12 @@ main() => foo(42);
       MessageKind.FINAL_FUNCTION_TYPE_PARAMETER: const MessageTemplate(
           MessageKind.FINAL_FUNCTION_TYPE_PARAMETER,
           "A function type parameter can't be declared final.",
-          howToFix: "Try removing 'final'.",
+          howToFix: "Try removing 'final'."),
-          examples: const [
-            """
-foo(final int x(int a)) {}
-main() => foo((y) => 42);
-""",
-            """
-foo({final int x(int a)}) {}
-main() => foo((y) => 42);
-""",
-            """
-foo([final int x(int a)]) {}
-main() => foo((y) => 42);
-"""
-          ]),
 
       MessageKind.VAR_FUNCTION_TYPE_PARAMETER: const MessageTemplate(
           MessageKind.VAR_FUNCTION_TYPE_PARAMETER,
           "A function type parameter can't be declared with 'var'.",
-          howToFix: "Try removing 'var'.",
+          howToFix: "Try removing 'var'."),
-          examples: const [
-            """
-foo(var int x(int a)) {}
-main() => foo((y) => 42);
-""",
-            """
-foo({var int x(int a)}) {}
-main() => foo((y) => 42);
-""",
-            """
-foo([var int x(int a)]) {}
-main() => foo((y) => 42);
-"""
-          ]),
 
       MessageKind.CANNOT_INSTANTIATE_TYPE_VARIABLE: const MessageTemplate(
           MessageKind.CANNOT_INSTANTIATE_TYPE_VARIABLE,

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

@@ -1934,6 +1934,19 @@ const MessageCode messageFunctionTypeDefaultValue = const MessageCode(
     dart2jsCode: "*ignored*",
     message: r"""Can't have a default value in a function type.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Null> codeFunctionTypedParameterVar =
+    messageFunctionTypedParameterVar;
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const MessageCode messageFunctionTypedParameterVar = const MessageCode(
+    "FunctionTypedParameterVar",
+    analyzerCode: "FUNCTION_TYPED_PARAMETER_VAR",
+    dart2jsCode: "*fatal*",
+    message:
+        r"""Function-typed parameters can't specify 'const', 'final' or 'var' in place of a return type.""",
+    tip: r"""Try replacing the keyword with a return type.""");
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeGeneratorReturnsValue = messageGeneratorReturnsValue;
 

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

@@ -74,6 +74,22 @@ Token skipToLastModifier(Token token) {
   return token;
 }
 
+TypeContinuation typeContinuationAfterVar(TypeContinuation typeContinuation) {
+  switch (typeContinuation ?? TypeContinuation.Required) {
+    case TypeContinuation.NormalFormalParameter:
+      return TypeContinuation.NormalFormalParameterAfterVar;
+
+    case TypeContinuation.OptionalPositionalFormalParameter:
+      return TypeContinuation.OptionalPositionalFormalParameterAfterVar;
+
+    case TypeContinuation.NamedFormalParameter:
+      return TypeContinuation.NamedFormalParameterAfterVar;
+
+    default:
+      return TypeContinuation.OptionalAfterVar;
+  }
+}
+
 TypeContinuation typeContinuationFromMemberKind(
         bool isVarAllowed, MemberKind memberKind) =>
     (isVarAllowed || memberKind == MemberKind.GeneralizedFunctionType)
@@ -109,6 +125,7 @@ class ModifierContext {
       memberKind != MemberKind.NonStaticField;
 
   Token parseOpt(Token token) {
+    assert(lastModifier != null);
     if (token != lastModifier) {
       if (optional('external', token.next)) {
         token = parseExternalOpt(token);
@@ -268,24 +285,7 @@ class ModifierContext {
           next, fasta.templateExtraneousModifier);
       return next;
     }
-    switch (typeContinuation ?? TypeContinuation.Required) {
+    typeContinuation = typeContinuationAfterVar(typeContinuation);
-      case TypeContinuation.NormalFormalParameter:
-        typeContinuation = TypeContinuation.NormalFormalParameterAfterVar;
-        break;
-
-      case TypeContinuation.OptionalPositionalFormalParameter:
-        typeContinuation =
-            TypeContinuation.OptionalPositionalFormalParameterAfterVar;
-        break;
-
-      case TypeContinuation.NamedFormalParameter:
-        typeContinuation = TypeContinuation.NamedFormalParameterAfterVar;
-        break;
-
-      default:
-        typeContinuation = TypeContinuation.OptionalAfterVar;
-        break;
-    }
     varFinalOrConst ??= next;
     modifierCount++;
     return parser.parseModifier(token);
@@ -306,7 +306,7 @@ class ModifierRecoveryContext extends ModifierContext {
       FormalParameterKind parameterKind,
       bool isVarAllowed,
       TypeContinuation typeContinuation,
-      Token lastModifier)
+      [Token lastModifier])
       : super(parser, memberKind, parameterKind, isVarAllowed, typeContinuation,
             lastModifier);
 
@@ -344,6 +344,52 @@ class ModifierRecoveryContext extends ModifierContext {
     return token;
   }
 
+  Token parseRecovery(Token token,
+      {Token covariantToken, Token varFinalOrConst}) {
+    if (covariantToken != null) {
+      this.covariantToken = covariantToken;
+      ++modifierCount;
+    }
+    if (varFinalOrConst != null) {
+      ++modifierCount;
+      if (optional('var', varFinalOrConst)) {
+        varToken = varFinalOrConst;
+      } else if (optional('final', varFinalOrConst)) {
+        finalToken = varFinalOrConst;
+      } else if (optional('const', varFinalOrConst)) {
+        constToken = varFinalOrConst;
+      } else {
+        throw "Internal error: Unexpected varFinalOrConst '$varFinalOrConst'.";
+      }
+    }
+
+    // Process invalid and out-of-order modifiers
+    Token next = token.next;
+    while (isModifier(next)) {
+      final value = next.stringValue;
+      if (identical('abstract', value)) {
+        token = parseAbstract(token);
+      } else if (identical('const', value)) {
+        token = parseConst(token);
+      } else if (identical('covariant', value)) {
+        token = parseCovariantOpt(token);
+      } else if (identical('external', value)) {
+        token = parseExternalOpt(token);
+      } else if (identical('final', value)) {
+        token = parseFinal(token);
+      } else if (identical('static', value)) {
+        token = parseStaticOpt(token);
+      } else if (identical('var', value)) {
+        token = parseVar(token);
+      } else {
+        token = parseExtraneousModifier(token);
+      }
+      next = token.next;
+    }
+
+    return token;
+  }
+
   Token parseAbstract(Token token) {
     assert(optional('abstract', token.next));
     if (memberKind == MemberKind.NonStaticField ||

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

@@ -79,10 +79,12 @@ import 'modifier_context.dart'
         ClassMethodModifierContext,
         FactoryModifierContext,
         ModifierContext,
+        ModifierRecoveryContext,
         TopLevelMethodModifierContext,
         isModifier,
         parseModifiersOpt,
         skipToLastModifier,
+        typeContinuationAfterVar,
         typeContinuationFromMemberKind;
 
 import 'recovery_listeners.dart'
@@ -1152,31 +1154,59 @@ class Parser {
   /// ```
   Token parseFormalParameter(
       Token token, FormalParameterKind parameterKind, MemberKind memberKind) {
+    assert(parameterKind != null);
     token = parseMetadataStar(token);
     Token next = token.next;
     listener.beginFormalParameter(next, memberKind);
 
     TypeContinuation typeContinuation =
         typeContinuationFromFormalParameterKind(parameterKind);
+    Token varFinalOrConst;
     if (isModifier(next)) {
-      ModifierContext modifierContext = parseModifiersOpt(
+      int modifierCount = 0;
-          this,
+      Token covariantToken;
-          token,
+      if (optional('covariant', next)) {
-          skipToLastModifier(token),
+        if (memberKind != MemberKind.StaticMethod &&
-          memberKind,
+            memberKind != MemberKind.TopLevelMethod) {
-          parameterKind,
+          covariantToken = token = parseModifier(token);
-          false,
+          ++modifierCount;
-          typeContinuation);
+          next = token.next;
-      typeContinuation = modifierContext.typeContinuation;
+        }
-      memberKind = modifierContext.memberKind;
+      }
-      token = modifierContext.lastModifier;
+
-      modifierContext = null;
+      if (isModifier(next)) {
+        if (optional('var', next)) {
+          typeContinuation = typeContinuationAfterVar(typeContinuation);
+          varFinalOrConst = token = parseModifier(token);
+          ++modifierCount;
+          next = token.next;
+        } else if (optional('final', next)) {
+          varFinalOrConst = token = parseModifier(token);
+          ++modifierCount;
+          next = token.next;
+        }
+
+        if (isModifier(next)) {
+          // Recovery
+          ModifierRecoveryContext modifierContext = new ModifierRecoveryContext(
+              this, memberKind, parameterKind, false, typeContinuation);
+          token = modifierContext.parseRecovery(token,
+              covariantToken: covariantToken, varFinalOrConst: varFinalOrConst);
+
+          memberKind = modifierContext.memberKind;
+          typeContinuation = modifierContext.typeContinuation;
+          varFinalOrConst = modifierContext.varFinalOrConst;
+          modifierCount = modifierContext.modifierCount;
+          modifierContext = null;
+        }
+      }
+      listener.handleModifiers(modifierCount);
     } else {
       listener.handleModifiers(0);
-      typeContinuation ??= typeContinuationFromMemberKind(false, memberKind);
     }
 
-    return parseType(token, typeContinuation, null, memberKind);
+    return parseType(
+        token, typeContinuation, null, memberKind, varFinalOrConst);
   }
 
   /// ```
@@ -2061,7 +2091,8 @@ class Parser {
   Token parseType(Token token,
       [TypeContinuation continuation = TypeContinuation.Required,
       IdentifierContext continuationContext,
-      MemberKind memberKind]) {
+      MemberKind memberKind,
+      Token varFinalOrConst]) {
     /// True if we've seen the `var` keyword.
     bool hasVar = false;
 
@@ -2580,12 +2611,20 @@ class Parser {
           Token closer = closeBraceTokenFor(token);
           if (closer != null) {
             if (optional("(", closer.next)) {
+              if (varFinalOrConst != null) {
+                reportRecoverableError(
+                    varFinalOrConst, fasta.messageFunctionTypedParameterVar);
+              }
               inlineFunctionTypeStart = beforeToken;
               beforeToken = token;
               token = token.next;
             }
           }
         } else if (optional("(", token)) {
+          if (varFinalOrConst != null) {
+            reportRecoverableError(
+                varFinalOrConst, fasta.messageFunctionTypedParameterVar);
+          }
           inlineFunctionTypeStart = beforeToken;
           beforeToken = closeBraceTokenFor(token);
           token = beforeToken.next;

pkg/front_end/messages.yaml

@@ -278,6 +278,16 @@ MissingConstFinalVarOrType:
   script:
     - "class C { static f; }"
 
+FunctionTypedParameterVar:
+  template: "Function-typed parameters can't specify 'const', 'final' or 'var' in place of a return type."
+  tip: "Try replacing the keyword with a return type."
+  analyzerCode: FUNCTION_TYPED_PARAMETER_VAR
+  dart2jsCode: "*fatal*"
+  script:
+    - "void f(const x()) {}"
+    - "void f(final x()) {}"
+    - "void f(var x()) {}"
+
 AbstractClassMember:
   template: "Members of classes can't be declared to be 'abstract'."
   tip: "Try removing the 'abstract' keyword. You can add the 'abstract' keyword before the class declaration."

tests/co19/co19-kernel.status

@@ -153,8 +153,6 @@ Language/Expressions/Type_Test/evaluation_t10: RuntimeError # Kernel Issue 28335
 Language/Functions/External_Functions/not_connected_to_a_body_t01: RuntimeError # Dartk Issue 28565
 Language/Functions/Formal_Parameters/Optional_Formals/default_value_t01: MissingCompileTimeError
 Language/Functions/Formal_Parameters/Optional_Formals/default_value_t02: MissingCompileTimeError
-Language/Functions/Formal_Parameters/Required_Formals/syntax_t06: MissingCompileTimeError
-Language/Functions/Formal_Parameters/Required_Formals/syntax_t07: MissingCompileTimeError
 Language/Libraries_and_Scripts/Exports/reexport_t01: MissingCompileTimeError
 Language/Libraries_and_Scripts/Imports/deferred_import_t01: CompileTimeError # Deferred loading kernel issue 28335.
 Language/Libraries_and_Scripts/Imports/deferred_import_t02: CompileTimeError # Deferred loading kernel issue 28335.