development/flutter
development/flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter synced 2024-10-13 11:42:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix message type inconsistency between locales (#120129)

Browse source 
* init

* fix error handling

* fix issue

* lint?

* error handling tests

* lint
This commit is contained in:
Tae Hyung Kim 2023-02-13 15:05:57 -08:00 committed by GitHub
parent ddebe833b0
commit becb6bd00a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 158 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
packages/flutter_tools/lib/src/localizations/gen_l10n.dart
View file

@ -877,6 +877,7 @@ class LocalizationsGenerator {
_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;
@ -915,16 +916,19 @@ class LocalizationsGenerator {
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,
);
});
@ -953,7 +957,7 @@ class LocalizationsGenerator {
});
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
@ -1103,8 +1107,8 @@ class LocalizationsGenerator {
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)

22
packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart
View file

@ -349,13 +349,20 @@ class Message {
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);
@ -369,6 +376,7 @@ class Message {
final Map<String, Placeholder> placeholders;
final bool useEscaping;
final Logger? logger;
bool hadErrors = false;
bool get placeholdersRequireFormatting => placeholders.values.any((Placeholder p) => p.requiresFormatting);

15
packages/flutter_tools/lib/src/localizations/message_parser.dart
View file

@ -587,17 +587,8 @@ class Parser {
}
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;
}
}

128
packages/flutter_tools/test/general.shard/generate_localizations_test.dart
View file

@ -1733,6 +1733,47 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e
).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', () {
@ -2258,6 +2299,93 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e
});
});
// 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)