Make gen-l10n error handling independent of logger state (#119644)
* init
* lint
* lint again
diff --git a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart
index 30bb27f..5a31e09 100644
--- a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart
+++ b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart
@@ -579,6 +579,10 @@
// Whether we want to use escaping for ICU messages.
bool useEscaping = false;
+ /// Whether any errors were caught. This is set after encountering any errors
+ /// from calling [_generateMethod].
+ bool hadErrors = false;
+
/// The list of all arb path strings in [inputDirectory].
List<String> get arbPathStrings {
return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList();
@@ -1093,151 +1097,152 @@
String _generateMethod(Message message, LocaleInfo locale) {
try {
- // Determine if we must import intl for date or number formatting.
- if (message.placeholdersRequireFormatting) {
- requiresIntlImport = true;
- }
+ // Determine if we must import intl for date or number formatting.
+ if (message.placeholdersRequireFormatting) {
+ requiresIntlImport = true;
+ }
- 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)) {
- // Use the parsed translation to handle escaping with the same behavior.
- return getterTemplate
- .replaceAll('@(name)', message.resourceId)
- .replaceAll('@(message)', "'${generateString(node.children.map((Node child) => child.value!).join())}'");
- }
+ 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)) {
+ // Use the parsed translation to handle escaping with the same behavior.
+ return getterTemplate
+ .replaceAll('@(name)', message.resourceId)
+ .replaceAll('@(message)', "'${generateString(node.children.map((Node child) => child.value!).join())}'");
+ }
- final List<String> tempVariables = <String>[];
- // Get a unique temporary variable name.
- int variableCount = 0;
- String getTempVariableName() {
- return '_temp${variableCount++}';
- }
+ final List<String> tempVariables = <String>[];
+ // Get a unique temporary variable name.
+ int variableCount = 0;
+ String getTempVariableName() {
+ return '_temp${variableCount++}';
+ }
- // Do a DFS post order traversal through placeholderExpr, pluralExpr, and selectExpr nodes.
- // When traversing through a placeholderExpr node, return "$placeholderName".
- // When traversing through a pluralExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
- // When traversing through a selectExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
- // When traversing through a message node, return concatenation of all of "generateVariables(child)" for each child.
- String generateVariables(Node node, { bool isRoot = false }) {
- switch (node.type) {
- case ST.message:
- final List<String> expressions = node.children.map<String>((Node node) {
- if (node.type == ST.string) {
- return node.value!;
+ // Do a DFS post order traversal through placeholderExpr, pluralExpr, and selectExpr nodes.
+ // When traversing through a placeholderExpr node, return "$placeholderName".
+ // When traversing through a pluralExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
+ // When traversing through a selectExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
+ // When traversing through a message node, return concatenation of all of "generateVariables(child)" for each child.
+ String generateVariables(Node node, { bool isRoot = false }) {
+ switch (node.type) {
+ case ST.message:
+ final List<String> expressions = node.children.map<String>((Node node) {
+ if (node.type == ST.string) {
+ return node.value!;
+ }
+ return generateVariables(node);
+ }).toList();
+ return generateReturnExpr(expressions);
+
+ case ST.placeholderExpr:
+ assert(node.children[1].type == ST.identifier);
+ final String identifier = node.children[1].value!;
+ final Placeholder placeholder = message.placeholders[identifier]!;
+ if (placeholder.requiresFormatting) {
+ return '\$${node.children[1].value}String';
}
- return generateVariables(node);
- }).toList();
- return generateReturnExpr(expressions);
+ return '\$${node.children[1].value}';
- case ST.placeholderExpr:
- assert(node.children[1].type == ST.identifier);
- final String identifier = node.children[1].value!;
- final Placeholder placeholder = message.placeholders[identifier]!;
- if (placeholder.requiresFormatting) {
- return '\$${node.children[1].value}String';
- }
- return '\$${node.children[1].value}';
+ case ST.pluralExpr:
+ requiresIntlImport = true;
+ final Map<String, String> pluralLogicArgs = <String, String>{};
+ // Recall that pluralExpr are of the form
+ // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
+ assert(node.children[1].type == ST.identifier);
+ assert(node.children[5].type == ST.pluralParts);
- case ST.pluralExpr:
- requiresIntlImport = true;
- final Map<String, String> pluralLogicArgs = <String, String>{};
- // Recall that pluralExpr are of the form
- // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
- assert(node.children[1].type == ST.identifier);
- assert(node.children[5].type == ST.pluralParts);
+ final Node identifier = node.children[1];
+ final Node pluralParts = node.children[5];
- final Node identifier = node.children[1];
- final Node pluralParts = node.children[5];
-
- for (final Node pluralPart in pluralParts.children.reversed) {
- String pluralCase;
- Node pluralMessage;
- if (pluralPart.children[0].value == '=') {
- assert(pluralPart.children[1].type == ST.number);
- assert(pluralPart.children[3].type == ST.message);
- pluralCase = pluralPart.children[1].value!;
- pluralMessage = pluralPart.children[3];
- } else {
- assert(pluralPart.children[0].type == ST.identifier || pluralPart.children[0].type == ST.other);
- assert(pluralPart.children[2].type == ST.message);
- pluralCase = pluralPart.children[0].value!;
- pluralMessage = pluralPart.children[2];
- }
- if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) {
- final String pluralPartExpression = generateVariables(pluralMessage);
- final String? transformedPluralCase = pluralCases[pluralCase];
- // A valid plural case is one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other".
- if (transformedPluralCase == null) {
- throw L10nParserException(
- '''
+ for (final Node pluralPart in pluralParts.children.reversed) {
+ String pluralCase;
+ Node pluralMessage;
+ if (pluralPart.children[0].value == '=') {
+ assert(pluralPart.children[1].type == ST.number);
+ assert(pluralPart.children[3].type == ST.message);
+ pluralCase = pluralPart.children[1].value!;
+ pluralMessage = pluralPart.children[3];
+ } else {
+ assert(pluralPart.children[0].type == ST.identifier || pluralPart.children[0].type == ST.other);
+ assert(pluralPart.children[2].type == ST.message);
+ pluralCase = pluralPart.children[0].value!;
+ pluralMessage = pluralPart.children[2];
+ }
+ if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) {
+ final String pluralPartExpression = generateVariables(pluralMessage);
+ final String? transformedPluralCase = pluralCases[pluralCase];
+ // A valid plural case is one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other".
+ if (transformedPluralCase == null) {
+ throw L10nParserException(
+ '''
The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other.
$pluralCase is not a valid plural case.''',
- _inputFileNames[locale]!,
- message.resourceId,
- translationForMessage,
- pluralPart.positionInMessage,
- );
- }
- pluralLogicArgs[transformedPluralCase] = ' ${pluralCases[pluralCase]}: $pluralPartExpression,';
- } else if (!suppressWarnings) {
- logger.printWarning('''
+ _inputFileNames[locale]!,
+ message.resourceId,
+ translationForMessage,
+ pluralPart.positionInMessage,
+ );
+ }
+ pluralLogicArgs[transformedPluralCase] = ' ${pluralCases[pluralCase]}: $pluralPartExpression,';
+ } else if (!suppressWarnings) {
+ logger.printWarning('''
[${_inputFileNames[locale]}:${message.resourceId}] ICU Syntax Warning: The plural part specified below is overridden by a later plural part.
$translationForMessage
${Parser.indentForError(pluralPart.positionInMessage)}''');
+ }
}
- }
- final String tempVarName = getTempVariableName();
- tempVariables.add(pluralVariableTemplate
- .replaceAll('@(varName)', tempVarName)
- .replaceAll('@(count)', identifier.value!)
- .replaceAll('@(pluralLogicArgs)', pluralLogicArgs.values.join('\n'))
- );
- return '\$$tempVarName';
+ final String tempVarName = getTempVariableName();
+ tempVariables.add(pluralVariableTemplate
+ .replaceAll('@(varName)', tempVarName)
+ .replaceAll('@(count)', identifier.value!)
+ .replaceAll('@(pluralLogicArgs)', pluralLogicArgs.values.join('\n'))
+ );
+ return '\$$tempVarName';
- case ST.selectExpr:
- requiresIntlImport = true;
- // Recall that pluralExpr are of the form
- // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
- assert(node.children[1].type == ST.identifier);
- assert(node.children[5].type == ST.selectParts);
+ case ST.selectExpr:
+ requiresIntlImport = true;
+ // Recall that pluralExpr are of the form
+ // pluralExpr := "{" ID "," "plural" "," pluralParts "}"
+ assert(node.children[1].type == ST.identifier);
+ assert(node.children[5].type == ST.selectParts);
- final Node identifier = node.children[1];
- final List<String> selectLogicArgs = <String>[];
- final Node selectParts = node.children[5];
- for (final Node selectPart in selectParts.children) {
- assert(selectPart.children[0].type == ST.identifier || selectPart.children[0].type == ST.other);
- assert(selectPart.children[2].type == ST.message);
- final String selectCase = selectPart.children[0].value!;
- final Node selectMessage = selectPart.children[2];
- final String selectPartExpression = generateVariables(selectMessage);
- selectLogicArgs.add(" '$selectCase': $selectPartExpression,");
- }
- final String tempVarName = getTempVariableName();
- tempVariables.add(selectVariableTemplate
- .replaceAll('@(varName)', tempVarName)
- .replaceAll('@(choice)', identifier.value!)
- .replaceAll('@(selectCases)', selectLogicArgs.join('\n'))
- );
- return '\$$tempVarName';
- // ignore: no_default_cases
- default:
- throw Exception('Cannot call "generateHelperMethod" on node type ${node.type}');
+ final Node identifier = node.children[1];
+ final List<String> selectLogicArgs = <String>[];
+ final Node selectParts = node.children[5];
+ for (final Node selectPart in selectParts.children) {
+ assert(selectPart.children[0].type == ST.identifier || selectPart.children[0].type == ST.other);
+ assert(selectPart.children[2].type == ST.message);
+ final String selectCase = selectPart.children[0].value!;
+ final Node selectMessage = selectPart.children[2];
+ final String selectPartExpression = generateVariables(selectMessage);
+ selectLogicArgs.add(" '$selectCase': $selectPartExpression,");
+ }
+ final String tempVarName = getTempVariableName();
+ tempVariables.add(selectVariableTemplate
+ .replaceAll('@(varName)', tempVarName)
+ .replaceAll('@(choice)', identifier.value!)
+ .replaceAll('@(selectCases)', selectLogicArgs.join('\n'))
+ );
+ return '\$$tempVarName';
+ // ignore: no_default_cases
+ default:
+ throw Exception('Cannot call "generateHelperMethod" on node type ${node.type}');
+ }
}
- }
- final String messageString = generateVariables(node, isRoot: true);
- final String tempVarLines = tempVariables.isEmpty ? '' : '${tempVariables.join('\n')}\n';
- return methodTemplate
- .replaceAll('@(name)', message.resourceId)
- .replaceAll('@(parameters)', generateMethodParameters(message).join(', '))
- .replaceAll('@(dateFormatting)', generateDateFormattingLogic(message))
- .replaceAll('@(numberFormatting)', generateNumberFormattingLogic(message))
- .replaceAll('@(tempVars)', tempVarLines)
- .replaceAll('@(message)', messageString)
- .replaceAll('@(none)\n', '');
+ final String messageString = generateVariables(node, isRoot: true);
+ final String tempVarLines = tempVariables.isEmpty ? '' : '${tempVariables.join('\n')}\n';
+ return methodTemplate
+ .replaceAll('@(name)', message.resourceId)
+ .replaceAll('@(parameters)', generateMethodParameters(message).join(', '))
+ .replaceAll('@(dateFormatting)', generateDateFormattingLogic(message))
+ .replaceAll('@(numberFormatting)', generateNumberFormattingLogic(message))
+ .replaceAll('@(tempVars)', tempVarLines)
+ .replaceAll('@(message)', messageString)
+ .replaceAll('@(none)\n', '');
} on L10nParserException catch (error) {
logger.printError(error.toString());
+ hadErrors = true;
return '';
}
}
@@ -1247,7 +1252,7 @@
final String generatedLocalizationsFile = _generateCode();
// If there were any syntax errors, don't write to files.
- if (logger.hadErrorOutput) {
+ if (hadErrors) {
throw L10nException('Found syntax errors.');
}
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 604b821..d40174a 100644
--- a/packages/flutter_tools/test/general.shard/generate_localizations_test.dart
+++ b/packages/flutter_tools/test/general.shard/generate_localizations_test.dart
@@ -785,6 +785,28 @@
});
group('generateLocalizations', () {
+ // Regression test for https://github.com/flutter/flutter/issues/119593
+ testWithoutContext('other logs from flutter_tools does not affect gen-l10n', () async {
+ _standardFlutterDirectoryL10nSetup(fs);
+
+ final Logger logger = BufferLogger.test();
+ logger.printError('An error output from a different tool in flutter_tools');
+
+ // Should run without error.
+ generateLocalizations(
+ fileSystem: fs,
+ options: LocalizationOptions(
+ arbDirectory: Uri.directory(defaultL10nPathString),
+ outputDirectory: Uri.directory(defaultL10nPathString, windows: false),
+ templateArbFile: Uri.file(defaultTemplateArbFileName, windows: false),
+ useSyntheticPackage: false,
+ ),
+ logger: logger,
+ projectDir: fs.currentDirectory,
+ dependenciesDir: fs.currentDirectory,
+ );
+ });
+
testWithoutContext('forwards arguments correctly', () async {
_standardFlutterDirectoryL10nSetup(fs);
final LocalizationOptions options = LocalizationOptions(
