improve extension message missing body error message

This improves the missing class, mixin, and extension method body
error messages and lays the groundwork for improving other
missing body error messages such as switch statement, finally clause,
and others.

Change-Id: I0a4d1338fe91eee3cfe7d61652e168df1302d212
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103570
Reviewed-by: Brian Wilkerson <[email protected]>

Author: Dan Rubel

pkg/analyzer/lib/error/error.dart

@@ -423,6 +423,7 @@ const List<ErrorCode> errorCodeValues = const [
   ParserErrorCode.EMPTY_ENUM_BODY,
   ParserErrorCode.ENUM_IN_CLASS,
   ParserErrorCode.EQUALITY_CANNOT_BE_EQUALITY_OPERAND,
+  ParserErrorCode.EXPECTED_BODY,
   ParserErrorCode.EXPECTED_CASE_OR_DEFAULT,
   ParserErrorCode.EXPECTED_CLASS_MEMBER,
   ParserErrorCode.EXPECTED_ELSE_OR_COMMA,

pkg/analyzer/lib/src/dart/error/syntactic_errors.dart

@@ -161,6 +161,8 @@ class ParserErrorCode extends ErrorCode {
   static const ParserErrorCode EQUALITY_CANNOT_BE_EQUALITY_OPERAND =
       _EQUALITY_CANNOT_BE_EQUALITY_OPERAND;
 
+  static const ParserErrorCode EXPECTED_BODY = _EXPECTED_BODY;
+
   static const ParserErrorCode EXPECTED_CASE_OR_DEFAULT = const ParserErrorCode(
       'EXPECTED_CASE_OR_DEFAULT', "Expected 'case' or 'default'.",
       correction: "Try placing this code inside a case clause.");
@@ -415,11 +417,7 @@ class ParserErrorCode extends ErrorCode {
   static const ParserErrorCode MISSING_CATCH_OR_FINALLY =
       _MISSING_CATCH_OR_FINALLY;
 
-  /// TODO(danrubel): Consider splitting this into two separate error messages.
-  static const ParserErrorCode MISSING_CLASS_BODY = const ParserErrorCode(
-      'MISSING_CLASS_BODY',
-      "A class or mixin definition must have a body, even if it is empty.",
-      correction: "Try adding a body to your class or mixin.");
+  static const ParserErrorCode MISSING_CLASS_BODY = _EXPECTED_BODY;
 
   static const ParserErrorCode MISSING_CLOSING_PARENTHESIS =
       const ParserErrorCode(

pkg/analyzer/lib/src/dart/error/syntactic_errors.g.dart

@@ -103,6 +103,7 @@ final fastaAnalyzerErrorCodes = <ErrorCode>[
   _EXPERIMENT_NOT_ENABLED,
   _EXPECTED_ELSE_OR_COMMA,
   _INVALID_SUPER_IN_INITIALIZER,
+  _EXPECTED_BODY,
 ];
 
 const ParserErrorCode _ABSTRACT_CLASS_MEMBER = const ParserErrorCode(
@@ -227,6 +228,10 @@ const ParserErrorCode _EQUALITY_CANNOT_BE_EQUALITY_OPERAND = const ParserErrorCo
     r"An equality expression can't be an operand of another equality expression.",
     correction: "Try re-writing the expression.");
 
+const ParserErrorCode _EXPECTED_BODY = const ParserErrorCode(
+    'EXPECTED_BODY', r"A #string must have a body, even if it is empty.",
+    correction: "Try adding an empty body.");
+
 const ParserErrorCode _EXPECTED_ELSE_OR_COMMA = const ParserErrorCode(
     'EXPECTED_ELSE_OR_COMMA', r"Expected 'else' or comma.");
 

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

@@ -205,10 +205,6 @@ class FastaErrorReporter {
         errorReporter?.reportErrorForOffset(
             StrongModeCode.INVALID_SUPER_INVOCATION, offset, length);
         return;
-      case "MISSING_CLASS_BODY":
-        errorReporter?.reportErrorForOffset(
-            ParserErrorCode.MISSING_CLASS_BODY, offset, length);
-        return;
       case "MISSING_DIGIT":
         errorReporter?.reportErrorForOffset(
             ScannerErrorCode.MISSING_DIGIT, offset, length);

pkg/analyzer/test/generated/parser_test.dart

@@ -4413,7 +4413,7 @@ class Wrong<T> {
 
   void test_missingClassBody() {
     parseCompilationUnit("class A class B {}",
-        errors: [expectedError(ParserErrorCode.MISSING_CLASS_BODY, 8, 5)]);
+        errors: [expectedError(ParserErrorCode.MISSING_CLASS_BODY, 6, 1)]);
   }
 
   void test_missingClosingParenthesis() {

pkg/analyzer/test/src/task/options_test.dart

@@ -240,6 +240,7 @@ class ErrorCodeValuesTest {
         declaredNames.remove('CONST_AND_COVARIANT');
         declaredNames.remove('CONST_AND_VAR');
         declaredNames.remove('COVARIANT_AFTER_FINAL');
+        declaredNames.remove('MISSING_CLASS_BODY');
       }
 
       // Assert that all remaining declared names are in errorCodeValues

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

@@ -2913,27 +2913,6 @@ Message _withArgumentsExpectedButGot(String string) {
       arguments: {'string': string});
 }
 
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Template<Message Function(Token token)> templateExpectedClassBodyToSkip =
-    const Template<Message Function(Token token)>(
-        messageTemplate:
-            r"""Expected a class or mixin body, but got '#lexeme'.""",
-        withArguments: _withArgumentsExpectedClassBodyToSkip);
-
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Code<Message Function(Token token)> codeExpectedClassBodyToSkip =
-    const Code<Message Function(Token token)>(
-        "ExpectedClassBodyToSkip", templateExpectedClassBodyToSkip,
-        analyzerCodes: <String>["MISSING_CLASS_BODY"]);
-
-// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-Message _withArgumentsExpectedClassBodyToSkip(Token token) {
-  String lexeme = token.lexeme;
-  return new Message(codeExpectedClassBodyToSkip,
-      message: """Expected a class or mixin body, but got '${lexeme}'.""",
-      arguments: {'token': token});
-}
-
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<Message Function(Token token)> templateExpectedClassMember =
     const Template<Message Function(Token token)>(
@@ -2955,24 +2934,27 @@ Message _withArgumentsExpectedClassMember(Token token) {
 }
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Template<Message Function(Token token)> templateExpectedClassOrMixinBody =
-    const Template<Message Function(Token token)>(
+const Template<Message Function(String string)>
+    templateExpectedClassOrMixinBody =
+    const Template<Message Function(String string)>(
         messageTemplate:
-            r"""Expected a class or mixin body, but got '#lexeme'.""",
+            r"""A #string must have a body, even if it is empty.""",
+        tipTemplate: r"""Try adding an empty body.""",
         withArguments: _withArgumentsExpectedClassOrMixinBody);
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-const Code<Message Function(Token token)> codeExpectedClassOrMixinBody =
-    const Code<Message Function(Token token)>(
+const Code<Message Function(String string)> codeExpectedClassOrMixinBody =
+    const Code<Message Function(String string)>(
         "ExpectedClassOrMixinBody", templateExpectedClassOrMixinBody,
-        analyzerCodes: <String>["MISSING_CLASS_BODY"]);
+        index: 96);
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
-Message _withArgumentsExpectedClassOrMixinBody(Token token) {
-  String lexeme = token.lexeme;
+Message _withArgumentsExpectedClassOrMixinBody(String string) {
+  if (string.isEmpty) throw 'No string provided';
   return new Message(codeExpectedClassOrMixinBody,
-      message: """Expected a class or mixin body, but got '${lexeme}'.""",
-      arguments: {'token': token});
+      message: """A ${string} must have a body, even if it is empty.""",
+      tip: """Try adding an empty body.""",
+      arguments: {'string': string});
 }
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.

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

@@ -1654,7 +1654,7 @@ class Parser {
 
   Token skipBlock(Token token) {
     // The scanner ensures that `{` always has a closing `}`.
-    return ensureBlock(token, null).endGroup;
+    return ensureBlock(token, null, null).endGroup;
   }
 
   /// ```
@@ -1713,7 +1713,8 @@ class Parser {
         }
       }
     } else {
-      leftBrace = ensureBlock(token, fasta.templateExpectedEnumBody);
+      // TODO(danrubel): merge this error message with missing class/mixin body
+      leftBrace = ensureBlock(token, fasta.templateExpectedEnumBody, null);
       token = leftBrace.endGroup;
     }
     assert(optional('}', token));
@@ -1772,7 +1773,7 @@ class Parser {
     if (!optional('{', token.next)) {
       // Recovery
       token = parseClassHeaderRecovery(start, begin, classKeyword);
-      ensureBlock(token, fasta.templateExpectedClassOrMixinBody);
+      ensureBlock(token, null, 'class declaration');
     }
     token = parseClassOrMixinBody(token);
     listener.endClassDeclaration(begin, token);
@@ -1938,7 +1939,7 @@ class Parser {
     if (!optional('{', token.next)) {
       // Recovery
       token = parseMixinHeaderRecovery(token, mixinKeyword, headerStart);
-      ensureBlock(token, fasta.templateExpectedClassOrMixinBody);
+      ensureBlock(token, null, 'mixin declaration');
     }
     token = parseClassOrMixinBody(token);
     listener.endMixinDeclaration(mixinKeyword, token);
@@ -2078,8 +2079,7 @@ class Parser {
     TypeInfo typeInfo = computeType(onKeyword, true);
     token = typeInfo.ensureTypeNotVoid(onKeyword, this);
     if (!optional('{', token.next)) {
-      // TODO(danrubel): replace this with a better error message.
-      ensureBlock(token, fasta.templateExpectedClassOrMixinBody);
+      ensureBlock(token, null, 'extension declaration');
     }
     // TODO(danrubel): Do not allow fields or constructors
     token = parseClassOrMixinBody(token);
@@ -2701,17 +2701,28 @@ class Parser {
   }
 
   /// If the next token is an opening curly brace, return it. Otherwise, use the
-  /// given [template] to report an error, insert an opening and a closing curly
-  /// brace, and return the newly inserted opening curly brace. If the
-  /// [template] is `null`, use a default error message instead.
-  Token ensureBlock(
-      Token token, Template<Message Function(Token token)> template) {
+  /// given [template] or [blockKind] to report an error, insert an opening and
+  /// a closing curly brace, and return the newly inserted opening curly brace.
+  /// If  [template] and [blockKind] are `null`, then use
+  /// a default error message instead.
+  Token ensureBlock(Token token,
+      Template<Message Function(Token token)> template, String blockKind) {
     Token next = token.next;
     if (optional('{', next)) return next;
-    Message message = template == null
-        ? fasta.templateExpectedButGot.withArguments('{')
-        : template.withArguments(next);
-    reportRecoverableError(next, message);
+    if (template == null) {
+      if (blockKind == null) {
+        // TODO(danrubel): rename ExpectedButGot to ExpectedBefore
+        reportRecoverableError(
+            next, fasta.templateExpectedButGot.withArguments('{'));
+      } else {
+        // TODO(danrubel): rename ExpectedClassOrMixinBody
+        //  to ExpectedDeclarationOrClauseBody
+        reportRecoverableError(token,
+            fasta.templateExpectedClassOrMixinBody.withArguments(blockKind));
+      }
+    } else {
+      reportRecoverableError(next, template.withArguments(next));
+    }
     return insertBlock(token);
   }
 
@@ -2842,7 +2853,7 @@ class Parser {
 
   Token skipClassOrMixinBody(Token token) {
     // The scanner ensures that `{` always has a closing `}`.
-    return ensureBlock(token, fasta.templateExpectedClassOrMixinBody);
+    return ensureBlock(token, null, null);
   }
 
   /// ```
@@ -3556,7 +3567,7 @@ class Parser {
         begin = next = token.next;
         // Fall through to parse the block.
       } else {
-        token = ensureBlock(token, fasta.templateExpectedFunctionBody);
+        token = ensureBlock(token, fasta.templateExpectedFunctionBody, null);
         listener.handleInvalidFunctionBody(token);
         return token.endGroup;
       }
@@ -3678,7 +3689,8 @@ class Parser {
     }
     final value = token.next.stringValue;
     if (identical(value, '{')) {
-      return parseBlock(token);
+      // The scanner ensures that `{` always has a closing `}`.
+      return parseBlock(token, null);
     } else if (identical(value, 'return')) {
       return parseReturnStatement(token);
     } else if (identical(value, 'var') || identical(value, 'final')) {
@@ -5631,8 +5643,8 @@ class Parser {
   ///   '{' statement* '}'
   /// ;
   /// ```
-  Token parseBlock(Token token) {
-    Token begin = token = ensureBlock(token, null);
+  Token parseBlock(Token token, String blockKind) {
+    Token begin = token = ensureBlock(token, null, blockKind);
     listener.beginBlock(begin);
     int statementCount = 0;
     Token startToken = token.next;
@@ -5661,7 +5673,8 @@ class Parser {
     // because an error has already been reported by the caller.
     Listener originalListener = listener;
     listener = new ForwardingListener(listener)..forwardErrors = false;
-    token = parseBlock(token);
+    // The scanner ensures that `{` always has a closing `}`.
+    token = parseBlock(token, null);
     listener = originalListener;
     listener.handleInvalidTopLevelBlock(begin);
     return token;
@@ -5776,7 +5789,8 @@ class Parser {
     Token tryKeyword = token.next;
     assert(optional('try', tryKeyword));
     listener.beginTryStatement(tryKeyword);
-    Token lastConsumed = parseBlock(tryKeyword);
+    // TODO(danrubel): pass 'try statement' for blockKind and update tests
+    Token lastConsumed = parseBlock(tryKeyword, null);
     token = lastConsumed.next;
     int catchCount = 0;
 
@@ -5871,7 +5885,8 @@ class Parser {
         token = lastConsumed.next;
       }
       listener.endCatchClause(token);
-      lastConsumed = parseBlock(lastConsumed);
+      // TODO(danrubel): pass 'catch clause' for blockKind and update tests
+      lastConsumed = parseBlock(lastConsumed, null);
       token = lastConsumed.next;
       ++catchCount;
       listener.handleCatchBlock(onKeyword, catchKeyword, comma);
@@ -5881,7 +5896,8 @@ class Parser {
     Token finallyKeyword = null;
     if (optional('finally', token)) {
       finallyKeyword = token;
-      lastConsumed = parseBlock(token);
+      // TODO(danrubel): pass 'finally clause' for blockKind and update tests
+      lastConsumed = parseBlock(token, null);
       token = lastConsumed.next;
       listener.handleFinallyBlock(finallyKeyword);
     } else {
@@ -5919,7 +5935,8 @@ class Parser {
   /// ;
   /// ```
   Token parseSwitchBlock(Token token) {
-    Token beginSwitch = token = ensureBlock(token, null);
+    // TODO(danrubel): pass 'switch statement' for blockKind and update tests
+    Token beginSwitch = token = ensureBlock(token, null, null);
     listener.beginSwitchBlock(beginSwitch);
     int caseCount = 0;
     Token defaultKeyword = null;

pkg/front_end/messages.yaml

@@ -413,10 +413,10 @@ ImplementsFutureOr:
       class A implements FutureOr<int> {}
 
 ExpectedClassOrMixinBody:
-  template: "Expected a class or mixin body, but got '#lexeme'."
-  analyzerCode: MISSING_CLASS_BODY
-
-ExpectedClassBodyToSkip: ExpectedClassOrMixinBody
+  index: 96
+  template: "A #string must have a body, even if it is empty."
+  tip: "Try adding an empty body."
+  analyzerCode: ParserErrorCode.EXPECTED_BODY
 
 ExpectedDeclaration:
   template: "Expected a declaration, but got '#lexeme'."

pkg/front_end/testcases/regress/issue_36647.dart.legacy.expect

@@ -19,9 +19,10 @@ library;
 //
 // Problems in library:
 //
-// pkg/front_end/testcases/regress/issue_36647_lib2.dart:5:11: Error: Expected a class or mixin body, but got 'xx'.
+// pkg/front_end/testcases/regress/issue_36647_lib2.dart:5:7: Error: A class declaration must have a body, even if it is empty.
+// Try adding an empty body.
 // class xxx xx XXX extends XXX {
-//           ^^
+//       ^^^
 //
 // pkg/front_end/testcases/regress/issue_36647_lib2.dart:5:14: Error: Expected ';' after this.
 // class xxx xx XXX extends XXX {