[messages] Prepare to add shared messages.yaml file.

Updates the code in `pkg/analyzer_utilities/lib/messages.dart` to
attempt to read CFE messages from both
`pkg/_fe_analyzer_shared/messages.yaml` and
`pkg/front_end/messages.yaml`, and updates clients accordingly.

Also updates the `messages_suite.dart` test and the presubmit rules to
ensure that the contents of `pkg/_fe_analyzer_shared/messages.yaml`
will be appropriately tested.

Since the file `pkg/_fe_analyzer_shared/messages.yaml` doesn't exist
yet, temporaryhacks have been added to pretend the file is empty if it
can't be found.

In a follow-up CL, I will move messages that are shared between the
analyzer and the CFE to `pkg/_fe_analyzer_shared/messages.yaml`.

Change-Id: I6a6a6964c1c02f20df9ae8e34f23e734bbe88c22
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448605
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

pkg/analyzer/test/src/fasta/message_coverage_test.dart

@@ -38,7 +38,7 @@ class AbstractRecoveryTest extends FastaParserTestCase {
   /// Return a list of the front end messages that define an 'analyzerCode'.
   List<String> getMappedCodes() {
     Set<String> codes = <String>{};
-    for (var entry in frontEndMessages.entries) {
+    for (var entry in frontEndAndSharedMessages.entries) {
       var name = entry.key;
       var errorCodeInfo = entry.value;
       if (errorCodeInfo.analyzerCode.isNotEmpty) {
@@ -52,7 +52,7 @@ class AbstractRecoveryTest extends FastaParserTestCase {
   /// `messages.yaml` file.
   List<String> getReferencedCodes() {
     Set<String> codes = <String>{};
-    for (var errorCodeInfo in frontEndMessages.values) {
+    for (var errorCodeInfo in frontEndAndSharedMessages.values) {
       codes.addAll(errorCodeInfo.analyzerCode);
     }
     return codes.toList();

pkg/analyzer/tool/messages/error_code_info.dart

@@ -168,7 +168,7 @@ final String analyzerPkgPath = normalize(
 
 /// A set of tables mapping between front end and analyzer error codes.
 final CfeToAnalyzerErrorCodeTables cfeToAnalyzerErrorCodeTables =
-    CfeToAnalyzerErrorCodeTables._(frontEndMessages);
+    CfeToAnalyzerErrorCodeTables._(frontEndAndSharedMessages);
 
 /// The path to the `linter` package.
 final String linterPkgPath = normalize(join(pkg_root.packageRoot, 'linter'));
@@ -334,7 +334,7 @@ class CfeToAnalyzerErrorCodeTables {
   /// automatically generated, and whose values are the front end error name.
   final Map<ErrorCodeInfo, String> infoToFrontEndCode = {};
 
-  CfeToAnalyzerErrorCodeTables._(Map<String, FrontEndErrorCodeInfo> messages) {
+  CfeToAnalyzerErrorCodeTables._(Map<String, CfeStyleErrorCodeInfo> messages) {
     for (var entry in messages.entries) {
       var errorCodeInfo = entry.value;
       var index = errorCodeInfo.index;

pkg/analyzer_utilities/lib/messages.dart

@@ -20,9 +20,29 @@ const Map<String, String> severityEnumNames = <String, String>{
   'INFO': 'info',
 };
 
+/// Decoded messages from the `_fe_analyzer_shared` package's `messages.yaml`
+/// file.
+final Map<String, CfeStyleErrorCodeInfo> feAnalyzerSharedMessages =
+    _loadCfeStyleMessages(
+      feAnalyzerSharedPkgPath,
+      allowNonExistent: true,
+      isShared: true,
+    );
+
+/// The path to the `fe_analyzer_shared` package.
+final String feAnalyzerSharedPkgPath = normalize(
+  join(pkg_root.packageRoot, '_fe_analyzer_shared'),
+);
+
+/// Decoded messages from the `messages.yaml` files in the front end and
+/// `_fe_analyzer_shared`.
+final Map<String, CfeStyleErrorCodeInfo> frontEndAndSharedMessages = Map.from(
+  frontEndMessages,
+)..addAll(feAnalyzerSharedMessages);
+
 /// Decoded messages from the front end's `messages.yaml` file.
-final Map<String, FrontEndErrorCodeInfo> frontEndMessages =
-    _loadFrontEndMessages();
+final Map<String, CfeStyleErrorCodeInfo> frontEndMessages =
+    _loadCfeStyleMessages(frontEndPkgPath, isShared: false);
 
 /// The path to the `front_end` package.
 final String frontEndPkgPath = normalize(
@@ -51,14 +71,17 @@ String convertTemplate(Map<String, int> placeholderToIndexMap, String entry) {
   );
 }
 
-/// Decodes a YAML object (obtained from `pkg/front_end/messages.yaml`) into a
-/// map from error name to [ErrorCodeInfo].
-Map<String, FrontEndErrorCodeInfo> decodeCfeMessagesYaml(Object? yaml) {
+/// Decodes a YAML object (in CFE style `messages.yaml` format) into a map from
+/// error name to [ErrorCodeInfo].
+Map<String, CfeStyleErrorCodeInfo> decodeCfeStyleMessagesYaml(
+  Object? yaml, {
+  required bool isShared,
+}) {
   Never problem(String message) {
     throw 'Problem in pkg/front_end/messages.yaml: $message';
   }
 
-  var result = <String, FrontEndErrorCodeInfo>{};
+  var result = <String, CfeStyleErrorCodeInfo>{};
   if (yaml is! Map<Object?, Object?>) {
     problem('root node is not a map');
   }
@@ -72,7 +95,10 @@ Map<String, FrontEndErrorCodeInfo> decodeCfeMessagesYaml(Object? yaml) {
       problem('value associated with error $errorName is not a map');
     }
     try {
-      result[errorName] = FrontEndErrorCodeInfo.fromYaml(errorValue);
+      result[errorName] = CfeStyleErrorCodeInfo.fromYaml(
+        errorValue,
+        isShared: isShared,
+      );
     } catch (e, st) {
       Error.throwWithStackTrace('while processing $errorName, $e', st);
     }
@@ -80,14 +106,25 @@ Map<String, FrontEndErrorCodeInfo> decodeCfeMessagesYaml(Object? yaml) {
   return result;
 }
 
-/// Loads front end messages from the front end's `messages.yaml` file.
-Map<String, FrontEndErrorCodeInfo> _loadFrontEndMessages() {
-  var path = join(frontEndPkgPath, 'messages.yaml');
+/// Loads messages in CFE style `messages.yaml` format.
+///
+/// If [allowNonExistent] is `true`, and the `messages.yaml` file does not
+/// exist, an empty map is returned. This is a temporary measure to allow for an
+/// easier transition when the file `pkg/_fe_analyzer_shared/messages.yaml` is
+/// created.
+// TODO(paulberry): remove [allowNonExistent] once it's no longer needed.
+Map<String, CfeStyleErrorCodeInfo> _loadCfeStyleMessages(
+  String packagePath, {
+  bool allowNonExistent = false,
+  required bool isShared,
+}) {
+  var path = join(packagePath, 'messages.yaml');
+  if (allowNonExistent && !File(path).existsSync()) return {};
   Object? messagesYaml = loadYaml(
     File(path).readAsStringSync(),
     sourceUrl: Uri.file(path),
   );
-  return decodeCfeMessagesYaml(messagesYaml);
+  return decodeCfeStyleMessagesYaml(messagesYaml, isShared: isShared);
 }
 
 /// Splits [text] on spaces using the given [maxWidth] (and [firstLineWidth] if
@@ -130,6 +167,86 @@ List<String> _splitText(
   return lines;
 }
 
+/// In-memory representation of error code information obtained from a
+/// `messages.yaml` file in `pkg/front_end` or `pkg/_fe_analyzer_shared`.
+class CfeStyleErrorCodeInfo extends ErrorCodeInfo {
+  /// The set of analyzer error codes that corresponds to this error code, if
+  /// any.
+  final List<String> analyzerCode;
+
+  /// The index of the error in the analyzer's `fastaAnalyzerErrorCodes` table.
+  final int? index;
+
+  /// The name of the [CfeSeverity] constant describing this error code's CFE
+  /// severity.
+  final String? cfeSeverity;
+
+  CfeStyleErrorCodeInfo.fromYaml(YamlMap yaml, {required bool isShared})
+    : analyzerCode = _decodeAnalyzerCode(yaml['analyzerCode']),
+      index = _decodeIndex(yaml['index']),
+      cfeSeverity = _decodeSeverity(yaml['severity']),
+      super.fromYaml(yaml) {
+    if (yaml['problemMessage'] == null) {
+      throw 'Missing problemMessage';
+    }
+    if (isShared && analyzerCode.length != 1) {
+      throw StateError('Shared messages must have exactly one analyzerCode');
+    }
+  }
+
+  @override
+  Map<Object?, Object?> toYaml() => {
+    if (analyzerCode.isNotEmpty)
+      'analyzerCode': _encodeAnalyzerCode(analyzerCode),
+    if (index != null) 'index': index,
+    ...super.toYaml(),
+  };
+
+  static List<String> _decodeAnalyzerCode(Object? value) {
+    if (value == null) {
+      return const [];
+    } else if (value is String) {
+      return [value];
+    } else if (value is List) {
+      return [for (var s in value) s as String];
+    } else {
+      throw 'Unrecognized analyzer code: $value';
+    }
+  }
+
+  static int? _decodeIndex(Object? value) {
+    switch (value) {
+      case null:
+        return null;
+      case int():
+        if (value >= 1) {
+          return value;
+        }
+    }
+    throw 'Expected positive int for "index:", but found $value';
+  }
+
+  static String? _decodeSeverity(Object? yamlEntry) {
+    switch (yamlEntry) {
+      case null:
+        return null;
+      case String():
+        return severityEnumNames[yamlEntry] ??
+            (throw "Unknown severity '$yamlEntry'");
+      default:
+        throw 'Bad severity type: ${yamlEntry.runtimeType}';
+    }
+  }
+
+  static Object _encodeAnalyzerCode(List<String> analyzerCode) {
+    if (analyzerCode.length == 1) {
+      return analyzerCode.single;
+    } else {
+      return analyzerCode;
+    }
+  }
+}
+
 /// Information about how to convert the CFE's internal representation of a
 /// template parameter to a string.
 ///
@@ -688,83 +805,6 @@ enum ErrorCodeParameterType {
   bool get isSupportedByAnalyzer => _analyzerName != null;
 }
 
-/// In-memory representation of error code information obtained from the front
-/// end's `messages.yaml` file.
-class FrontEndErrorCodeInfo extends ErrorCodeInfo {
-  /// The set of analyzer error codes that corresponds to this error code, if
-  /// any.
-  final List<String> analyzerCode;
-
-  /// The index of the error in the analyzer's `fastaAnalyzerErrorCodes` table.
-  final int? index;
-
-  /// The name of the [CfeSeverity] constant describing this error code's CFE
-  /// severity.
-  final String? cfeSeverity;
-
-  FrontEndErrorCodeInfo.fromYaml(YamlMap yaml)
-    : analyzerCode = _decodeAnalyzerCode(yaml['analyzerCode']),
-      index = _decodeIndex(yaml['index']),
-      cfeSeverity = _decodeSeverity(yaml['severity']),
-      super.fromYaml(yaml) {
-    if (yaml['problemMessage'] == null) {
-      throw 'Missing problemMessage';
-    }
-  }
-
-  @override
-  Map<Object?, Object?> toYaml() => {
-    if (analyzerCode.isNotEmpty)
-      'analyzerCode': _encodeAnalyzerCode(analyzerCode),
-    if (index != null) 'index': index,
-    ...super.toYaml(),
-  };
-
-  static List<String> _decodeAnalyzerCode(Object? value) {
-    if (value == null) {
-      return const [];
-    } else if (value is String) {
-      return [value];
-    } else if (value is List) {
-      return [for (var s in value) s as String];
-    } else {
-      throw 'Unrecognized analyzer code: $value';
-    }
-  }
-
-  static int? _decodeIndex(Object? value) {
-    switch (value) {
-      case null:
-        return null;
-      case int():
-        if (value >= 1) {
-          return value;
-        }
-    }
-    throw 'Expected positive int for "index:", but found $value';
-  }
-
-  static String? _decodeSeverity(Object? yamlEntry) {
-    switch (yamlEntry) {
-      case null:
-        return null;
-      case String():
-        return severityEnumNames[yamlEntry] ??
-            (throw "Unknown severity '$yamlEntry'");
-      default:
-        throw 'Bad severity type: ${yamlEntry.runtimeType}';
-    }
-  }
-
-  static Object _encodeAnalyzerCode(List<String> analyzerCode) {
-    if (analyzerCode.length == 1) {
-      return analyzerCode.single;
-    } else {
-      return analyzerCode;
-    }
-  }
-}
-
 /// Representation of a single file containing generated error codes.
 class GeneratedErrorCodeFile {
   /// The file path (relative to the SDK's `pkg` directory) of the generated

pkg/front_end/presubmit_helper.dart

@@ -85,6 +85,7 @@ const Set<String> _generatedFilesUpToDateFiles = {
   "pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart",
   "pkg/_fe_analyzer_shared/lib/src/parser/listener.dart",
   "pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart",
+  "pkg/_fe_analyzer_shared/messages.yaml",
   "pkg/front_end/lib/src/api_prototype/experimental_flags_generated.dart",
   "pkg/front_end/lib/src/codes/cfe_codes_generated.dart",
   "pkg/front_end/lib/src/util/parser_ast_helper.dart",
@@ -201,17 +202,21 @@ LintWork? _createLintWork(List<String> changedFiles) {
   return new LintWork(filters: filters, repoDir: _repoDir);
 }
 
+final RegExp _messagesYamlPathRegExp = RegExp('^pkg/(.+)/messages.yaml\$');
+
 MessagesWork? _createMessagesTestWork(List<String> changedFiles) {
   // TODO(jensj): Could we detect what ones are changed/added and only test
   // those?
+  List<String> filters = [];
   for (String file in changedFiles) {
-    if (file == "pkg/front_end/messages.yaml") {
-      return new MessagesWork(repoDir: _repoDir);
+    if (_messagesYamlPathRegExp.matchAsPrefix(file) case var match?) {
+      filters.add('messages/${match.group(1)}/...');
     }
   }
 
-  // messages.yaml not changed.
-  return null;
+  if (filters.isEmpty) return null;
+
+  return new MessagesWork(filters: filters, repoDir: _repoDir);
 }
 
 SpellNotSourceWork? _createSpellingTestNotSourceWork(
@@ -555,9 +560,10 @@ class LintWork extends Work {
 }
 
 class MessagesWork extends Work {
+  final List<String> filters;
   final Uri repoDir;
 
-  MessagesWork({required this.repoDir});
+  MessagesWork({required this.filters, required this.repoDir});
 
   @override
   String get name => "messages test";
@@ -566,12 +572,16 @@ class MessagesWork extends Work {
   Map<String, Object?> toJson() {
     return {
       "WorkTypeIndex": WorkEnum.Messages.index,
+      "filters": filters,
       "repoDir": repoDir.toString(),
     };
   }
 
   static Work fromJson(Map<String, Object?> json) {
-    return new MessagesWork(repoDir: Uri.parse(json["repoDir"] as String));
+    return new MessagesWork(
+      filters: List<String>.from(json["filters"] as Iterable),
+      repoDir: Uri.parse(json["repoDir"] as String),
+    );
   }
 }
 

pkg/front_end/presubmit_helper_spawn.dart

@@ -89,7 +89,7 @@ Then run that file through your debugger or similar.
           ok = await Isolate.run(() async {
             ErrorNotingLogger logger = new ErrorNotingLogger();
             await testing.runMe(
-              const ["-DfastOnly=true"],
+              ["-DfastOnly=true", "--", ...work.filters],
               messages_suite.createContext,
               me: work.repoDir.resolve(
                 "pkg/front_end/test/messages_suite.dart",

pkg/front_end/test/messages_suite.dart

@@ -2,7 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import "dart:convert" show utf8;
+import "dart:convert" show utf8, json;
 import 'dart:io' show File, Platform;
 import "dart:typed_data" show Uint8List;
 
@@ -152,8 +152,37 @@ class MessageTestSuite extends ChainContext {
   @override
   Future<List<MessageTestDescription>> list(Chain suite) {
     List<MessageTestDescription> result = [];
-    Uri uri = suite.root.resolve("messages.yaml");
+    var rootString = suite.root.toString();
+    for (var subRoot in suite.subRoots) {
+      var subRootString = subRoot.toString();
+      if (!subRootString.startsWith(rootString)) {
+        throw StateError(
+          'Expected sub-root ${json.encode(subRootString)} to start with '
+          '${json.encode(rootString)}',
+        );
+      }
+      if (!subRootString.endsWith('/')) {
+        throw StateError(
+          'Expected sub-root ${json.encode(subRootString)} to end with "/"',
+        );
+      }
+      var prefix = subRootString.substring(rootString.length);
+      result.addAll(_ListSubRoot(subRoot, prefix: prefix));
+    }
+    return Future.value(result);
+  }
+
+  List<MessageTestDescription> _ListSubRoot(
+    Uri root, {
+    required String prefix,
+  }) {
+    List<MessageTestDescription> result = [];
+    Uri uri = root.resolve("messages.yaml");
     File file = new File.fromUri(uri);
+    // Allow for the possibility that the file might not exist yet.
+    // TODO(paulberry): remove this hack once
+    // `pkg/_fe_analyzer_shared/messages.yaml` exists
+    if (!file.existsSync()) return const [];
     String fileContent = file.readAsStringSync();
     YamlMap messages = loadYamlNode(fileContent, sourceUrl: uri) as YamlMap;
     for (String name in messages.keys) {
@@ -500,7 +529,7 @@ class MessageTestSuite extends ChainContext {
         ({String message, KnownExpectation expectation})? problem, {
         location,
       }) {
-        String shortName = "$name/$subName";
+        String shortName = "$prefix$name/$subName";
         if (problem != null) {
           String filename = relativize(uri);
           location ??= message.span.start;
@@ -634,15 +663,13 @@ class MessageTestSuite extends ChainContext {
           null,
           exampleAndAnalyzerCodeRequired &&
                   externalTest != null &&
-                  !(new File.fromUri(
-                    suite.root.resolve(externalTest),
-                  ).existsSync())
+                  !(new File.fromUri(root.resolve(externalTest)).existsSync())
               ? (
                   expectation: KnownExpectation.missingExternalFile,
                   message:
                       "Given external example for $name points to a "
                       "nonexisting file  "
-                      "(${suite.root.resolve(externalTest)}).",
+                      "(${root.resolve(externalTest)}).",
                 )
               : null,
         ),
@@ -684,7 +711,7 @@ class MessageTestSuite extends ChainContext {
         ),
       );
     }
-    return Future.value(result);
+    return result;
   }
 
   String formatProblems(

pkg/front_end/testing.json

@@ -8,7 +8,11 @@
       "name": "messages",
       "kind": "Chain",
       "source": "test/messages_suite.dart",
-      "root": "./",
+      "root": "../",
+      "subRoots": [
+        "_fe_analyzer_shared/",
+        "front_end/"
+      ],
       "status": "messages.status"
     },
     {

pkg/front_end/tool/generate_messages_lib.dart

@@ -69,9 +69,9 @@ part of 'cfe_codes.dart';
   int largestIndex = 0;
   final indexNameMap = new Map<int, String>();
 
-  List<String> keys = frontEndMessages.keys.toList()..sort();
+  List<String> keys = frontEndAndSharedMessages.keys.toList()..sort();
   for (String name in keys) {
-    var errorCodeInfo = frontEndMessages[name]!;
+    var errorCodeInfo = frontEndAndSharedMessages[name]!;
     var index = errorCodeInfo.index;
     if (index != null) {
       String? otherName = indexNameMap[index];
@@ -175,7 +175,7 @@ class _TemplateCompiler {
   _TemplateCompiler({
     required this.name,
     required this.index,
-    required FrontEndErrorCodeInfo errorCodeInfo,
+    required CfeStyleErrorCodeInfo errorCodeInfo,
   }) : problemMessage = errorCodeInfo.problemMessage,
        correctionMessage = errorCodeInfo.correctionMessage,
        analyzerCodes = errorCodeInfo.analyzerCode,