Skip to content

Commit ff6e9d4

Browse files
christopherfujinothkim1011XilaiZhang
authored
CP: Throw error when plural case had undefined behavior (flutter#116622) (flutter#119384)
* Throw error when plural case had undefined behavior (flutter#116622) * init * add comment * make error more actionable * improve test * increase debugging for find_commit.dart --------- Co-authored-by: Tae Hyung Kim <[email protected]> Co-authored-by: Xilai Zhang <[email protected]>
1 parent c96f14a commit ff6e9d4

File tree

3 files changed

+49
-3
lines changed

3 files changed

+49
-3
lines changed

dev/tools/bin/find_commit.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
import 'dart:io';
1212

13-
const bool debugLogging = false;
13+
const bool debugLogging = true;
1414

1515
void log(String message) {
1616
if (debugLogging) {
@@ -84,7 +84,7 @@ String findCommit({
8484
String git(String workingDirectory, List<String> arguments) {
8585
final ProcessResult result = Process.runSync('git', arguments, workingDirectory: workingDirectory);
8686
if (result.exitCode != 0 || '${result.stderr}'.isNotEmpty) {
87-
throw ProcessException('git', arguments, '${result.stdout}${result.stderr}', result.exitCode);
87+
throw ProcessException('git', arguments, 'working directory: "$workingDirectory"\n${result.stdout}\n${result.stderr}', result.exitCode);
8888
}
8989
return '${result.stdout}';
9090
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1163,7 +1163,20 @@ class LocalizationsGenerator {
11631163
}
11641164
if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) {
11651165
final String pluralPartExpression = generateVariables(pluralMessage);
1166-
pluralLogicArgs[pluralCases[pluralCase]!] = ' ${pluralCases[pluralCase]}: $pluralPartExpression,';
1166+
final String? transformedPluralCase = pluralCases[pluralCase];
1167+
// A valid plural case is one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other".
1168+
if (transformedPluralCase == null) {
1169+
throw L10nParserException(
1170+
'''
1171+
The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other.
1172+
$pluralCase is not a valid plural case.''',
1173+
_inputFileNames[locale]!,
1174+
message.resourceId,
1175+
translationForMessage,
1176+
pluralPart.positionInMessage,
1177+
);
1178+
}
1179+
pluralLogicArgs[transformedPluralCase] = ' ${pluralCases[pluralCase]}: $pluralPartExpression,';
11671180
} else if (!suppressWarnings) {
11681181
logger.printWarning('''
11691182
[${_inputFileNames[locale]}:${message.resourceId}] ICU Syntax Warning: The plural part specified below is overridden by a later plural part.

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1957,6 +1957,39 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e
19571957
^'''));
19581958
});
19591959

1960+
testWithoutContext('undefined plural cases throws syntax error', () {
1961+
const String pluralMessageWithUndefinedParts = '''
1962+
{
1963+
"count": "{count,plural, =0{None} =1{One} =2{Two} =3{Undefined Behavior!} other{Hmm...}}"
1964+
}''';
1965+
final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n')
1966+
..createSync(recursive: true);
1967+
l10nDirectory.childFile(defaultTemplateArbFileName)
1968+
.writeAsStringSync(pluralMessageWithUndefinedParts);
1969+
try {
1970+
LocalizationsGenerator(
1971+
fileSystem: fs,
1972+
inputPathString: defaultL10nPathString,
1973+
outputPathString: defaultL10nPathString,
1974+
templateArbFileName: defaultTemplateArbFileName,
1975+
outputFileString: defaultOutputFileString,
1976+
classNameString: defaultClassNameString,
1977+
logger: logger,
1978+
)
1979+
..loadResources()
1980+
..writeOutputFiles();
1981+
fail('expected L10nException');
1982+
} on L10nException catch (error) {
1983+
expect(error.message, contains('Found syntax errors.'));
1984+
expect(logger.hadErrorOutput, isTrue);
1985+
expect(logger.errorText, contains('''
1986+
[app_en.arb:count] The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "many", or "other.
1987+
3 is not a valid plural case.
1988+
{count,plural, =0{None} =1{One} =2{Two} =3{Undefined Behavior!} other{Hmm...}}
1989+
^'''));
1990+
}
1991+
});
1992+
19601993
testWithoutContext('should automatically infer plural placeholders that are not explicitly defined', () {
19611994
const String pluralMessageWithoutPlaceholdersAttribute = '''
19621995
{

0 commit comments

Comments
 (0)