Skip to content

Commit 3884381

Browse files
thkim1011thkim1011
authored
Make gen-l10n error handling independent of logger state (#119644)
* init * lint * lint again
1 parent 8f90e2a commit 3884381

File tree

2 files changed

+160
-133
lines changed

2 files changed

+160
-133
lines changed

‎packages/flutter_tools/lib/src/localizations/gen_l10n.dart‎

Lines changed: 138 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,10 @@ class LocalizationsGenerator {
579579
// Whether we want to use escaping for ICU messages.
580580
bool useEscaping = false;
581581

582+
/// Whether any errors were caught. This is set after encountering any errors
583+
/// from calling [_generateMethod].
584+
bool hadErrors = false;
585+
582586
/// The list of all arb path strings in [inputDirectory].
583587
List<String> get arbPathStrings {
584588
return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList();
@@ -1093,151 +1097,152 @@ class LocalizationsGenerator {
10931097

10941098
String _generateMethod(Message message, LocaleInfo locale) {
10951099
try {
1096-
// Determine if we must import intl for date or number formatting.
1097-
if (message.placeholdersRequireFormatting) {
1098-
requiresIntlImport = true;
1099-
}
1100+
// Determine if we must import intl for date or number formatting.
1101+
if (message.placeholdersRequireFormatting) {
1102+
requiresIntlImport = true;
1103+
}
11001104

1101-
final String translationForMessage = message.messages[locale]!;
1102-
final Node node = message.parsedMessages[locale]!;
1103-
// If parse tree is only a string, then return a getter method.
1104-
if (node.children.every((Node child) => child.type == ST.string)) {
1105-
// Use the parsed translation to handle escaping with the same behavior.
1106-
return getterTemplate
1107-
.replaceAll('@(name)', message.resourceId)
1108-
.replaceAll('@(message)', "'${generateString(node.children.map((Node child) => child.value!).join())}'");
1109-
}
1105+
final String translationForMessage = message.messages[locale]!;
1106+
final Node node = message.parsedMessages[locale]!;
1107+
// If parse tree is only a string, then return a getter method.
1108+
if (node.children.every((Node child) => child.type == ST.string)) {
1109+
// Use the parsed translation to handle escaping with the same behavior.
1110+
return getterTemplate
1111+
.replaceAll('@(name)', message.resourceId)
1112+
.replaceAll('@(message)', "'${generateString(node.children.map((Node child) => child.value!).join())}'");
1113+
}
11101114

1111-
final List<String> tempVariables = <String>[];
1112-
// Get a unique temporary variable name.
1113-
int variableCount = 0;
1114-
String getTempVariableName() {
1115-
return '_temp${variableCount++}';
1116-
}
1115+
final List<String> tempVariables = <String>[];
1116+
// Get a unique temporary variable name.
1117+
int variableCount = 0;
1118+
String getTempVariableName() {
1119+
return '_temp${variableCount++}';
1120+
}
11171121

1118-
// Do a DFS post order traversal through placeholderExpr, pluralExpr, and selectExpr nodes.
1119-
// When traversing through a placeholderExpr node, return "$placeholderName".
1120-
// When traversing through a pluralExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
1121-
// When traversing through a selectExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
1122-
// When traversing through a message node, return concatenation of all of "generateVariables(child)" for each child.
1123-
String generateVariables(Node node, { bool isRoot = false }) {
1124-
switch (node.type) {
1125-
case ST.message:
1126-
final List<String> expressions = node.children.map<String>((Node node) {
1127-
if (node.type == ST.string) {
1128-
return node.value!;
1129-
}
1130-
return generateVariables(node);
1131-
}).toList();
1132-
return generateReturnExpr(expressions);
1133-
1134-
case ST.placeholderExpr:
1135-
assert(node.children[1].type == ST.identifier);
1136-
final String identifier = node.children[1].value!;
1137-
final Placeholder placeholder = message.placeholders[identifier]!;
1138-
if (placeholder.requiresFormatting) {
1139-
return '\$${node.children[1].value}String';
1140-
}
1141-
return '\$${node.children[1].value}';
1142-
1143-
case ST.pluralExpr:
1144-
requiresIntlImport = true;
1145-
final Map<String, String> pluralLogicArgs = <String, String>{};
1146-
// Recall that pluralExpr are of the form
1147-
// pluralExpr := "{" ID "," "plural" "," pluralParts "}"
1148-
assert(node.children[1].type == ST.identifier);
1149-
assert(node.children[5].type == ST.pluralParts);
1150-
1151-
final Node identifier = node.children[1];
1152-
final Node pluralParts = node.children[5];
1153-
1154-
for (final Node pluralPart in pluralParts.children.reversed) {
1155-
String pluralCase;
1156-
Node pluralMessage;
1157-
if (pluralPart.children[0].value == '=') {
1158-
assert(pluralPart.children[1].type == ST.number);
1159-
assert(pluralPart.children[3].type == ST.message);
1160-
pluralCase = pluralPart.children[1].value!;
1161-
pluralMessage = pluralPart.children[3];
1162-
} else {
1163-
assert(pluralPart.children[0].type == ST.identifier || pluralPart.children[0].type == ST.other);
1164-
assert(pluralPart.children[2].type == ST.message);
1165-
pluralCase = pluralPart.children[0].value!;
1166-
pluralMessage = pluralPart.children[2];
1122+
// Do a DFS post order traversal through placeholderExpr, pluralExpr, and selectExpr nodes.
1123+
// When traversing through a placeholderExpr node, return "$placeholderName".
1124+
// When traversing through a pluralExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
1125+
// When traversing through a selectExpr node, return "$tempVarN" and add variable declaration in "tempVariables".
1126+
// When traversing through a message node, return concatenation of all of "generateVariables(child)" for each child.
1127+
String generateVariables(Node node, { bool isRoot = false }) {
1128+
switch (node.type) {
1129+
case ST.message:
1130+
final List<String> expressions = node.children.map<String>((Node node) {
1131+
if (node.type == ST.string) {
1132+
return node.value!;
1133+
}
1134+
return generateVariables(node);
1135+
}).toList();
1136+
return generateReturnExpr(expressions);
1137+
1138+
case ST.placeholderExpr:
1139+
assert(node.children[1].type == ST.identifier);
1140+
final String identifier = node.children[1].value!;
1141+
final Placeholder placeholder = message.placeholders[identifier]!;
1142+
if (placeholder.requiresFormatting) {
1143+
return '\$${node.children[1].value}String';
11671144
}
1168-
if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) {
1169-
final String pluralPartExpression = generateVariables(pluralMessage);
1170-
final String? transformedPluralCase = pluralCases[pluralCase];
1171-
// A valid plural case is one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other".
1172-
if (transformedPluralCase == null) {
1173-
throw L10nParserException(
1174-
'''
1145+
return '\$${node.children[1].value}';
1146+
1147+
case ST.pluralExpr:
1148+
requiresIntlImport = true;
1149+
final Map<String, String> pluralLogicArgs = <String, String>{};
1150+
// Recall that pluralExpr are of the form
1151+
// pluralExpr := "{" ID "," "plural" "," pluralParts "}"
1152+
assert(node.children[1].type == ST.identifier);
1153+
assert(node.children[5].type == ST.pluralParts);
1154+
1155+
final Node identifier = node.children[1];
1156+
final Node pluralParts = node.children[5];
1157+
1158+
for (final Node pluralPart in pluralParts.children.reversed) {
1159+
String pluralCase;
1160+
Node pluralMessage;
1161+
if (pluralPart.children[0].value == '=') {
1162+
assert(pluralPart.children[1].type == ST.number);
1163+
assert(pluralPart.children[3].type == ST.message);
1164+
pluralCase = pluralPart.children[1].value!;
1165+
pluralMessage = pluralPart.children[3];
1166+
} else {
1167+
assert(pluralPart.children[0].type == ST.identifier || pluralPart.children[0].type == ST.other);
1168+
assert(pluralPart.children[2].type == ST.message);
1169+
pluralCase = pluralPart.children[0].value!;
1170+
pluralMessage = pluralPart.children[2];
1171+
}
1172+
if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) {
1173+
final String pluralPartExpression = generateVariables(pluralMessage);
1174+
final String? transformedPluralCase = pluralCases[pluralCase];
1175+
// A valid plural case is one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other".
1176+
if (transformedPluralCase == null) {
1177+
throw L10nParserException(
1178+
'''
11751179
The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other.
11761180
$pluralCase is not a valid plural case.''',
1177-
_inputFileNames[locale]!,
1178-
message.resourceId,
1179-
translationForMessage,
1180-
pluralPart.positionInMessage,
1181-
);
1182-
}
1183-
pluralLogicArgs[transformedPluralCase] = ' ${pluralCases[pluralCase]}: $pluralPartExpression,';
1184-
} else if (!suppressWarnings) {
1185-
logger.printWarning('''
1181+
_inputFileNames[locale]!,
1182+
message.resourceId,
1183+
translationForMessage,
1184+
pluralPart.positionInMessage,
1185+
);
1186+
}
1187+
pluralLogicArgs[transformedPluralCase] = ' ${pluralCases[pluralCase]}: $pluralPartExpression,';
1188+
} else if (!suppressWarnings) {
1189+
logger.printWarning('''
11861190
[${_inputFileNames[locale]}:${message.resourceId}] ICU Syntax Warning: The plural part specified below is overridden by a later plural part.
11871191
$translationForMessage
11881192
${Parser.indentForError(pluralPart.positionInMessage)}''');
1193+
}
11891194
}
1190-
}
1191-
final String tempVarName = getTempVariableName();
1192-
tempVariables.add(pluralVariableTemplate
1193-
.replaceAll('@(varName)', tempVarName)
1194-
.replaceAll('@(count)', identifier.value!)
1195-
.replaceAll('@(pluralLogicArgs)', pluralLogicArgs.values.join('\n'))
1196-
);
1197-
return '\$$tempVarName';
1198-
1199-
case ST.selectExpr:
1200-
requiresIntlImport = true;
1201-
// Recall that pluralExpr are of the form
1202-
// pluralExpr := "{" ID "," "plural" "," pluralParts "}"
1203-
assert(node.children[1].type == ST.identifier);
1204-
assert(node.children[5].type == ST.selectParts);
1205-
1206-
final Node identifier = node.children[1];
1207-
final List<String> selectLogicArgs = <String>[];
1208-
final Node selectParts = node.children[5];
1209-
for (final Node selectPart in selectParts.children) {
1210-
assert(selectPart.children[0].type == ST.identifier || selectPart.children[0].type == ST.other);
1211-
assert(selectPart.children[2].type == ST.message);
1212-
final String selectCase = selectPart.children[0].value!;
1213-
final Node selectMessage = selectPart.children[2];
1214-
final String selectPartExpression = generateVariables(selectMessage);
1215-
selectLogicArgs.add(" '$selectCase': $selectPartExpression,");
1216-
}
1217-
final String tempVarName = getTempVariableName();
1218-
tempVariables.add(selectVariableTemplate
1219-
.replaceAll('@(varName)', tempVarName)
1220-
.replaceAll('@(choice)', identifier.value!)
1221-
.replaceAll('@(selectCases)', selectLogicArgs.join('\n'))
1222-
);
1223-
return '\$$tempVarName';
1224-
// ignore: no_default_cases
1225-
default:
1226-
throw Exception('Cannot call "generateHelperMethod" on node type ${node.type}');
1195+
final String tempVarName = getTempVariableName();
1196+
tempVariables.add(pluralVariableTemplate
1197+
.replaceAll('@(varName)', tempVarName)
1198+
.replaceAll('@(count)', identifier.value!)
1199+
.replaceAll('@(pluralLogicArgs)', pluralLogicArgs.values.join('\n'))
1200+
);
1201+
return '\$$tempVarName';
1202+
1203+
case ST.selectExpr:
1204+
requiresIntlImport = true;
1205+
// Recall that pluralExpr are of the form
1206+
// pluralExpr := "{" ID "," "plural" "," pluralParts "}"
1207+
assert(node.children[1].type == ST.identifier);
1208+
assert(node.children[5].type == ST.selectParts);
1209+
1210+
final Node identifier = node.children[1];
1211+
final List<String> selectLogicArgs = <String>[];
1212+
final Node selectParts = node.children[5];
1213+
for (final Node selectPart in selectParts.children) {
1214+
assert(selectPart.children[0].type == ST.identifier || selectPart.children[0].type == ST.other);
1215+
assert(selectPart.children[2].type == ST.message);
1216+
final String selectCase = selectPart.children[0].value!;
1217+
final Node selectMessage = selectPart.children[2];
1218+
final String selectPartExpression = generateVariables(selectMessage);
1219+
selectLogicArgs.add(" '$selectCase': $selectPartExpression,");
1220+
}
1221+
final String tempVarName = getTempVariableName();
1222+
tempVariables.add(selectVariableTemplate
1223+
.replaceAll('@(varName)', tempVarName)
1224+
.replaceAll('@(choice)', identifier.value!)
1225+
.replaceAll('@(selectCases)', selectLogicArgs.join('\n'))
1226+
);
1227+
return '\$$tempVarName';
1228+
// ignore: no_default_cases
1229+
default:
1230+
throw Exception('Cannot call "generateHelperMethod" on node type ${node.type}');
1231+
}
12271232
}
1228-
}
1229-
final String messageString = generateVariables(node, isRoot: true);
1230-
final String tempVarLines = tempVariables.isEmpty ? '' : '${tempVariables.join('\n')}\n';
1231-
return methodTemplate
1232-
.replaceAll('@(name)', message.resourceId)
1233-
.replaceAll('@(parameters)', generateMethodParameters(message).join(', '))
1234-
.replaceAll('@(dateFormatting)', generateDateFormattingLogic(message))
1235-
.replaceAll('@(numberFormatting)', generateNumberFormattingLogic(message))
1236-
.replaceAll('@(tempVars)', tempVarLines)
1237-
.replaceAll('@(message)', messageString)
1238-
.replaceAll('@(none)\n', '');
1233+
final String messageString = generateVariables(node, isRoot: true);
1234+
final String tempVarLines = tempVariables.isEmpty ? '' : '${tempVariables.join('\n')}\n';
1235+
return methodTemplate
1236+
.replaceAll('@(name)', message.resourceId)
1237+
.replaceAll('@(parameters)', generateMethodParameters(message).join(', '))
1238+
.replaceAll('@(dateFormatting)', generateDateFormattingLogic(message))
1239+
.replaceAll('@(numberFormatting)', generateNumberFormattingLogic(message))
1240+
.replaceAll('@(tempVars)', tempVarLines)
1241+
.replaceAll('@(message)', messageString)
1242+
.replaceAll('@(none)\n', '');
12391243
} on L10nParserException catch (error) {
12401244
logger.printError(error.toString());
1245+
hadErrors = true;
12411246
return '';
12421247
}
12431248
}
@@ -1247,7 +1252,7 @@ The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "
12471252
final String generatedLocalizationsFile = _generateCode();
12481253

12491254
// If there were any syntax errors, don't write to files.
1250-
if (logger.hadErrorOutput) {
1255+
if (hadErrors) {
12511256
throw L10nException('Found syntax errors.');
12521257
}
12531258

‎packages/flutter_tools/test/general.shard/generate_localizations_test.dart‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,28 @@ void main() {
785785
});
786786

787787
group('generateLocalizations', () {
788+
// Regression test for https://github.com/flutter/flutter/issues/119593
789+
testWithoutContext('other logs from flutter_tools does not affect gen-l10n', () async {
790+
_standardFlutterDirectoryL10nSetup(fs);
791+
792+
final Logger logger = BufferLogger.test();
793+
logger.printError('An error output from a different tool in flutter_tools');
794+
795+
// Should run without error.
796+
generateLocalizations(
797+
fileSystem: fs,
798+
options: LocalizationOptions(
799+
arbDirectory: Uri.directory(defaultL10nPathString),
800+
outputDirectory: Uri.directory(defaultL10nPathString, windows: false),
801+
templateArbFile: Uri.file(defaultTemplateArbFileName, windows: false),
802+
useSyntheticPackage: false,
803+
),
804+
logger: logger,
805+
projectDir: fs.currentDirectory,
806+
dependenciesDir: fs.currentDirectory,
807+
);
808+
});
809+
788810
testWithoutContext('forwards arguments correctly', () async {
789811
_standardFlutterDirectoryL10nSetup(fs);
790812
final LocalizationOptions options = LocalizationOptions(

0 commit comments

Comments
 (0)