Skip to content

Commit f8ccea7

Browse files
stereotype441Commit Queue
authored andcommitted
[messages] Rework message decoding and validation.
Reworks the message code generation logic so that the contents of the `messages.yaml` files are (nearly) fully validated at the time they are decoded. The only fields that are not validated are those whose names are passed to `MessageYaml.allowExtraKeys`. These fields are validated elsewhere: - The fields `bytes`, `declaration`, `exampleAllowMultipleReports`, `exampleAllowOtherCodes`, `experiments`, `expression`, `external`, `includeErrorContext`, `script`, and `statement`, when they appear in `pkg/_fe_analyzer_shared/messages.yaml` or `pkg/front_end/messages.yaml`. These fields are validated by the logic in `messages_suite.dart`, as part of verifying that the front end produces the expected diagnostics. - The field `experiment`, when it appears in `pkg/analyzer/messages.yaml` or `pkg/linter/messages.yaml`. This field is validated by the logic in `verify_diagnostics_test.dart`, as part of verifying that the analyzer produces the expected diagnostics. - The fields `categories`, `deprecatedDetails`, and `state`, when they appear in `pkg/linter/messages.yaml`. These fields are validated by the logic in `pkg/linter/tool/messages_info.dart`. Change-Id: I6a6a69648bb3f5c3248489701bdf2ccc082a59ea Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458067 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent e1f9348 commit f8ccea7

File tree

5 files changed

+380
-245
lines changed

5 files changed

+380
-245
lines changed

pkg/analyzer/tool/messages/generate.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ part of ${json.encode(file.parentLibrary)};
130130
var diagnosticCode = entry.key;
131131
var message = entry.value;
132132

133-
LocatedError.wrap(node: message.keyNode, () {
133+
LocatedError.wrap(span: message.keySpan, () {
134134
if (message is! AliasMessage &&
135135
diagnosticClass.includeInDiagnosticCodeValues) {
136136
generatedCodes.add(diagnosticCode);

pkg/analyzer_utilities/lib/analyzer_messages.dart

Lines changed: 57 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -189,11 +189,18 @@ final String linterPkgPath = normalize(join(pkg_root.packageRoot, 'linter'));
189189
/// Decoded messages from the linter's `messages.yaml` file.
190190
final List<AnalyzerMessage> lintMessages = decodeAnalyzerMessagesYaml(
191191
linterPkgPath,
192+
allowLinterKeys: true,
192193
);
193194

194195
/// Decodes a YAML object (in analyzer style `messages.yaml` format) into a list
195196
/// of [AnalyzerMessage]s.
196-
List<AnalyzerMessage> decodeAnalyzerMessagesYaml(String packagePath) {
197+
///
198+
/// If [allowLinterKeys], error checking logic will not reject key/value pairs
199+
/// that are used by the linter.
200+
List<AnalyzerMessage> decodeAnalyzerMessagesYaml(
201+
String packagePath, {
202+
bool allowLinterKeys = false,
203+
}) {
197204
var path = join(packagePath, 'messages.yaml');
198205
var yaml = loadYamlNode(
199206
File(path).readAsStringSync(),
@@ -202,22 +209,22 @@ List<AnalyzerMessage> decodeAnalyzerMessagesYaml(String packagePath) {
202209

203210
var result = <AnalyzerMessage>[];
204211
if (yaml is! YamlMap) {
205-
throw LocatedError('root node is not a map', node: yaml);
212+
throw LocatedError('root node is not a map', span: yaml.span);
206213
}
207214
for (var classEntry in yaml.nodes.entries) {
208215
var keyNode = classEntry.key as YamlScalar;
209216
var className = keyNode.value;
210217
if (className is! String) {
211218
throw LocatedError(
212219
'non-string class key ${json.encode(className)}',
213-
node: keyNode,
220+
span: keyNode.span,
214221
);
215222
}
216223
var classValue = classEntry.value;
217224
if (classValue is! YamlMap) {
218225
throw LocatedError(
219226
'value associated with class key $className is not a map',
220-
node: classValue,
227+
span: classValue.span,
221228
);
222229
}
223230
for (var diagnosticEntry in classValue.nodes.entries) {
@@ -226,36 +233,40 @@ List<AnalyzerMessage> decodeAnalyzerMessagesYaml(String packagePath) {
226233
if (diagnosticName is! String) {
227234
throw LocatedError(
228235
'non-string diagnostic key ${json.encode(diagnosticName)}',
229-
node: keyNode,
236+
span: keyNode.span,
230237
);
231238
}
232239
var diagnosticValue = diagnosticEntry.value;
233240
if (diagnosticValue is! YamlMap) {
234241
throw LocatedError(
235242
'value associated with diagnostic is not a map',
236-
node: diagnosticValue,
243+
span: diagnosticValue.span,
237244
);
238245
}
239246

240-
AnalyzerMessage message = LocatedError.wrap(node: diagnosticValue, () {
241-
var analyzerCode = AnalyzerCode(
242-
diagnosticClass: DiagnosticClassInfo.byName(className),
243-
snakeCaseName: diagnosticName,
244-
);
245-
return AnalyzerMessage.fromYaml(
246-
diagnosticValue,
247-
keyNode: keyNode,
248-
analyzerCode: analyzerCode,
249-
);
250-
});
247+
AnalyzerMessage message = MessageYaml.decode(
248+
key: keyNode,
249+
value: diagnosticValue,
250+
decoder: (messageYaml) {
251+
var analyzerCode = AnalyzerCode(
252+
diagnosticClass: DiagnosticClassInfo.byName(className),
253+
snakeCaseName: diagnosticName,
254+
);
255+
return AnalyzerMessage(
256+
messageYaml,
257+
analyzerCode: analyzerCode,
258+
allowLinterKeys: allowLinterKeys,
259+
);
260+
},
261+
);
251262
result.add(message);
252263

253264
if (message case AliasMessage(:var aliasFor)) {
254265
var aliasForPath = aliasFor.split('.');
255266
if (aliasForPath.isEmpty) {
256267
throw LocatedError(
257268
"The 'aliasFor' value is empty",
258-
node: diagnosticValue,
269+
span: diagnosticValue.span,
259270
);
260271
}
261272
var node = yaml;
@@ -264,7 +275,7 @@ List<AnalyzerMessage> decodeAnalyzerMessagesYaml(String packagePath) {
264275
if (value is! YamlMap) {
265276
throw LocatedError(
266277
'No Map value at "$aliasFor"',
267-
node: diagnosticValue,
278+
span: diagnosticValue.span,
268279
);
269280
}
270281
node = value;
@@ -280,12 +291,12 @@ List<AnalyzerMessage> decodeAnalyzerMessagesYaml(String packagePath) {
280291
class AliasMessage extends AnalyzerMessage {
281292
String aliasFor;
282293

283-
AliasMessage._fromYaml(
284-
super.yaml, {
294+
AliasMessage(
295+
super.messageYaml, {
285296
required this.aliasFor,
286-
required super.keyNode,
287297
required super.analyzerCode,
288-
}) : super._fromYaml();
298+
required super.allowLinterKeys,
299+
}) : super._();
289300

290301
String get aliasForClass => aliasFor.split('.').first;
291302

@@ -312,33 +323,38 @@ class AnalyzerMessage extends Message with MessageWithAnalyzerCode {
312323
@override
313324
final bool hasPublishedDocs;
314325

315-
factory AnalyzerMessage.fromYaml(
316-
YamlMap yaml, {
317-
required YamlScalar keyNode,
326+
factory AnalyzerMessage(
327+
MessageYaml messageYaml, {
318328
required AnalyzerCode analyzerCode,
329+
required bool allowLinterKeys,
319330
}) {
320-
if (yaml['aliasFor'] case var aliasFor?) {
321-
return AliasMessage._fromYaml(
322-
yaml,
323-
aliasFor: aliasFor as String,
324-
keyNode: keyNode,
331+
if (messageYaml.getOptionalString('aliasFor') case var aliasFor?) {
332+
return AliasMessage(
333+
messageYaml,
334+
aliasFor: aliasFor,
325335
analyzerCode: analyzerCode,
336+
allowLinterKeys: allowLinterKeys,
326337
);
327338
} else {
328-
return AnalyzerMessage._fromYaml(
329-
yaml,
330-
keyNode: keyNode,
339+
return AnalyzerMessage._(
340+
messageYaml,
331341
analyzerCode: analyzerCode,
342+
allowLinterKeys: allowLinterKeys,
332343
);
333344
}
334345
}
335346

336-
AnalyzerMessage._fromYaml(
337-
super.yaml, {
338-
required super.keyNode,
347+
AnalyzerMessage._(
348+
MessageYaml messageYaml, {
339349
required this.analyzerCode,
340-
}) : hasPublishedDocs =
341-
yaml['hasPublishedDocs'] as bool? ??
342-
(throw 'Missing key "hasPublishedDocs".'),
343-
super.fromYaml();
350+
required bool allowLinterKeys,
351+
}) : hasPublishedDocs = messageYaml.getBool('hasPublishedDocs'),
352+
super(messageYaml) {
353+
// Ignore extra keys related to analyzer example-based tests.
354+
messageYaml.allowExtraKeys({'experiment'});
355+
if (allowLinterKeys) {
356+
// Ignore extra keys understood by the linter.
357+
messageYaml.allowExtraKeys({'categories', 'deprecatedDetails', 'state'});
358+
}
359+
}
344360
}

0 commit comments

Comments
 (0)