firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add Escaping Option for ICU MessageFormat Syntax (#116137)

Browse Source 
* init

* make more descriptive

* fix lint
This commit is contained in:
Tae Hyung Kim 2022-11-28 19:35:06 -08:00 committed by GitHub
parent d6995aa249
commit 39a73cabed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 133 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
packages/flutter_tools/lib/src/commands/generate_localizations.dart
View File

@ -192,6 +192,15 @@ class GenerateLocalizationsCommand extends FlutterCommand {
'format', 'format',
help: 'When specified, the "dart format" command is run after generating the localization files.' help: 'When specified, the "dart format" command is run after generating the localization files.'
); );
argParser.addFlag(
'use-escaping',
help: 'Whether or not to use escaping for messages.\n'
'\n'
'By default, this value is set to false for backwards compatibility. '
'Turning this flag on will cause the parser to treat any special characters '
'contained within pairs of single quotes as normal strings and treat all '
'consecutive pairs of single quotes as a single quote character.',
);
} }
final FileSystem _fileSystem; final FileSystem _fileSystem;
@ -248,6 +257,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
final String? projectPathString = stringArgDeprecated('project-dir'); final String? projectPathString = stringArgDeprecated('project-dir');
final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes'); final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes');
final bool usesNullableGetter = boolArgDeprecated('nullable-getter'); final bool usesNullableGetter = boolArgDeprecated('nullable-getter');
final bool useEscaping = boolArgDeprecated('use-escaping');
precacheLanguageAndRegionTags(); precacheLanguageAndRegionTags();
@ -269,6 +279,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
areResourceAttributesRequired: areResourceAttributesRequired, areResourceAttributesRequired: areResourceAttributesRequired,
untranslatedMessagesFile: untranslatedMessagesFile, untranslatedMessagesFile: untranslatedMessagesFile,
usesNullableGetter: usesNullableGetter, usesNullableGetter: usesNullableGetter,
useEscaping: useEscaping,
logger: _logger, logger: _logger,
) )
..loadResources() ..loadResources()

9
packages/flutter_tools/lib/src/localizations/gen_l10n.dart
View File

@ -65,6 +65,7 @@ LocalizationsGenerator generateLocalizations({
areResourceAttributesRequired: options.areResourceAttributesRequired, areResourceAttributesRequired: options.areResourceAttributesRequired,
untranslatedMessagesFile: options.untranslatedMessagesFile?.toFilePath(), untranslatedMessagesFile: options.untranslatedMessagesFile?.toFilePath(),
usesNullableGetter: options.usesNullableGetter, usesNullableGetter: options.usesNullableGetter,
useEscaping: options.useEscaping,
logger: logger, logger: logger,
) )
..loadResources() ..loadResources()
@ -453,6 +454,7 @@ class LocalizationsGenerator {
bool areResourceAttributesRequired = false, bool areResourceAttributesRequired = false,
String? untranslatedMessagesFile, String? untranslatedMessagesFile,
bool usesNullableGetter = true, bool usesNullableGetter = true,
bool useEscaping = false,
required Logger logger, required Logger logger,
}) { }) {
final Directory? projectDirectory = projectDirFromPath(fileSystem, projectPathString); final Directory? projectDirectory = projectDirFromPath(fileSystem, projectPathString);
@ -474,6 +476,7 @@ class LocalizationsGenerator {
untranslatedMessagesFile: _untranslatedMessagesFileFromPath(fileSystem, untranslatedMessagesFile), untranslatedMessagesFile: _untranslatedMessagesFileFromPath(fileSystem, untranslatedMessagesFile),
inputsAndOutputsListFile: _inputsAndOutputsListFileFromPath(fileSystem, inputsAndOutputsListPath), inputsAndOutputsListFile: _inputsAndOutputsListFileFromPath(fileSystem, inputsAndOutputsListPath),
areResourceAttributesRequired: areResourceAttributesRequired, areResourceAttributesRequired: areResourceAttributesRequired,
useEscaping: useEscaping,
logger: logger, logger: logger,
); );
} }
@ -497,6 +500,7 @@ class LocalizationsGenerator {
this.untranslatedMessagesFile, this.untranslatedMessagesFile,
this.usesNullableGetter = true, this.usesNullableGetter = true,
required this.logger, required this.logger,
this.useEscaping = false,
}); });
final FileSystem _fs; final FileSystem _fs;
@ -566,6 +570,9 @@ class LocalizationsGenerator {
// all of the messages. // all of the messages.
bool requiresIntlImport = false; bool requiresIntlImport = false;
// Whether we want to use escaping for ICU messages.
bool useEscaping = false;
/// The list of all arb path strings in [inputDirectory]. /// The list of all arb path strings in [inputDirectory].
List<String> get arbPathStrings { List<String> get arbPathStrings {
return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList(); return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList();
@ -845,7 +852,7 @@ class LocalizationsGenerator {
// files in inputDirectory. Also initialized: supportedLocales. // files in inputDirectory. Also initialized: supportedLocales.
void loadResources() { void loadResources() {
_allMessages = _templateBundle.resourceIds.map((String id) => Message( _allMessages = _templateBundle.resourceIds.map((String id) => Message(
_templateBundle, _allBundles, id, areResourceAttributesRequired, _templateBundle, _allBundles, id, areResourceAttributesRequired, useEscaping: useEscaping,
)); ));
for (final String resourceId in _templateBundle.resourceIds) { for (final String resourceId in _templateBundle.resourceIds) {
if (!_isValidGetterAndMethodName(resourceId)) { if (!_isValidGetterAndMethodName(resourceId)) {

6
packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart
View File

@ -318,7 +318,8 @@ class Message {
AppResourceBundle templateBundle, AppResourceBundle templateBundle,
AppResourceBundleCollection allBundles, AppResourceBundleCollection allBundles,
this.resourceId, this.resourceId,
bool isResourceAttributeRequired bool isResourceAttributeRequired,
{ this.useEscaping = false }
) : assert(templateBundle != null), ) : assert(templateBundle != null),
assert(allBundles != null), assert(allBundles != null),
assert(resourceId != null && resourceId.isNotEmpty), assert(resourceId != null && resourceId.isNotEmpty),
@ -334,7 +335,7 @@ class Message {
filenames[bundle.locale] = bundle.file.basename; filenames[bundle.locale] = bundle.file.basename;
final String? translation = bundle.translationFor(resourceId); final String? translation = bundle.translationFor(resourceId);
messages[bundle.locale] = translation; messages[bundle.locale] = translation;
parsedMessages[bundle.locale] = translation == null ? null : Parser(resourceId, bundle.file.basename, translation).parse(); 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. // Using parsed translations, attempt to infer types of placeholders used by plurals and selects.
for (final LocaleInfo locale in parsedMessages.keys) { for (final LocaleInfo locale in parsedMessages.keys) {
@ -400,6 +401,7 @@ class Message {
late final Map<LocaleInfo, String?> messages; late final Map<LocaleInfo, String?> messages;
final Map<LocaleInfo, Node?> parsedMessages; final Map<LocaleInfo, Node?> parsedMessages;
final Map<String, Placeholder> placeholders; final Map<String, Placeholder> placeholders;
final bool useEscaping;
bool get placeholdersRequireFormatting => placeholders.values.any((Placeholder p) => p.requiresFormatting); bool get placeholdersRequireFormatting => placeholders.values.any((Placeholder p) => p.requiresFormatting);

7
packages/flutter_tools/lib/src/localizations/localizations_utils.dart
View File

@ -339,6 +339,7 @@ class LocalizationOptions {
this.areResourceAttributesRequired = false, this.areResourceAttributesRequired = false,
this.usesNullableGetter = true, this.usesNullableGetter = true,
this.format = false, this.format = false,
this.useEscaping = false,
}) : assert(useSyntheticPackage != null); }) : assert(useSyntheticPackage != null);
/// The `--arb-dir` argument. /// The `--arb-dir` argument.
@ -410,6 +411,11 @@ class LocalizationOptions {
/// ///
/// Whether or not to format the generated files. /// Whether or not to format the generated files.
final bool format; final bool format;
/// The `use-escaping` argument.
///
/// Whether or not the ICU escaping syntax is used.
final bool useEscaping;
} }
/// Parse the localizations configuration options from [file]. /// Parse the localizations configuration options from [file].
@ -445,6 +451,7 @@ LocalizationOptions parseLocalizationsOptions({
areResourceAttributesRequired: _tryReadBool(yamlNode, 'required-resource-attributes', logger) ?? false, areResourceAttributesRequired: _tryReadBool(yamlNode, 'required-resource-attributes', logger) ?? false,
usesNullableGetter: _tryReadBool(yamlNode, 'nullable-getter', logger) ?? true, usesNullableGetter: _tryReadBool(yamlNode, 'nullable-getter', logger) ?? true,
format: _tryReadBool(yamlNode, 'format', logger) ?? true, format: _tryReadBool(yamlNode, 'format', logger) ?? true,
useEscaping: _tryReadBool(yamlNode, 'use-escaping', logger) ?? false,
); );
} }

62
packages/flutter_tools/lib/src/localizations/message_parser.dart
View File

@ -149,7 +149,10 @@ $indent])''';
} }
} }
RegExp unescapedString = RegExp(r'[^{}]+'); RegExp escapedString = RegExp(r"'[^']*'");
RegExp unescapedString = RegExp(r"[^{}']+");
RegExp normalString = RegExp(r'[^{}]+');
RegExp brace = RegExp(r'{|}'); RegExp brace = RegExp(r'{|}');
RegExp whitespace = RegExp(r'\s+'); RegExp whitespace = RegExp(r'\s+');
@ -174,11 +177,17 @@ Map<ST, RegExp> matchers = <ST, RegExp>{
}; };
class Parser { class Parser {
Parser(this.messageId, this.filename, this.messageString); Parser(
this.messageId,
this.filename,
this.messageString,
{ this.useEscaping = false }
);
final String messageId; final String messageId;
final String messageString; final String messageString;
final String filename; final String filename;
final bool useEscaping;
static String indentForError(int position) { static String indentForError(int position) {
return '${List<String>.filled(position, ' ').join()}^'; return '${List<String>.filled(position, ' ').join()}^';
@ -201,20 +210,41 @@ class Parser {
while (startIndex < messageString.length) { while (startIndex < messageString.length) {
Match? match; Match? match;
if (isString) { if (isString) {
// TODO(thkim1011): Uncomment this when we add escaping as an option. if (useEscaping) {
// See https://github.com/flutter/flutter/issues/113455. // This case is slightly involved. Essentially, wrapping any syntax in
// match = escapedString.matchAsPrefix(message, startIndex); // single quotes escapes the syntax except when there are consecutive pair of single
// if (match != null) { // quotes. For example, "Hello! 'Flutter''s amazing'. { unescapedPlaceholder }"
// final String string = match.group(0)!; // converts the '' in "Flutter's" to a single quote for convenience, since technically,
// tokens.add(Node.string(startIndex, string == "''" ? "'" : string.substring(1, string.length - 1))); // we should interpret this as two strings 'Flutter' and 's amazing'. To get around this,
// startIndex = match.end; // we also check if the previous character is a ', and if so, add a single quote at the beginning
// continue; // of the token.
// } match = escapedString.matchAsPrefix(messageString, startIndex);
match = unescapedString.matchAsPrefix(messageString, startIndex); if (match != null) {
if (match != null) { final String string = match.group(0)!;
tokens.add(Node.string(startIndex, match.group(0)!)); if (string == "''") {
startIndex = match.end; tokens.add(Node.string(startIndex, "'"));
continue; } else if (startIndex > 1 && messageString[startIndex - 1] == "'") {
// Include a single quote in the beginning of the token.
tokens.add(Node.string(startIndex, string.substring(0, string.length - 1)));
} else {
tokens.add(Node.string(startIndex, string.substring(1, string.length - 1)));
}
startIndex = match.end;
continue;
}
match = unescapedString.matchAsPrefix(messageString, startIndex);
if (match != null) {
tokens.add(Node.string(startIndex, match.group(0)!));
startIndex = match.end;
continue;
}
} else {
match = normalString.matchAsPrefix(messageString, startIndex);
if (match != null) {
tokens.add(Node.string(startIndex, match.group(0)!));
startIndex = match.end;
continue;
}
} }
match = brace.matchAsPrefix(messageString, startIndex); match = brace.matchAsPrefix(messageString, startIndex);
if (match != null) { if (match != null) {

107
packages/flutter_tools/test/general.shard/message_parser_test.dart
View File

@ -187,32 +187,36 @@ void main() {
])); ]));
}); });
// TODO(thkim1011): Uncomment when implementing escaping. testWithoutContext('lexer escaping', () {
// See https://github.com/flutter/flutter/issues/113455. final List<Node> tokens1 = Parser('escaping', 'app_en.arb', "''", useEscaping: true).lexIntoTokens();
// testWithoutContext('lexer escaping', () { expect(tokens1, equals(<Node>[Node.string(0, "'")]));
// final List<Node> tokens1 = Parser("''").lexIntoTokens();
// expect(tokens1, equals(<Node>[Node.string(0, "'")]));
// final List<Node> tokens2 = Parser("'hello world { name }'").lexIntoTokens(); final List<Node> tokens2 = Parser('escaping', 'app_en.arb', "'hello world { name }'", useEscaping: true).lexIntoTokens();
// expect(tokens2, equals(<Node>[Node.string(0, 'hello world { name }')])); expect(tokens2, equals(<Node>[Node.string(0, 'hello world { name }')]));
// final List<Node> tokens3 = Parser("'{ escaped string }' { not escaped }").lexIntoTokens(); final List<Node> tokens3 = Parser('escaping', 'app_en.arb', "'{ escaped string }' { not escaped }", useEscaping: true).lexIntoTokens();
// expect(tokens3, equals(<Node>[ expect(tokens3, equals(<Node>[
// Node.string(0, '{ escaped string }'), Node.string(0, '{ escaped string }'),
// Node.string(20, ' '), Node.string(20, ' '),
// Node.openBrace(21), Node.openBrace(21),
// Node.identifier(23, 'not'), Node.identifier(23, 'not'),
// Node.identifier(27, 'escaped'), Node.identifier(27, 'escaped'),
// Node.closeBrace(35), Node.closeBrace(35),
// ])); ]));
// final List<Node> tokens4 = Parser("Flutter''s amazing!").lexIntoTokens(); final List<Node> tokens4 = Parser('escaping', 'app_en.arb', "Flutter''s amazing!", useEscaping: true).lexIntoTokens();
// expect(tokens4, equals(<Node>[ expect(tokens4, equals(<Node>[
// Node.string(0, 'Flutter'), Node.string(0, 'Flutter'),
// Node.string(7, "'"), Node.string(7, "'"),
// Node.string(9, 's amazing!'), Node.string(9, 's amazing!'),
// ])); ]));
// });
final List<Node> tokens5 = Parser('escaping', 'app_en.arb', "'Flutter''s amazing!'", useEscaping: true).lexIntoTokens();
expect(tokens5, equals(<Node>[
Node(ST.string, 0, value: 'Flutter'),
Node(ST.string, 9, value: "'s amazing!"),
]));
});
testWithoutContext('lexer: lexically correct but syntactically incorrect', () { testWithoutContext('lexer: lexically correct but syntactically incorrect', () {
final List<Node> tokens = Parser( final List<Node> tokens = Parser(
@ -235,22 +239,20 @@ void main() {
])); ]));
}); });
// TODO(thkim1011): Uncomment when implementing escaping. testWithoutContext('lexer unmatched single quote', () {
// See https://github.com/flutter/flutter/issues/113455. const String message = "here''s an unmatched single quote: '";
// testWithoutContext('lexer unmatched single quote', () { const String expectedError = '''
// const String message = "here''s an unmatched single quote: '"; ICU Lexing Error: Unmatched single quotes.
// const String expectedError = ''' [app_en.arb:escaping] here''s an unmatched single quote: '
// ICU Lexing Error: Unmatched single quotes. ^''';
// here''s an unmatched single quote: ' expect(
// ^'''; () => Parser('escaping', 'app_en.arb', message, useEscaping: true).lexIntoTokens(),
// expect( throwsA(isA<L10nException>().having(
// () => Parser(message).lexIntoTokens(), (L10nException e) => e.message,
// throwsA(isA<L10nException>().having( 'message',
// (L10nException e) => e.message, contains(expectedError),
// 'message', )));
// contains(expectedError), });
// )));
// });
testWithoutContext('lexer unexpected character', () { testWithoutContext('lexer unexpected character', () {
const String message = '{ * }'; const String message = '{ * }';
@ -367,17 +369,22 @@ ICU Lexing Error: Unexpected character.
)); ));
}); });
// TODO(thkim1011): Uncomment when implementing escaping. testWithoutContext('parser escaping', () {
// See https://github.com/flutter/flutter/issues/113455. expect(Parser('escaping', 'app_en.arb', "Flutter''s amazing!", useEscaping: true).parse(), equals(
// testWithoutContext('parser basic 2', () { Node(ST.message, 0, children: <Node>[
// expect(Parser("Flutter''s amazing!").parse(), equals( Node(ST.string, 0, value: 'Flutter'),
// Node(ST.message, 0, children: <Node>[ Node(ST.string, 7, value: "'"),
// Node(ST.string, 0, value: 'Flutter'), Node(ST.string, 9, value: 's amazing!'),
// Node(ST.string, 7, value: "'"), ])
// Node(ST.string, 9, value: 's amazing!'), ));
// ])
// )); expect(Parser('escaping', 'app_en.arb', "'Flutter''s amazing!'", useEscaping: true).parse(), equals(
// }); Node(ST.message, 0, children: <Node> [
Node(ST.string, 0, value: 'Flutter'),
Node(ST.string, 9, value: "'s amazing!"),
])
));
});
testWithoutContext('parser recursive', () { testWithoutContext('parser recursive', () {
expect(Parser( expect(Parser(