Skip to content

Commit e6228e3

Browse files
stereotype441Commit Queue
authored andcommitted
[messages] Handle camelCase-formatted files.
Updates the logic in `pkg/analyzer_utilities` and `pkg/front_end/test/messages_suite.dart` for processing `messages.yaml` files so that: - Message keys in analyzer-style `messages.yaml` files can be `camelCase`, `lower_snake_case`, or `UPPER_SNAKE_CASE` (previously they could only be `lower_snake_case` or `UPPER_SNAKE_CASE`). - Message keys in CFE-style `messages.yaml` files can be either `camelCase` or `PascalCase` (previously they could only be `PascalCase`). - `sharedName` fields can be `camelCase`, `lower_snake_case`, or `UPPER_SNAKE_CASE` (previously they could only be `lower_snake_case` or `UPPER_SNAKE_CASE`). - `analyzerCode` fields in `pkg/_fe_analyzer_shared/messages.yaml` can be `ClassName.lower_snake_case`, `ClassName.UPPER_SNAKE_CASE`, or `camelCase`, where `ClassName` is ignored (previously they could only be `ClassName.lower_snake_case` or `ClassName.UPPER_SNAKE_CASE`). This paves the way for a follow-up CL in which all these fields and keys will be standardized to `camelCase`, and then the ability to specify them in `lower_snake_case` or `UPPER_SNAKE_CASE` will be removed. This will eliminate a significant inconsistency between the diagnostic code formats in the analyzer and the front end. Note that a few diagnostic names contain an underscore immediately followed by a digit in their snake case representation: - `final_not_initialized_constructor_1` - `final_not_initialized_constructor_2` - `final_not_initialized_constructor_3_plus` - `lines_longer_than_80_chars` When these are converted to `camelCase` form, the code generator will no longer know to introduce the underscores when converting them back to `snake_case` form (e.g. `finalNotInitializedConstructor1` will get converted to `final_not_initialized_constructor1`). The snake case forms are an important part of the customer facing API (since they are what is accepted in `// ignore:` comments), so in order to preserve the existing snake case names, a hardcoded map is introduced, `_snakeCaseExceptions`. Change-Id: I6a6a696444ccd92dd6574a7cde08da88ac5a7135 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/466540 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent d0b5024 commit e6228e3

File tree

5 files changed

+88
-18
lines changed

5 files changed

+88
-18
lines changed

pkg/analyzer_utilities/lib/analyzer_messages.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,9 @@ List<M> decodeAnalyzerMessagesYaml<M extends AnalyzerMessage>(
196196
key: keyNode,
197197
value: diagnosticValue,
198198
decoder: (messageYaml) {
199-
var analyzerCode = DiagnosticCodeName(snakeCaseName: diagnosticName);
199+
var analyzerCode = DiagnosticCodeName.fromCamelOrSnakeCase(
200+
diagnosticName,
201+
);
200202
return decodeMessage(
201203
messageYaml,
202204
analyzerCode: analyzerCode,

pkg/analyzer_utilities/lib/messages.dart

Lines changed: 77 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ abstract class CfeStyleMessage extends Message {
153153
///
154154
/// This is the key corresponding to the diagnostic's entry in
155155
/// `messages.yaml`.
156-
final String frontEndCode;
156+
final DiagnosticCodeName frontEndCode;
157157

158158
CfeStyleMessage(MessageYaml messageYaml)
159159
: cfeSeverity = messageYaml.get(
@@ -172,7 +172,18 @@ abstract class CfeStyleMessage extends Message {
172172
},
173173
ifAbsent: () => null,
174174
),
175-
frontEndCode = messageYaml.keyString,
175+
frontEndCode = switch (messageYaml.keyString) {
176+
var s when s.isCamelCase => DiagnosticCodeName.fromCamelCase(
177+
messageYaml.keyString,
178+
),
179+
var s when s.isPascalCase => DiagnosticCodeName.fromPascalCase(
180+
messageYaml.keyString,
181+
),
182+
_ => throw LocatedError(
183+
'Front end codes must be camelCase or PascalCase',
184+
span: messageYaml.keySpan,
185+
),
186+
},
176187
super(messageYaml, requireProblemMessage: true) {
177188
// Ignore extra keys related to front end example-based tests.
178189
messageYaml.allowExtraKeys({
@@ -211,24 +222,74 @@ sealed class Conversion {
211222
/// This class implements [Comparable], so lists of it can be safely
212223
/// [List.sort]ed.
213224
class DiagnosticCodeName implements Comparable<DiagnosticCodeName> {
214-
/// The diagnostic name.
225+
/// Exceptions to the usual rules for converting diagnostic code names from
226+
/// camel case to snake case.
215227
///
216-
/// The diagnostic name is in "snake case", meaning it consists of words
217-
/// separated by underscores. Those words might be lower case or upper case.
228+
/// Normally, diagnostic code names are converted from camel case to snake
229+
/// case using [StringExtension.toSnakeCase]. But in rare situations when the
230+
/// name contains numbers, this can produce results that aren't ideal. Rather
231+
/// than try to fix these rare situations in a general fashion, it's easier to
232+
/// just have an explicit map of the problematic names, with their preferred
233+
/// snake case forms.
234+
static const Map<String, String> _snakeCaseExceptions = {
235+
'finalNotInitializedConstructor1': 'final_not_initialized_constructor_1',
236+
'finalNotInitializedConstructor2': 'final_not_initialized_constructor_2',
237+
'finalNotInitializedConstructor3Plus':
238+
'final_not_initialized_constructor_3_plus',
239+
'linesLongerThan80Chars': 'lines_longer_than_80_chars',
240+
};
241+
242+
/// The diagnostic name, as a "snake case" name (words separated by
243+
/// underscores).
218244
///
245+
/// The name might be lower case or upper case.
219246
// TODO(paulberry): change `messages.yaml` to consistently use lower snake
220247
// case, and remove [lowerSnakeCaseName].
221248
final String snakeCaseName;
222249

223-
DiagnosticCodeName({required this.snakeCaseName});
250+
/// The diagnostic name, as a "camel case" name (lower case word followed by
251+
/// capitalized words, with no separation between words).
252+
final String camelCaseName;
253+
254+
DiagnosticCodeName.fromCamelCase(this.camelCaseName)
255+
: snakeCaseName =
256+
_snakeCaseExceptions[camelCaseName] ?? camelCaseName.toSnakeCase() {
257+
if (snakeCaseName.toLowerCase() != snakeCaseName) {
258+
throw 'Snake case name ${json.encode(snakeCaseName)} is not all lower '
259+
'case';
260+
}
261+
if (snakeCaseName.toCamelCase() != camelCaseName) {
262+
throw 'Round-trip conversion from ${json.encode(camelCaseName)} to snake '
263+
'case and back produces ${json.encode(snakeCaseName.toCamelCase())}';
264+
}
265+
}
266+
267+
factory DiagnosticCodeName.fromCamelOrSnakeCase(String value) =>
268+
value.isCamelCase
269+
? DiagnosticCodeName.fromCamelCase(value)
270+
: DiagnosticCodeName.fromSnakeCase(value);
271+
272+
factory DiagnosticCodeName.fromPascalCase(String pascalCaseName) {
273+
var snakeCaseName = pascalCaseName.toSnakeCase();
274+
var camelCaseName = snakeCaseName.toPascalCase();
275+
return DiagnosticCodeName._(
276+
snakeCaseName: snakeCaseName,
277+
camelCaseName: camelCaseName,
278+
);
279+
}
280+
281+
DiagnosticCodeName.fromSnakeCase(this.snakeCaseName)
282+
: camelCaseName = snakeCaseName.toCamelCase();
283+
284+
DiagnosticCodeName._({
285+
required this.snakeCaseName,
286+
required this.camelCaseName,
287+
});
224288

225289
/// The string that should be generated into analyzer source code to refer to
226290
/// this diagnostic code.
227291
String get analyzerCodeReference => ['diag', camelCaseName].join('.');
228292

229-
/// The diagnostic name, converted to camel case.
230-
String get camelCaseName => snakeCaseName.toCamelCase();
231-
232293
@override
233294
int get hashCode => snakeCaseName.hashCode;
234295

@@ -410,7 +471,7 @@ class DiagnosticTables {
410471
activeMessagesByPackage = {};
411472

412473
DiagnosticTables._(List<Message> messages) {
413-
var frontEndCodeDuplicateChecker = _DuplicateChecker<String>(
474+
var frontEndCodeDuplicateChecker = _DuplicateChecker<DiagnosticCodeName>(
414475
kind: 'Front end code',
415476
);
416477
var analyzerCodeDuplicateChecker = _DuplicateChecker<DiagnosticCodeName>(
@@ -602,7 +663,7 @@ abstract class Message {
602663
) ??
603664
[],
604665
sharedName = switch (messageYaml.getOptionalString('sharedName')) {
605-
var s? => DiagnosticCodeName(snakeCaseName: s),
666+
var s? => DiagnosticCodeName.fromCamelOrSnakeCase(s),
606667
null => null,
607668
},
608669
removedIn = messageYaml.getOptionalString('removedIn'),
@@ -949,10 +1010,13 @@ class SharedMessage extends CfeStyleMessage with MessageWithAnalyzerCode {
9491010
switch (s.split('.')) {
9501011
case [_, var snakeCaseName]
9511012
when snakeCaseName == snakeCaseName.toUpperCase():
952-
return DiagnosticCodeName(snakeCaseName: snakeCaseName);
1013+
return DiagnosticCodeName.fromSnakeCase(snakeCaseName);
1014+
case [var camelCaseName] when camelCaseName.isCamelCase:
1015+
return DiagnosticCodeName.fromCamelCase(camelCaseName);
9531016
}
9541017
}
955-
throw 'Analyzer codes must take the form ClassName.DIAGNOSTIC_NAME.';
1018+
throw 'Analyzer codes must be either camelCase names or must take the form '
1019+
'ClassName.DIAGNOSTIC_NAME.';
9561020
}
9571021
}
9581022

pkg/front_end/test/messages_suite.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:_fe_analyzer_shared/src/messages/diagnostic_message.dart'
1313
getMessageRelatedInformation;
1414
import 'package:_fe_analyzer_shared/src/messages/severity.dart'
1515
show CfeSeverity, severityEnumValues;
16+
import 'package:analyzer_utilities/extensions/string.dart';
1617
import 'package:front_end/src/api_prototype/compiler_options.dart'
1718
show CompilerOptions, parseExperimentalArguments, parseExperimentalFlags;
1819
import 'package:front_end/src/api_prototype/experimental_flags.dart'
@@ -176,9 +177,11 @@ class MessageTestSuite extends ChainContext {
176177
File file = new File.fromUri(uri);
177178
String fileContent = file.readAsStringSync();
178179
YamlMap messages = loadYamlNode(fileContent, sourceUrl: uri) as YamlMap;
179-
for (String name in messages.keys) {
180+
for (String camelCaseName in messages.keys) {
180181
try {
181-
YamlMap messageNode = messages.nodes[name] as YamlMap;
182+
// TODO(paulberry): switch CFE to camelCase conventions.
183+
var name = camelCaseName.toSnakeCase().toPascalCase();
184+
YamlMap messageNode = messages.nodes[camelCaseName] as YamlMap;
182185
dynamic message = messageNode.value;
183186
if (message is String) continue;
184187

@@ -552,7 +555,7 @@ class MessageTestSuite extends ChainContext {
552555
),
553556
);
554557
} catch (e, st) {
555-
Error.throwWithStackTrace('While processing $name: $e', st);
558+
Error.throwWithStackTrace('While processing $camelCaseName: $e', st);
556559
}
557560
}
558561
return result;

pkg/front_end/test/spell_checking_list_tests.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,7 @@ parallax
631631
parameterized
632632
partfoo
633633
party
634+
pascal
634635
pause
635636
paused
636637
pays

pkg/front_end/tool/generate_messages_lib.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class _TemplateCompiler {
152152
_TemplateCompiler({
153153
required this.message,
154154
required this.pseudoSharedCodeValues,
155-
}) : name = message.frontEndCode,
155+
}) : name = message.frontEndCode.pascalCaseName,
156156
problemMessage = message.problemMessage,
157157
correctionMessage = message.correctionMessage,
158158
severity = message.cfeSeverity,

0 commit comments

Comments
 (0)