From 7802c7acd886b783e658eb8b0844072a81117d2a Mon Sep 17 00:00:00 2001 From: Tae Hyung Kim Date: Wed, 30 Nov 2022 15:07:06 -0800 Subject: [PATCH] [gen_l10n] Improvements to `gen_l10n` (#116202) * init * fix tests * fix lint * extra changes * oops missed some merge conflicts * fix lexer add tests * consistent warnings and errors * throw error at the end * improve efficiency, improve code generation * fix * nit * fix test * remove helper method class * two d's * oops * empty commit as google testing won't pass :( --- .../src/commands/generate_localizations.dart | 6 + .../lib/src/localizations/gen_l10n.dart | 193 +++---- .../src/localizations/gen_l10n_templates.dart | 36 +- .../lib/src/localizations/gen_l10n_types.dart | 197 ++++---- .../localizations/localizations_utils.dart | 35 +- .../lib/src/localizations/message_parser.dart | 23 +- .../generate_localizations_test.dart | 478 +++++++----------- .../general.shard/message_parser_test.dart | 44 +- 8 files changed, 431 insertions(+), 581 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/generate_localizations.dart b/packages/flutter_tools/lib/src/commands/generate_localizations.dart index beae3f30a0d7..f693930a7d9d 100644 --- a/packages/flutter_tools/lib/src/commands/generate_localizations.dart +++ b/packages/flutter_tools/lib/src/commands/generate_localizations.dart @@ -201,6 +201,10 @@ class GenerateLocalizationsCommand extends FlutterCommand { 'contained within pairs of single quotes as normal strings and treat all ' 'consecutive pairs of single quotes as a single quote character.', ); + argParser.addFlag( + 'suppress-warnings', + help: 'When specified, all warnings will be suppressed.\n' + ); } final FileSystem _fileSystem; @@ -258,6 +262,7 @@ class GenerateLocalizationsCommand extends FlutterCommand { final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes'); final bool usesNullableGetter = boolArgDeprecated('nullable-getter'); final bool useEscaping = boolArgDeprecated('use-escaping'); + final bool suppressWarnings = boolArgDeprecated('suppress-warnings'); precacheLanguageAndRegionTags(); @@ -281,6 +286,7 @@ class GenerateLocalizationsCommand extends FlutterCommand { usesNullableGetter: usesNullableGetter, useEscaping: useEscaping, logger: _logger, + suppressWarnings: suppressWarnings, ) ..loadResources() ..writeOutputFiles()) diff --git a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart index b5831af3d501..75fc944042cb 100644 --- a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart +++ b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart @@ -67,6 +67,7 @@ LocalizationsGenerator generateLocalizations({ usesNullableGetter: options.usesNullableGetter, useEscaping: options.useEscaping, logger: logger, + suppressWarnings: options.suppressWarnings, ) ..loadResources() ..writeOutputFiles(isFromYaml: true); @@ -90,8 +91,6 @@ String _syntheticL10nPackagePath(FileSystem fileSystem) => fileSystem.path.join( // For example, if placeholders are used for plurals and no type was specified, then the type will // automatically set to 'num'. Similarly, if such placeholders are used for selects, then the type // will be set to 'String'. For such placeholders that are used for both, we should throw an error. -// TODO(thkim1011): Let's store the output of this function in the Message class, so that we don't -// recompute this. See https://github.com/flutter/flutter/issues/112709 List generateMethodParameters(Message message) { return message.placeholders.values.map((Placeholder placeholder) { return '${placeholder.type} ${placeholder.name}'; @@ -456,6 +455,7 @@ class LocalizationsGenerator { bool usesNullableGetter = true, bool useEscaping = false, required Logger logger, + bool suppressWarnings = false, }) { final Directory? projectDirectory = projectDirFromPath(fileSystem, projectPathString); final Directory inputDirectory = inputDirectoryFromPath(fileSystem, inputPathString, projectDirectory); @@ -478,6 +478,7 @@ class LocalizationsGenerator { areResourceAttributesRequired: areResourceAttributesRequired, useEscaping: useEscaping, logger: logger, + suppressWarnings: suppressWarnings, ); } @@ -501,10 +502,11 @@ class LocalizationsGenerator { this.usesNullableGetter = true, required this.logger, this.useEscaping = false, + this.suppressWarnings = false, }); final FileSystem _fs; - Iterable _allMessages = []; + List _allMessages = []; late final AppResourceBundleCollection _allBundles = AppResourceBundleCollection(inputDirectory); late final AppResourceBundle _templateBundle = AppResourceBundle(templateArbFile); late final Map _inputFileNames = Map.fromEntries( @@ -637,6 +639,9 @@ class LocalizationsGenerator { /// Logger to be used during the execution of the script. Logger logger; + /// Whether or not to suppress warnings or not. + final bool suppressWarnings; + static bool _isNotReadable(FileStat fileStat) { final String rawStatString = fileStat.modeString(); // Removes potential prepended permission bits, such as '(suid)' and '(guid)'. @@ -851,9 +856,6 @@ class LocalizationsGenerator { // Load _allMessages from templateArbFile and _allBundles from all of the ARB // files in inputDirectory. Also initialized: supportedLocales. void loadResources() { - _allMessages = _templateBundle.resourceIds.map((String id) => Message( - _templateBundle, _allBundles, id, areResourceAttributesRequired, useEscaping: useEscaping, - )); for (final String resourceId in _templateBundle.resourceIds) { if (!_isValidGetterAndMethodName(resourceId)) { throw L10nException( @@ -864,7 +866,10 @@ class LocalizationsGenerator { ); } } - + // The call to .toList() is absolutely necessary. Otherwise, it is an iterator and will call Message's constructor again. + _allMessages = _templateBundle.resourceIds.map((String id) => Message( + _templateBundle, _allBundles, id, areResourceAttributesRequired, useEscaping: useEscaping, logger: logger, + )).toList(); if (inputsAndOutputsListFile != null) { _inputFileList.addAll(_allBundles.bundles.map((AppResourceBundle bundle) { return bundle.file.absolute.path; @@ -1083,6 +1088,7 @@ class LocalizationsGenerator { } String _generateMethod(Message message, LocaleInfo locale) { + try { // Determine if we must import intl for date or number formatting. if (message.placeholdersRequireFormatting) { requiresIntlImport = true; @@ -1098,74 +1104,37 @@ class LocalizationsGenerator { .replaceAll('@(message)', "'${generateString(node.children.map((Node child) => child.value!).join())}'"); } - final List helperMethods = []; - - // Get a unique helper method name. - int methodNameCount = 0; - String getHelperMethodName() { - return '_${message.resourceId}${methodNameCount++}'; + final List tempVariables = []; + // Get a unique temporary variable name. + int variableCount = 0; + String getTempVariableName() { + return '_temp${variableCount++}'; } - // Do a DFS post order traversal, generating dependent - // placeholder, plural, select helper methods, and combine these into - // one message. Returns the method/placeholder to use in parent string. - HelperMethod generateHelperMethods(Node node, { bool isRoot = false }) { - final Set dependentPlaceholders = {}; + // 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 helpers = node.children.map((Node node) { + final List expressions = node.children.map((Node node) { if (node.type == ST.string) { - return HelperMethod({}, string: node.value); + return node.value!; } - final HelperMethod helper = generateHelperMethods(node); - dependentPlaceholders.addAll(helper.dependentPlaceholders); - return helper; + return generateVariables(node); }).toList(); - final String messageString = generateReturnExpr(helpers); - - // If the message is just a normal string, then only return the string. - if (dependentPlaceholders.isEmpty) { - return HelperMethod(dependentPlaceholders, string: messageString); - } - - // For messages, if we are generating the actual overridden method, then we should also deal with - // date and number formatting here. - final String helperMethodName = getHelperMethodName(); - final HelperMethod messageHelper = HelperMethod(dependentPlaceholders, helper: helperMethodName); - if (isRoot) { - helperMethods.add(methodTemplate - .replaceAll('@(name)', message.resourceId) - .replaceAll('@(parameters)', generateMethodParameters(message).join(', ')) - .replaceAll('@(dateFormatting)', generateDateFormattingLogic(message)) - .replaceAll('@(numberFormatting)', generateNumberFormattingLogic(message)) - .replaceAll('@(message)', messageString) - .replaceAll('@(none)\n', '') - ); - } else { - helperMethods.add(messageHelperTemplate - .replaceAll('@(name)', helperMethodName) - .replaceAll('@(parameters)', messageHelper.methodParameters) - .replaceAll('@(message)', messageString) - ); - } - return messageHelper; + return generateReturnExpr(expressions); case ST.placeholderExpr: assert(node.children[1].type == ST.identifier); - final Node identifier = node.children[1]; - // Check that placeholders exist. - final Placeholder? placeholder = message.placeholders[identifier.value]; - if (placeholder == null) { - throw L10nParserException( - 'Make sure that the specified placeholder is defined in your arb file.', - _inputFileNames[locale]!, - message.resourceId, - translationForMessage, - identifier.positionInMessage, - ); + final String identifier = node.children[1].value!; + final Placeholder placeholder = message.placeholders[identifier]!; + if (placeholder.requiresFormatting) { + return '\$${node.children[1].value}String'; } - dependentPlaceholders.add(placeholder); - return HelperMethod(dependentPlaceholders, placeholder: placeholder); + return '\$${node.children[1].value}'; case ST.pluralExpr: requiresIntlImport = true; @@ -1178,28 +1147,6 @@ class LocalizationsGenerator { final Node identifier = node.children[1]; final Node pluralParts = node.children[5]; - // Check that placeholders exist and is of type int or num. - final Placeholder? placeholder = message.placeholders[identifier.value]; - if (placeholder == null) { - throw L10nParserException( - 'Make sure that the specified placeholder is defined in your arb file.', - _inputFileNames[locale]!, - message.resourceId, - translationForMessage, - identifier.positionInMessage, - ); - } - if (placeholder.type != 'num' && placeholder.type != 'int') { - throw L10nParserException( - 'The specified placeholder must be of type int or num.', - _inputFileNames[locale]!, - message.resourceId, - translationForMessage, - identifier.positionInMessage, - ); - } - dependentPlaceholders.add(placeholder); - for (final Node pluralPart in pluralParts.children.reversed) { String pluralCase; Node pluralMessage; @@ -1215,26 +1162,22 @@ class LocalizationsGenerator { pluralMessage = pluralPart.children[2]; } if (!pluralLogicArgs.containsKey(pluralCases[pluralCase])) { - final HelperMethod pluralPartHelper = generateHelperMethods(pluralMessage); - pluralLogicArgs[pluralCases[pluralCase]!] = ' ${pluralCases[pluralCase]}: ${pluralPartHelper.helperOrPlaceholder},'; - dependentPlaceholders.addAll(pluralPartHelper.dependentPlaceholders); - } else { + final String pluralPartExpression = generateVariables(pluralMessage); + pluralLogicArgs[pluralCases[pluralCase]!] = ' ${pluralCases[pluralCase]}: $pluralPartExpression,'; + } else if (!suppressWarnings) { logger.printWarning(''' -The plural part specified below is overrided by a later plural part. -$translationForMessage -${Parser.indentForError(pluralPart.positionInMessage)} -'''); +[${_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 helperMethodName = getHelperMethodName(); - final HelperMethod pluralHelper = HelperMethod(dependentPlaceholders, helper: helperMethodName); - helperMethods.add(pluralHelperTemplate - .replaceAll('@(name)', helperMethodName) - .replaceAll('@(parameters)', pluralHelper.methodParameters) + final String tempVarName = getTempVariableName(); + tempVariables.add(pluralVariableTemplate + .replaceAll('@(varName)', tempVarName) .replaceAll('@(count)', identifier.value!) .replaceAll('@(pluralLogicArgs)', pluralLogicArgs.values.join('\n')) ); - return pluralHelper; + return '\$$tempVarName'; case ST.selectExpr: requiresIntlImport = true; @@ -1244,53 +1187,53 @@ ${Parser.indentForError(pluralPart.positionInMessage)} assert(node.children[5].type == ST.selectParts); final Node identifier = node.children[1]; - // Check that placeholders exist. - final Placeholder? placeholder = message.placeholders[identifier.value]; - if (placeholder == null) { - throw L10nParserException( - 'Make sure that the specified placeholder is defined in your arb file.', - _inputFileNames[locale]!, - message.resourceId, - translationForMessage, - identifier.positionInMessage, - ); - } - dependentPlaceholders.add(placeholder); final List selectLogicArgs = []; 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 HelperMethod selectPartHelper = generateHelperMethods(selectMessage); - selectLogicArgs.add(" '$selectCase': ${selectPartHelper.helperOrPlaceholder},"); - dependentPlaceholders.addAll(selectPartHelper.dependentPlaceholders); + final String selectPartExpression = generateVariables(selectMessage); + selectLogicArgs.add(" '$selectCase': $selectPartExpression,"); } - final String helperMethodName = getHelperMethodName(); - final HelperMethod selectHelper = HelperMethod(dependentPlaceholders, helper: helperMethodName); - - helperMethods.add(selectHelperTemplate - .replaceAll('@(name)', helperMethodName) - .replaceAll('@(parameters)', selectHelper.methodParameters) + final String tempVarName = getTempVariableName(); + tempVariables.add(selectVariableTemplate + .replaceAll('@(varName)', tempVarName) .replaceAll('@(choice)', identifier.value!) .replaceAll('@(selectCases)', selectLogicArgs.join('\n')) ); - return HelperMethod(dependentPlaceholders, helper: helperMethodName); + return '\$$tempVarName'; // ignore: no_default_cases default: throw Exception('Cannot call "generateHelperMethod" on node type ${node.type}'); } } - generateHelperMethods(node, isRoot: true); - return helperMethods.last.replaceAll('@(helperMethods)', helperMethods.sublist(0, helperMethods.length - 1).join('\n\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()); + return ''; + } } List writeOutputFiles({ bool isFromYaml = false }) { // First, generate the string contents of all necessary files. final String generatedLocalizationsFile = _generateCode(); + // If there were any syntax errors, don't write to files. + if (logger.hadErrorOutput) { + throw L10nException('Found syntax errors.'); + } + // A pubspec.yaml file is required when using a synthetic package. If it does not // exist, create a blank one. if (useSyntheticPackage) { diff --git a/packages/flutter_tools/lib/src/localizations/gen_l10n_templates.dart b/packages/flutter_tools/lib/src/localizations/gen_l10n_templates.dart index c2bb67d84e1f..5fe28380c229 100644 --- a/packages/flutter_tools/lib/src/localizations/gen_l10n_templates.dart +++ b/packages/flutter_tools/lib/src/localizations/gen_l10n_templates.dart @@ -139,33 +139,23 @@ const String methodTemplate = ''' String @(name)(@(parameters)) { @(dateFormatting) @(numberFormatting) -@(helperMethods) - return @(message); +@(tempVars) return @(message); }'''; -const String messageHelperTemplate = ''' - String @(name)(@(parameters)) { - return @(message); - }'''; - -const String pluralHelperTemplate = ''' - String @(name)(@(parameters)) { - return intl.Intl.pluralLogic( - @(count), - locale: localeName, +const String pluralVariableTemplate = ''' + String @(varName) = intl.Intl.pluralLogic( + @(count), + locale: localeName, @(pluralLogicArgs) - ); - }'''; - -const String selectHelperTemplate = ''' - String @(name)(@(parameters)) { - return intl.Intl.selectLogic( - @(choice), - { + );'''; + +const String selectVariableTemplate = ''' + String @(varName) = intl.Intl.selectLogic( + @(choice), + { @(selectCases) - }, - ); - }'''; + }, + );'''; const String classFileTemplate = ''' @(header)@(requiresIntlImport)import '@(fileName)'; 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 ea12fe6a2be6..d61cd5dcf876 100644 --- a/packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart +++ b/packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart @@ -5,6 +5,7 @@ import 'package:intl/locale.dart'; import '../base/file_system.dart'; +import '../base/logger.dart'; import '../convert.dart'; import 'localizations_utils.dart'; import 'message_parser.dart'; @@ -138,17 +139,31 @@ class L10nParserException extends L10nException { this.messageString, this.charNumber ): super(''' -$error -[$fileName:$messageId] $messageString -${List.filled(4 + fileName.length + messageId.length + charNumber, ' ').join()}^'''); +[$fileName:$messageId] $error + $messageString + ${List.filled(charNumber, ' ').join()}^'''); final String error; final String fileName; final String messageId; final String messageString; + // Position of character within the "messageString" where the error is. final int charNumber; } +class L10nMissingPlaceholderException extends L10nParserException { + L10nMissingPlaceholderException( + super.error, + super.fileName, + super.messageId, + super.messageString, + super.charNumber, + this.placeholderName, + ); + + final String placeholderName; +} + // One optional named parameter to be used by a NumberFormat. // // Some of the NumberFormat factory constructors have optional named parameters. @@ -319,7 +334,10 @@ class Message { AppResourceBundleCollection allBundles, this.resourceId, bool isResourceAttributeRequired, - { this.useEscaping = false } + { + this.useEscaping = false, + this.logger, + } ) : assert(templateBundle != null), assert(allBundles != null), assert(resourceId != null && resourceId.isNotEmpty), @@ -335,64 +353,16 @@ 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).parse(); - } - // Using parsed translations, attempt to infer types of placeholders used by plurals and selects. - for (final LocaleInfo locale in parsedMessages.keys) { - if (parsedMessages[locale] == null) { - continue; - } - final List traversalStack = [parsedMessages[locale]!]; - while (traversalStack.isNotEmpty) { - final Node node = traversalStack.removeLast(); - if (node.type == ST.pluralExpr) { - final Placeholder? placeholder = placeholders[node.children[1].value!]; - if (placeholder == null) { - throw L10nParserException( - 'Make sure that the specified plural placeholder is defined in your arb file.', - filenames[locale]!, - resourceId, - messages[locale]!, - node.children[1].positionInMessage - ); - } - placeholders[node.children[1].value!]!.isPlural = true; - } - if (node.type == ST.selectExpr) { - final Placeholder? placeholder = placeholders[node.children[1].value!]; - if (placeholder == null) { - throw L10nParserException( - 'Make sure that the specified select placeholder is defined in your arb file.', - filenames[locale]!, - resourceId, - messages[locale]!, - node.children[1].positionInMessage - ); - } - placeholders[node.children[1].value!]!.isSelect = true; - } - traversalStack.addAll(node.children); - } - } - for (final Placeholder placeholder in placeholders.values) { - if (placeholder.isPlural && placeholder.isSelect) { - throw L10nException('Placeholder is used as both a plural and select in certain languages.'); - } else if (placeholder.isPlural) { - if (placeholder.type == null) { - placeholder.type = 'num'; - } - else if (!['num', 'int'].contains(placeholder.type)) { - throw L10nException("Placeholders used in plurals must be of type 'num' or 'int'"); - } - } else if (placeholder.isSelect) { - if (placeholder.type == null) { - placeholder.type = 'String'; - } else if (placeholder.type != 'String') { - throw L10nException("Placeholders used in selects must be of type 'String'"); - } - } - placeholder.type ??= 'Object'; + parsedMessages[bundle.locale] = translation == null ? null : Parser( + resourceId, + bundle.file.basename, + translation, + useEscaping: useEscaping, + logger: logger + ).parse(); } + // Infer the placeholders + _inferPlaceholders(filenames); } final String resourceId; @@ -402,6 +372,7 @@ class Message { final Map parsedMessages; final Map placeholders; final bool useEscaping; + final Logger? logger; bool get placeholdersRequireFormatting => placeholders.values.any((Placeholder p) => p.requiresFormatting); @@ -496,6 +467,63 @@ class Message { }), ); } + + // Using parsed translations, attempt to infer types of placeholders used by plurals and selects. + // For undeclared placeholders, create a new placeholder. + void _inferPlaceholders(Map filenames) { + // We keep the undeclared placeholders separate so that we can sort them alphabetically afterwards. + final Map undeclaredPlaceholders = {}; + // Helper for getting placeholder by name. + Placeholder? getPlaceholder(String name) => placeholders[name] ?? undeclaredPlaceholders[name]; + for (final LocaleInfo locale in parsedMessages.keys) { + if (parsedMessages[locale] == null) { + continue; + } + final List traversalStack = [parsedMessages[locale]!]; + while (traversalStack.isNotEmpty) { + final Node node = traversalStack.removeLast(); + if ([ST.placeholderExpr, ST.pluralExpr, ST.selectExpr].contains(node.type)) { + final String identifier = node.children[1].value!; + Placeholder? placeholder = getPlaceholder(identifier); + if (placeholder == null) { + placeholder = Placeholder(resourceId, identifier, {}); + undeclaredPlaceholders[identifier] = placeholder; + } + if (node.type == ST.pluralExpr) { + placeholder.isPlural = true; + } else if (node.type == ST.selectExpr) { + placeholder.isSelect = true; + } + } + traversalStack.addAll(node.children); + } + } + placeholders.addEntries( + undeclaredPlaceholders.entries + .toList() + ..sort((MapEntry p1, MapEntry p2) => p1.key.compareTo(p2.key)) + ); + + for (final Placeholder placeholder in placeholders.values) { + if (placeholder.isPlural && placeholder.isSelect) { + throw L10nException('Placeholder is used as both a plural and select in certain languages.'); + } else if (placeholder.isPlural) { + if (placeholder.type == null) { + placeholder.type = 'num'; + } + else if (!['num', 'int'].contains(placeholder.type)) { + throw L10nException("Placeholders used in plurals must be of type 'num' or 'int'"); + } + } else if (placeholder.isSelect) { + if (placeholder.type == null) { + placeholder.type = 'String'; + } else if (placeholder.type != 'String') { + throw L10nException("Placeholders used in selects must be of type 'String'"); + } + } + placeholder.type ??= 'Object'; + } + } } // Represents the contents of one ARB file. @@ -834,50 +862,3 @@ final Set _iso639Languages = { 'zh', 'zu', }; - -// Used in LocalizationsGenerator._generateMethod.generateHelperMethod. -class HelperMethod { - HelperMethod(this.dependentPlaceholders, {this.helper, this.placeholder, this.string }): - assert((() { - // At least one of helper, placeholder, string must be nonnull. - final bool a = helper == null; - final bool b = placeholder == null; - final bool c = string == null; - return (!a && b && c) || (a && !b && c) || (a && b && !c); - })()); - - Set dependentPlaceholders; - String? helper; - Placeholder? placeholder; - String? string; - - String get helperOrPlaceholder { - if (helper != null) { - return '$helper($methodArguments)'; - } else if (string != null) { - return '$string'; - } else { - if (placeholder!.requiresFormatting) { - return '${placeholder!.name}String'; - } else { - return placeholder!.name; - } - } - } - - String get methodParameters { - assert(helper != null); - return dependentPlaceholders.map((Placeholder placeholder) => - (placeholder.requiresFormatting) - ? 'String ${placeholder.name}String' - : '${placeholder.type} ${placeholder.name}').join(', '); - } - - String get methodArguments { - assert(helper != null); - return dependentPlaceholders.map((Placeholder placeholder) => - (placeholder.requiresFormatting) - ? '${placeholder.name}String' - : placeholder.name).join(', '); - } -} diff --git a/packages/flutter_tools/lib/src/localizations/localizations_utils.dart b/packages/flutter_tools/lib/src/localizations/localizations_utils.dart index 048686db238c..c8e29d21b507 100644 --- a/packages/flutter_tools/lib/src/localizations/localizations_utils.dart +++ b/packages/flutter_tools/lib/src/localizations/localizations_utils.dart @@ -297,25 +297,23 @@ String generateString(String value) { /// Given a list of strings, placeholders, or helper function calls, concatenate /// them into one expression to be returned. -String generateReturnExpr(List helpers) { - if (helpers.isEmpty) { +/// If isSingleStringVar is passed, then we want to convert "'$expr'" to simply "expr". +String generateReturnExpr(List expressions, { bool isSingleStringVar = false }) { + if (expressions.isEmpty) { return "''"; - } else if ( - helpers.length == 1 - && helpers[0].string == null - && (helpers[0].placeholder?.type == 'String' || helpers[0].helper != null) - ) { - return helpers[0].helperOrPlaceholder; + } else if (isSingleStringVar) { + // If our expression is "$varName" where varName is a String, this is equivalent to just varName. + return expressions[0].substring(1); } else { - final String string = helpers.reversed.fold('', (String string, HelperMethod helper) { - if (helper.string != null) { - return generateString(helper.string!) + string; + final String string = expressions.reversed.fold('', (String string, String expression) { + if (expression[0] != r'$') { + return generateString(expression) + string; } final RegExp alphanumeric = RegExp(r'^([0-9a-zA-Z]|_)+$'); - if (alphanumeric.hasMatch(helper.helperOrPlaceholder) && !(string.isNotEmpty && alphanumeric.hasMatch(string[0]))) { - return '\$${helper.helperOrPlaceholder}$string'; + if (alphanumeric.hasMatch(expression.substring(1)) && !(string.isNotEmpty && alphanumeric.hasMatch(string[0]))) { + return '$expression$string'; } else { - return '\${${helper.helperOrPlaceholder}}$string'; + return '\${${expression.substring(1)}}$string'; } }); return "'$string'"; @@ -340,6 +338,7 @@ class LocalizationOptions { this.usesNullableGetter = true, this.format = false, this.useEscaping = false, + this.suppressWarnings = false, }) : assert(useSyntheticPackage != null); /// The `--arb-dir` argument. @@ -416,6 +415,11 @@ class LocalizationOptions { /// /// Whether or not the ICU escaping syntax is used. final bool useEscaping; + + /// The `suppress-warnings` argument. + /// + /// Whether or not to suppress warnings. + final bool suppressWarnings; } /// Parse the localizations configuration options from [file]. @@ -450,8 +454,9 @@ LocalizationOptions parseLocalizationsOptions({ useSyntheticPackage: _tryReadBool(yamlNode, 'synthetic-package', logger) ?? true, areResourceAttributesRequired: _tryReadBool(yamlNode, 'required-resource-attributes', logger) ?? false, usesNullableGetter: _tryReadBool(yamlNode, 'nullable-getter', logger) ?? true, - format: _tryReadBool(yamlNode, 'format', logger) ?? true, + format: _tryReadBool(yamlNode, 'format', logger) ?? false, useEscaping: _tryReadBool(yamlNode, 'use-escaping', logger) ?? false, + suppressWarnings: _tryReadBool(yamlNode, 'suppress-warnings', logger) ?? false, ); } diff --git a/packages/flutter_tools/lib/src/localizations/message_parser.dart b/packages/flutter_tools/lib/src/localizations/message_parser.dart index 3624f84a8c00..b4fbb139164a 100644 --- a/packages/flutter_tools/lib/src/localizations/message_parser.dart +++ b/packages/flutter_tools/lib/src/localizations/message_parser.dart @@ -6,6 +6,7 @@ // See https://flutter.dev/go/icu-message-parser. // Symbol Types +import '../base/logger.dart'; import 'gen_l10n_types.dart'; enum ST { @@ -181,13 +182,17 @@ class Parser { this.messageId, this.filename, this.messageString, - { this.useEscaping = false } + { + this.useEscaping = false, + this.logger + } ); final String messageId; final String messageString; final String filename; final bool useEscaping; + final Logger? logger; static String indentForError(int position) { return '${List.filled(position, ' ').join()}^'; @@ -297,6 +302,11 @@ class Parser { // Do not add whitespace as a token. startIndex = match.end; continue; + } else if ([ST.plural, ST.select].contains(matchedType) && tokens.last.type == ST.openBrace) { + // Treat "plural" or "select" as identifier if it comes right after an open brace. + tokens.add(Node(ST.identifier, startIndex, value: match.group(0))); + startIndex = match.end; + continue; } else { tokens.add(Node(matchedType!, startIndex, value: match.group(0))); startIndex = match.end; @@ -566,8 +576,13 @@ class Parser { } Node parse() { - final Node syntaxTree = compress(parseIntoTree()); - checkExtraRules(syntaxTree); - return syntaxTree; + try { + final Node syntaxTree = compress(parseIntoTree()); + checkExtraRules(syntaxTree); + return syntaxTree; + } on L10nParserException catch (error) { + logger?.printError(error.toString()); + return Node(ST.empty, 0, value: ''); + } } } 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 33fc53ebc889..18ca5227b494 100644 --- a/packages/flutter_tools/test/general.shard/generate_localizations_test.dart +++ b/packages/flutter_tools/test/general.shard/generate_localizations_test.dart @@ -1284,6 +1284,40 @@ class AppLocalizationsEn extends AppLocalizations { }); group('writeOutputFiles', () { + testWithoutContext('multiple messages with syntax error all log their errors', () { + final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') + ..createSync(recursive: true); + l10nDirectory.childFile(defaultTemplateArbFileName) + .writeAsStringSync(r''' +{ + "msg1": "{", + "msg2": "{ {" +}'''); + l10nDirectory.childFile(esArbFileName) + .writeAsStringSync(singleEsMessageArbFileString); + try { + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + ) + ..loadResources() + ..writeOutputFiles(); + } on L10nException catch (error) { + expect(error.message, equals('Found syntax errors.')); + expect(logger.errorText, contains(''' +[app_en.arb:msg1] ICU Syntax Error: Expected "identifier" but found no tokens. + { + ^ +[app_en.arb:msg2] ICU Syntax Error: Expected "identifier" but found "{". + { { + ^''')); + } + }); testWithoutContext('message without placeholders - should generate code comment with description and template message translation', () { _standardFlutterDirectoryL10nSetup(fs); LocalizationsGenerator( @@ -1581,47 +1615,31 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e }); group('placeholder tests', () { - testWithoutContext('should throw attempting to generate a select message without placeholders', () { - const String selectMessageWithoutPlaceholdersAttribute = ''' + testWithoutContext('should automatically infer placeholders that are not explicitly defined', () { + const String messageWithoutDefinedPlaceholder = ''' { - "helloWorld": "Hello {name}", - "@helloWorld": { - "description": "Improperly formatted since it has no placeholder attribute.", - "placeholders": { - "hello": {}, - "world": {} - } - } + "helloWorld": "Hello {name}" }'''; final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') ..createSync(recursive: true); l10nDirectory.childFile(defaultTemplateArbFileName) - .writeAsStringSync(selectMessageWithoutPlaceholdersAttribute); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Make sure that the specified placeholder is defined in your arb file. -[app_en.arb:helloWorld] Hello {name} - ^'''), - )), - ); + .writeAsStringSync(messageWithoutDefinedPlaceholder); + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + ) + ..loadResources() + ..writeOutputFiles(); + final String localizationsFile = fs.file( + fs.path.join(syntheticL10nPackagePath, 'output-localization-file_en.dart'), + ).readAsStringSync(); + expect(localizationsFile, contains('String helloWorld(Object name) {')); }); }); @@ -1909,119 +1927,64 @@ Make sure that the specified placeholder is defined in your arb file. }); group('plural messages', () { - testWithoutContext('should throw attempting to generate a plural message without placeholders', () { - const String pluralMessageWithoutPlaceholdersAttribute = ''' + testWithoutContext('warnings are generated when plural parts are repeated', () { + const String pluralMessageWithOverriddenParts = ''' { - "helloWorlds": "{count,plural, =0{Hello}=1{Hello World}=2{Hello two worlds}few{Hello {count} worlds}many{Hello all {count} worlds}other{Hello other {count} worlds}}", + "helloWorlds": "{count,plural, =0{Hello}zero{hello} other{hi}}", "@helloWorlds": { - "description": "Improperly formatted since it has no placeholder attribute." + "description": "Properly formatted but has redundant zero cases." } }'''; - final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') ..createSync(recursive: true); l10nDirectory.childFile(defaultTemplateArbFileName) - .writeAsStringSync(pluralMessageWithoutPlaceholdersAttribute); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Make sure that the specified plural placeholder is defined in your arb file. -[app_en.arb:helloWorlds] {count,plural, =0{Hello}=1{Hello World}=2{Hello two worlds}few{Hello {count} worlds}many{Hello all {count} worlds}other{Hello other {count} worlds}} - ^'''), - )), - ); + .writeAsStringSync(pluralMessageWithOverriddenParts); + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + ) + ..loadResources() + ..writeOutputFiles(); + expect(logger.hadWarningOutput, isTrue); + expect(logger.warningText, contains(''' +[app_en.arb:helloWorlds] ICU Syntax Warning: The plural part specified below is overridden by a later plural part. + {count,plural, =0{Hello}zero{hello} other{hi}} + ^''')); }); - testWithoutContext('should throw attempting to generate a plural message with an empty placeholders map', () { - const String pluralMessageWithEmptyPlaceholdersMap = ''' + testWithoutContext('should automatically infer plural placeholders that are not explicitly defined', () { + const String pluralMessageWithoutPlaceholdersAttribute = ''' { "helloWorlds": "{count,plural, =0{Hello}=1{Hello World}=2{Hello two worlds}few{Hello {count} worlds}many{Hello all {count} worlds}other{Hello other {count} worlds}}", "@helloWorlds": { - "description": "Improperly formatted since it has no placeholder attribute.", - "placeholders": {} + "description": "Improperly formatted since it has no placeholder attribute." } }'''; final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') ..createSync(recursive: true); l10nDirectory.childFile(defaultTemplateArbFileName) - .writeAsStringSync(pluralMessageWithEmptyPlaceholdersMap); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Make sure that the specified plural placeholder is defined in your arb file. -[app_en.arb:helloWorlds] {count,plural, =0{Hello}=1{Hello World}=2{Hello two worlds}few{Hello {count} worlds}many{Hello all {count} worlds}other{Hello other {count} worlds}} - ^'''), - )), - ); - }); - - testWithoutContext('should throw attempting to generate a plural message with no resource attributes', () { - const String pluralMessageWithoutResourceAttributes = ''' -{ - "helloWorlds": "{count,plural, =0{Hello}=1{Hello World}=2{Hello two worlds}few{Hello {count} worlds}many{Hello all {count} worlds}other{Hello other {count} worlds}}" -}'''; - - final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') - ..createSync(recursive: true); - l10nDirectory.childFile(defaultTemplateArbFileName) - .writeAsStringSync(pluralMessageWithoutResourceAttributes); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Make sure that the specified plural placeholder is defined in your arb file. -[app_en.arb:helloWorlds] {count,plural, =0{Hello}=1{Hello World}=2{Hello two worlds}few{Hello {count} worlds}many{Hello all {count} worlds}other{Hello other {count} worlds}} - ^'''), - )), - ); + .writeAsStringSync(pluralMessageWithoutPlaceholdersAttribute); + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + ) + ..loadResources() + ..writeOutputFiles(); + final String localizationsFile = fs.file( + fs.path.join(syntheticL10nPackagePath, 'output-localization-file_en.dart'), + ).readAsStringSync(); + expect(localizationsFile, contains('String helloWorlds(num count) {')); }); testWithoutContext('should throw attempting to generate a plural message with incorrect format for placeholders', () { @@ -2065,7 +2028,7 @@ Make sure that the specified plural placeholder is defined in your arb file. }); group('select messages', () { - testWithoutContext('should throw attempting to generate a select message without placeholders', () { + testWithoutContext('should auotmatically infer select placeholders that are not explicitly defined', () { const String selectMessageWithoutPlaceholdersAttribute = ''' { "genderSelect": "{gender, select, female {She} male {He} other {they} }", @@ -2078,106 +2041,21 @@ Make sure that the specified plural placeholder is defined in your arb file. ..createSync(recursive: true); l10nDirectory.childFile(defaultTemplateArbFileName) .writeAsStringSync(selectMessageWithoutPlaceholdersAttribute); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Make sure that the specified select placeholder is defined in your arb file. -[app_en.arb:genderSelect] {gender, select, female {She} male {He} other {they} } - ^'''), - )), - ); - }); - - testWithoutContext('should throw attempting to generate a select message with an empty placeholders map', () { - const String selectMessageWithEmptyPlaceholdersMap = ''' -{ - "genderSelect": "{gender, select, female {She} male {He} other {they} }", - "@genderSelect": { - "description": "Improperly formatted since it has no placeholder attribute.", - "placeholders": {} - } -}'''; - - final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') - ..createSync(recursive: true); - l10nDirectory.childFile(defaultTemplateArbFileName) - .writeAsStringSync(selectMessageWithEmptyPlaceholdersMap); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Make sure that the specified select placeholder is defined in your arb file. -[app_en.arb:genderSelect] {gender, select, female {She} male {He} other {they} } - ^'''), - )), - ); - }); - - testWithoutContext('should throw attempting to generate a select message with no resource attributes', () { - const String selectMessageWithoutResourceAttributes = ''' -{ - "genderSelect": "{gender, select, female {She} male {He} other {they} }" -}'''; - - final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') - ..createSync(recursive: true); - l10nDirectory.childFile(defaultTemplateArbFileName) - .writeAsStringSync(selectMessageWithoutResourceAttributes); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Make sure that the specified select placeholder is defined in your arb file. -[app_en.arb:genderSelect] {gender, select, female {She} male {He} other {they} } - ^'''), - )), - ); + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + ) + ..loadResources() + ..writeOutputFiles(); + final String localizationsFile = fs.file( + fs.path.join(syntheticL10nPackagePath, 'output-localization-file_en.dart'), + ).readAsStringSync(); + expect(localizationsFile, contains('String genderSelect(String gender) {')); }); testWithoutContext('should throw attempting to generate a select message with incorrect format for placeholders', () { @@ -2234,30 +2112,25 @@ Make sure that the specified select placeholder is defined in your arb file. ..createSync(recursive: true); l10nDirectory.childFile(defaultTemplateArbFileName) .writeAsStringSync(selectMessageWithoutPlaceholdersAttribute); - - expect( - () { - LocalizationsGenerator( - fileSystem: fs, - inputPathString: defaultL10nPathString, - outputPathString: defaultL10nPathString, - templateArbFileName: defaultTemplateArbFileName, - outputFileString: defaultOutputFileString, - classNameString: defaultClassNameString, - logger: logger, - ) - ..loadResources() - ..writeOutputFiles(); - }, - throwsA(isA().having( - (L10nException e) => e.message, - 'message', - contains(''' -Select expressions must have an "other" case. -[app_en.arb:genderSelect] {gender, select,} - ^'''), - )), - ); + try { + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + ) + ..loadResources() + ..writeOutputFiles(); + } on L10nException { + expect(logger.errorText, contains(''' +[app_en.arb:genderSelect] ICU Syntax Error: Select expressions must have an "other" case. + {gender, select,} + ^''') + ); + } }); }); @@ -2984,37 +2857,66 @@ AppLocalizations lookupAppLocalizations(Locale locale) { ''')); }); - // TODO(thkim1011): Uncomment when implementing escaping. - // See https://github.com/flutter/flutter/issues/113455. -// testWithoutContext('escaping with single quotes', () { -// const String arbFile = ''' -// { -// "singleQuote": "Flutter''s amazing!", -// "@singleQuote": { -// "description": "A message with a single quote." -// } -// }'''; - -// final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') -// ..createSync(recursive: true); -// l10nDirectory.childFile(defaultTemplateArbFileName) -// .writeAsStringSync(arbFile); - -// LocalizationsGenerator( -// fileSystem: fs, -// inputPathString: defaultL10nPathString, -// outputPathString: defaultL10nPathString, -// templateArbFileName: defaultTemplateArbFileName, -// outputFileString: defaultOutputFileString, -// classNameString: defaultClassNameString, -// logger: logger, -// ) -// ..loadResources() -// ..writeOutputFiles(); - -// final String localizationsFile = fs.file( -// fs.path.join(syntheticL10nPackagePath, 'output-localization-file_en.dart'), -// ).readAsStringSync(); -// expect(localizationsFile, contains(r"Flutter\'s amazing")); -// }); + testWithoutContext('escaping with single quotes', () { + const String arbFile = ''' +{ + "singleQuote": "Flutter''s amazing!", + "@singleQuote": { + "description": "A message with a single quote." + } +}'''; + + final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') + ..createSync(recursive: true); + l10nDirectory.childFile(defaultTemplateArbFileName) + .writeAsStringSync(arbFile); + + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + useEscaping: true, + ) + ..loadResources() + ..writeOutputFiles(); + + final String localizationsFile = fs.file( + fs.path.join(syntheticL10nPackagePath, 'output-localization-file_en.dart'), + ).readAsStringSync(); + expect(localizationsFile, contains(r"Flutter\'s amazing")); + }); + + testWithoutContext('suppress warnings flag actually suppresses warnings', () { + const String pluralMessageWithOverriddenParts = ''' +{ + "helloWorlds": "{count,plural, =0{Hello}zero{hello} other{hi}}", + "@helloWorlds": { + "description": "Properly formatted but has redundant zero cases.", + "placeholders": { + "count": {} + } + } +}'''; + final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n') + ..createSync(recursive: true); + l10nDirectory.childFile(defaultTemplateArbFileName) + .writeAsStringSync(pluralMessageWithOverriddenParts); + LocalizationsGenerator( + fileSystem: fs, + inputPathString: defaultL10nPathString, + outputPathString: defaultL10nPathString, + templateArbFileName: defaultTemplateArbFileName, + outputFileString: defaultOutputFileString, + classNameString: defaultClassNameString, + logger: logger, + suppressWarnings: true, + ) + ..loadResources() + ..writeOutputFiles(); + expect(logger.hadWarningOutput, isFalse); + }); } diff --git a/packages/flutter_tools/test/general.shard/message_parser_test.dart b/packages/flutter_tools/test/general.shard/message_parser_test.dart index ddeb60c932bc..851d4ad59809 100644 --- a/packages/flutter_tools/test/general.shard/message_parser_test.dart +++ b/packages/flutter_tools/test/general.shard/message_parser_test.dart @@ -218,6 +218,14 @@ void main() { ])); }); + testWithoutContext('lexer identifier names can be "select" or "plural"', () { + final List tokens = Parser('keywords', 'app_en.arb', '{ select } { plural, select, singular{test} other{hmm} }').lexIntoTokens(); + expect(tokens[1].value, equals('select')); + expect(tokens[1].type, equals(ST.identifier)); + expect(tokens[5].value, equals('plural')); + expect(tokens[5].type, equals(ST.identifier)); + }); + testWithoutContext('lexer: lexically correct but syntactically incorrect', () { final List tokens = Parser( 'syntax', @@ -242,9 +250,9 @@ void main() { testWithoutContext('lexer unmatched single quote', () { const String message = "here''s an unmatched single quote: '"; const String expectedError = ''' -ICU Lexing Error: Unmatched single quotes. -[app_en.arb:escaping] here''s an unmatched single quote: ' - ^'''; +[app_en.arb:escaping] ICU Lexing Error: Unmatched single quotes. + here''s an unmatched single quote: ' + ^'''; expect( () => Parser('escaping', 'app_en.arb', message, useEscaping: true).lexIntoTokens(), throwsA(isA().having( @@ -257,9 +265,9 @@ ICU Lexing Error: Unmatched single quotes. testWithoutContext('lexer unexpected character', () { const String message = '{ * }'; const String expectedError = ''' -ICU Lexing Error: Unexpected character. -[app_en.arb:lex] { * } - ^'''; +[app_en.arb:lex] ICU Lexing Error: Unexpected character. + { * } + ^'''; expect( () => Parser('lex', 'app_en.arb', message).lexIntoTokens(), throwsA(isA().having( @@ -460,11 +468,11 @@ ICU Lexing Error: Unexpected character. testWithoutContext('parser unexpected token', () { // unexpected token const String expectedError1 = ''' -ICU Syntax Error: Expected "}" but found "=". -[app_en.arb:unexpectedToken] { placeholder = - ^'''; +[app_en.arb:unexpectedToken] ICU Syntax Error: Expected "}" but found "=". + { placeholder = + ^'''; expect( - () => Parser('unexpectedToken', 'app_en.arb', '{ placeholder =').parse(), + () => Parser('unexpectedToken', 'app_en.arb', '{ placeholder =').parseIntoTree(), throwsA(isA().having( (L10nException e) => e.message, 'message', @@ -472,11 +480,11 @@ ICU Syntax Error: Expected "}" but found "=". ))); const String expectedError2 = ''' -ICU Syntax Error: Expected "number" but found "}". -[app_en.arb:unexpectedToken] { count, plural, = } - ^'''; +[app_en.arb:unexpectedToken] ICU Syntax Error: Expected "number" but found "}". + { count, plural, = } + ^'''; expect( - () => Parser('unexpectedToken', 'app_en.arb', '{ count, plural, = }').parse(), + () => Parser('unexpectedToken', 'app_en.arb', '{ count, plural, = }').parseIntoTree(), throwsA(isA().having( (L10nException e) => e.message, 'message', @@ -484,11 +492,11 @@ ICU Syntax Error: Expected "number" but found "}". ))); const String expectedError3 = ''' -ICU Syntax Error: Expected "identifier" but found ",". -[app_en.arb:unexpectedToken] { , plural , = } - ^'''; +[app_en.arb:unexpectedToken] ICU Syntax Error: Expected "identifier" but found ",". + { , plural , = } + ^'''; expect( - () => Parser('unexpectedToken', 'app_en.arb', '{ , plural , = }').parse(), + () => Parser('unexpectedToken', 'app_en.arb', '{ , plural , = }').parseIntoTree(), throwsA(isA().having( (L10nException e) => e.message, 'message',