Fix message type inconsistency between locales (#120129) (#120678)
(cherry picked from commit becb6bd00aab3e1a904eeb7fe9a19b57e8cfda86)
Co-authored-by: Tae Hyung Kim <thkim1011@users.noreply.github.com>
diff --git a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart
index e594951..3fca692 100644
--- a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart
+++ b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart
@@ -874,6 +874,7 @@
_allMessages = _templateBundle.resourceIds.map((String id) => Message(
_templateBundle, _allBundles, id, areResourceAttributesRequired, useEscaping: useEscaping, logger: logger,
)).toList();
+ hadErrors = _allMessages.any((Message message) => message.hadErrors);
if (inputsAndOutputsListFile != null) {
_inputFileList.addAll(_allBundles.bundles.map((AppResourceBundle bundle) {
return bundle.file.absolute.path;
@@ -912,16 +913,19 @@
final LocaleInfo locale,
) {
final Iterable<String> methods = _allMessages.map((Message message) {
+ LocaleInfo localeWithFallback = locale;
if (message.messages[locale] == null) {
_addUnimplementedMessage(locale, message.resourceId);
- return _generateMethod(
- message,
- _templateArbLocale,
- );
+ localeWithFallback = _templateArbLocale;
+ }
+ if (message.parsedMessages[localeWithFallback] == null) {
+ // The message exists, but parsedMessages[locale] is null due to a syntax error.
+ // This means that we have already set hadErrors = true while constructing the Message.
+ return '';
}
return _generateMethod(
message,
- locale,
+ localeWithFallback,
);
});
@@ -950,7 +954,7 @@
});
final Iterable<String> methods = _allMessages
- .where((Message message) => message.messages[locale] != null)
+ .where((Message message) => message.parsedMessages[locale] != null)
.map((Message message) => _generateMethod(message, locale));
return subclassTemplate
@@ -1100,8 +1104,8 @@
final String translationForMessage = message.messages[locale]!;
final Node node = message.parsedMessages[locale]!;
- // If parse tree is only a string, then return a getter method.
- if (node.children.every((Node child) => child.type == ST.string)) {
+ // If the placeholders list is empty, then return a getter method.
+ if (message.placeholders.isEmpty) {
// Use the parsed translation to handle escaping with the same behavior.
return getterTemplate
.replaceAll('@(name)', message.resourceId)
diff --git a/packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart b/packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart
index d61cd5d..7e1c8dd 100644
--- a/packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart
+++ b/packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart
@@ -353,13 +353,20 @@
filenames[bundle.locale] = bundle.file.basename;
final String? translation = bundle.translationFor(resourceId);
messages[bundle.locale] = translation;
- parsedMessages[bundle.locale] = translation == null ? null : Parser(
- resourceId,
- bundle.file.basename,
- translation,
- useEscaping: useEscaping,
- logger: logger
- ).parse();
+ try {
+ parsedMessages[bundle.locale] = translation == null ? null : Parser(
+ resourceId,
+ bundle.file.basename,
+ translation,
+ useEscaping: useEscaping,
+ logger: logger
+ ).parse();
+ } on L10nParserException catch (error) {
+ logger?.printError(error.toString());
+ // Treat it as an untranslated message in case we can't parse.
+ parsedMessages[bundle.locale] = null;
+ hadErrors = true;
+ }
}
// Infer the placeholders
_inferPlaceholders(filenames);
@@ -373,6 +380,7 @@
final Map<String, Placeholder> placeholders;
final bool useEscaping;
final Logger? logger;
+ bool hadErrors = false;
bool get placeholdersRequireFormatting => placeholders.values.any((Placeholder p) => p.requiresFormatting);
diff --git a/packages/flutter_tools/lib/src/localizations/message_parser.dart b/packages/flutter_tools/lib/src/localizations/message_parser.dart
index ae030e0..a7198ef 100644
--- a/packages/flutter_tools/lib/src/localizations/message_parser.dart
+++ b/packages/flutter_tools/lib/src/localizations/message_parser.dart
@@ -342,7 +342,7 @@
parsingStack.addAll(grammarRule.reversed);
// For tree construction, add nodes to the parent until the parent has all
- // all the children it is expecting.
+ // the children it is expecting.
parent.children.add(node);
if (parent.isFull) {
treeTraversalStack.removeLast();
@@ -587,17 +587,8 @@
}
Node parse() {
- try {
- final Node syntaxTree = compress(parseIntoTree());
- checkExtraRules(syntaxTree);
- return syntaxTree;
- } on L10nParserException catch (error) {
- // For debugging purposes.
- if (logger == null) {
- rethrow;
- }
- logger?.printError(error.toString());
- return Node(ST.empty, 0, value: '');
- }
+ final Node syntaxTree = compress(parseIntoTree());
+ checkExtraRules(syntaxTree);
+ return syntaxTree;
}
}
diff --git a/packages/flutter_tools/test/general.shard/generate_localizations_test.dart b/packages/flutter_tools/test/general.shard/generate_localizations_test.dart
index 821263d..b0a2774 100644
--- a/packages/flutter_tools/test/general.shard/generate_localizations_test.dart
+++ b/packages/flutter_tools/test/general.shard/generate_localizations_test.dart
@@ -1663,6 +1663,47 @@
).readAsStringSync();
expect(localizationsFile, contains('String helloWorld(Object name) {'));
});
+
+ testWithoutContext('placeholder parameter list should be consistent between languages', () {
+ const String messageEn = '''
+{
+ "helloWorld": "Hello {name}",
+ "@helloWorld": {
+ "placeholders": {
+ "name": {}
+ }
+ }
+}''';
+ const String messageEs = '''
+{
+ "helloWorld": "Hola"
+}
+''';
+ final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n')
+ ..createSync(recursive: true);
+ l10nDirectory.childFile(defaultTemplateArbFileName)
+ .writeAsStringSync(messageEn);
+ l10nDirectory.childFile('app_es.arb')
+ .writeAsStringSync(messageEs);
+ LocalizationsGenerator(
+ fileSystem: fs,
+ inputPathString: defaultL10nPathString,
+ templateArbFileName: defaultTemplateArbFileName,
+ outputFileString: defaultOutputFileString,
+ classNameString: defaultClassNameString,
+ logger: logger,
+ )
+ ..loadResources()
+ ..writeOutputFiles();
+ final String localizationsFileEn = fs.file(
+ fs.path.join(syntheticL10nPackagePath, 'output-localization-file_en.dart'),
+ ).readAsStringSync();
+ final String localizationsFileEs = fs.file(
+ fs.path.join(syntheticL10nPackagePath, 'output-localization-file_es.dart'),
+ ).readAsStringSync();
+ expect(localizationsFileEn, contains('String helloWorld(Object name) {'));
+ expect(localizationsFileEs, contains('String helloWorld(Object name) {'));
+ });
});
group('DateTime tests', () {
@@ -2189,6 +2230,93 @@
});
});
+ // All error handling for messages should collect errors on a per-error
+ // basis and log them out individually. Then, it will throw an L10nException.
+ group('error handling tests', () {
+ testWithoutContext('syntax/code-gen errors properly logs errors per message', () {
+ // TODO(thkim1011): Fix error handling so that long indents don't get truncated.
+ // See https://github.com/flutter/flutter/issues/120490.
+ const String messagesWithSyntaxErrors = '''
+{
+ "hello": "Hello { name",
+ "plural": "This is an incorrectly formatted plural: { count, plural, zero{No frog} one{One frog} other{{count} frogs}",
+ "explanationWithLexingError": "The 'string above is incorrect as it forgets to close the brace",
+ "pluralWithInvalidCase": "{ count, plural, woohoo{huh?} other{lol} }"
+}''';
+ try {
+ final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n')
+ ..createSync(recursive: true);
+ l10nDirectory.childFile(defaultTemplateArbFileName)
+ .writeAsStringSync(messagesWithSyntaxErrors);
+ LocalizationsGenerator(
+ fileSystem: fs,
+ inputPathString: defaultL10nPathString,
+ outputPathString: defaultL10nPathString,
+ templateArbFileName: defaultTemplateArbFileName,
+ outputFileString: defaultOutputFileString,
+ classNameString: defaultClassNameString,
+ useEscaping: true,
+ logger: logger,
+ )
+ ..loadResources()
+ ..writeOutputFiles();
+ } on L10nException {
+ expect(logger.errorText, contains('''
+[app_en.arb:hello] ICU Syntax Error: Expected "}" but found no tokens.
+ Hello { name
+ ^
+[app_en.arb:plural] ICU Syntax Error: Expected "}" but found no tokens.
+ This is an incorrectly formatted plural: { count, plural, zero{No frog} one{One frog} other{{count} frogs}
+ ^
+[app_en.arb:explanationWithLexingError] ICU Lexing Error: Unmatched single quotes.
+ The 'string above is incorrect as it forgets to close the brace
+ ^
+[app_en.arb:pluralWithInvalidCase] ICU Syntax Error: Plural expressions case must be one of "zero", "one", "two", "few", "many", or "other".
+ { count, plural, woohoo{huh?} other{lol} }
+ ^'''));
+ }
+ });
+
+ testWithoutContext('errors thrown in multiple languages are all shown', () {
+ const String messageEn = '''
+{
+ "hello": "Hello { name"
+}''';
+ const String messageEs = '''
+{
+ "hello": "Hola { name"
+}''';
+ try {
+ final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n')
+ ..createSync(recursive: true);
+ l10nDirectory.childFile(defaultTemplateArbFileName)
+ .writeAsStringSync(messageEn);
+ l10nDirectory.childFile('app_es.arb')
+ .writeAsStringSync(messageEs);
+ LocalizationsGenerator(
+ fileSystem: fs,
+ inputPathString: defaultL10nPathString,
+ outputPathString: defaultL10nPathString,
+ templateArbFileName: defaultTemplateArbFileName,
+ outputFileString: defaultOutputFileString,
+ classNameString: defaultClassNameString,
+ useEscaping: true,
+ logger: logger,
+ )
+ ..loadResources()
+ ..writeOutputFiles();
+ } on L10nException {
+ expect(logger.errorText, contains('''
+[app_en.arb:hello] ICU Syntax Error: Expected "}" but found no tokens.
+ Hello { name
+ ^
+[app_es.arb:hello] ICU Syntax Error: Expected "}" but found no tokens.
+ Hola { name
+ ^'''));
+ }
+ });
+ });
+
testWithoutContext('intl package import should be omitted in subclass files when no plurals are included', () {
fs.currentDirectory.childDirectory('lib').childDirectory('l10n')..createSync(recursive: true)
..childFile(defaultTemplateArbFileName).writeAsStringSync(singleMessageArbFileString)
