Skip to content

Commit 39a73ca

Browse files
thkim1011thkim1011
authored
Add Escaping Option for ICU MessageFormat Syntax (#116137)
* init * make more descriptive * fix lint
1 parent d6995aa commit 39a73ca

File tree

6 files changed

+133
-69
lines changed

6 files changed

+133
-69
lines changed

‎packages/flutter_tools/lib/src/commands/generate_localizations.dart‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,15 @@ class GenerateLocalizationsCommand extends FlutterCommand {
192192
'format',
193193
help: 'When specified, the "dart format" command is run after generating the localization files.'
194194
);
195+
argParser.addFlag(
196+
'use-escaping',
197+
help: 'Whether or not to use escaping for messages.\n'
198+
'\n'
199+
'By default, this value is set to false for backwards compatibility. '
200+
'Turning this flag on will cause the parser to treat any special characters '
201+
'contained within pairs of single quotes as normal strings and treat all '
202+
'consecutive pairs of single quotes as a single quote character.',
203+
);
195204
}
196205

197206
final FileSystem _fileSystem;
@@ -248,6 +257,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
248257
final String? projectPathString = stringArgDeprecated('project-dir');
249258
final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes');
250259
final bool usesNullableGetter = boolArgDeprecated('nullable-getter');
260+
final bool useEscaping = boolArgDeprecated('use-escaping');
251261

252262
precacheLanguageAndRegionTags();
253263

@@ -269,6 +279,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
269279
areResourceAttributesRequired: areResourceAttributesRequired,
270280
untranslatedMessagesFile: untranslatedMessagesFile,
271281
usesNullableGetter: usesNullableGetter,
282+
useEscaping: useEscaping,
272283
logger: _logger,
273284
)
274285
..loadResources()

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ LocalizationsGenerator generateLocalizations({
6565
areResourceAttributesRequired: options.areResourceAttributesRequired,
6666
untranslatedMessagesFile: options.untranslatedMessagesFile?.toFilePath(),
6767
usesNullableGetter: options.usesNullableGetter,
68+
useEscaping: options.useEscaping,
6869
logger: logger,
6970
)
7071
..loadResources()
@@ -453,6 +454,7 @@ class LocalizationsGenerator {
453454
bool areResourceAttributesRequired = false,
454455
String? untranslatedMessagesFile,
455456
bool usesNullableGetter = true,
457+
bool useEscaping = false,
456458
required Logger logger,
457459
}) {
458460
final Directory? projectDirectory = projectDirFromPath(fileSystem, projectPathString);
@@ -474,6 +476,7 @@ class LocalizationsGenerator {
474476
untranslatedMessagesFile: _untranslatedMessagesFileFromPath(fileSystem, untranslatedMessagesFile),
475477
inputsAndOutputsListFile: _inputsAndOutputsListFileFromPath(fileSystem, inputsAndOutputsListPath),
476478
areResourceAttributesRequired: areResourceAttributesRequired,
479+
useEscaping: useEscaping,
477480
logger: logger,
478481
);
479482
}
@@ -497,6 +500,7 @@ class LocalizationsGenerator {
497500
this.untranslatedMessagesFile,
498501
this.usesNullableGetter = true,
499502
required this.logger,
503+
this.useEscaping = false,
500504
});
501505

502506
final FileSystem _fs;
@@ -566,6 +570,9 @@ class LocalizationsGenerator {
566570
// all of the messages.
567571
bool requiresIntlImport = false;
568572

573+
// Whether we want to use escaping for ICU messages.
574+
bool useEscaping = false;
575+
569576
/// The list of all arb path strings in [inputDirectory].
570577
List<String> get arbPathStrings {
571578
return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList();
@@ -845,7 +852,7 @@ class LocalizationsGenerator {
845852
// files in inputDirectory. Also initialized: supportedLocales.
846853
void loadResources() {
847854
_allMessages = _templateBundle.resourceIds.map((String id) => Message(
848-
_templateBundle, _allBundles, id, areResourceAttributesRequired,
855+
_templateBundle, _allBundles, id, areResourceAttributesRequired, useEscaping: useEscaping,
849856
));
850857
for (final String resourceId in _templateBundle.resourceIds) {
851858
if (!_isValidGetterAndMethodName(resourceId)) {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,8 @@ class Message {
318318
AppResourceBundle templateBundle,
319319
AppResourceBundleCollection allBundles,
320320
this.resourceId,
321-
bool isResourceAttributeRequired
321+
bool isResourceAttributeRequired,
322+
{ this.useEscaping = false }
322323
) : assert(templateBundle != null),
323324
assert(allBundles != null),
324325
assert(resourceId != null && resourceId.isNotEmpty),
@@ -334,7 +335,7 @@ class Message {
334335
filenames[bundle.locale] = bundle.file.basename;
335336
final String? translation = bundle.translationFor(resourceId);
336337
messages[bundle.locale] = translation;
337-
parsedMessages[bundle.locale] = translation == null ? null : Parser(resourceId, bundle.file.basename, translation).parse();
338+
parsedMessages[bundle.locale] = translation == null ? null : Parser(resourceId, bundle.file.basename, translation, useEscaping: useEscaping).parse();
338339
}
339340
// Using parsed translations, attempt to infer types of placeholders used by plurals and selects.
340341
for (final LocaleInfo locale in parsedMessages.keys) {
@@ -400,6 +401,7 @@ class Message {
400401
late final Map<LocaleInfo, String?> messages;
401402
final Map<LocaleInfo, Node?> parsedMessages;
402403
final Map<String, Placeholder> placeholders;
404+
final bool useEscaping;
403405

404406
bool get placeholdersRequireFormatting => placeholders.values.any((Placeholder p) => p.requiresFormatting);
405407

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ class LocalizationOptions {
339339
this.areResourceAttributesRequired = false,
340340
this.usesNullableGetter = true,
341341
this.format = false,
342+
this.useEscaping = false,
342343
}) : assert(useSyntheticPackage != null);
343344

344345
/// The `--arb-dir` argument.
@@ -410,6 +411,11 @@ class LocalizationOptions {
410411
///
411412
/// Whether or not to format the generated files.
412413
final bool format;
414+
415+
/// The `use-escaping` argument.
416+
///
417+
/// Whether or not the ICU escaping syntax is used.
418+
final bool useEscaping;
413419
}
414420

415421
/// Parse the localizations configuration options from [file].
@@ -445,6 +451,7 @@ LocalizationOptions parseLocalizationsOptions({
445451
areResourceAttributesRequired: _tryReadBool(yamlNode, 'required-resource-attributes', logger) ?? false,
446452
usesNullableGetter: _tryReadBool(yamlNode, 'nullable-getter', logger) ?? true,
447453
format: _tryReadBool(yamlNode, 'format', logger) ?? true,
454+
useEscaping: _tryReadBool(yamlNode, 'use-escaping', logger) ?? false,
448455
);
449456
}
450457

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

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ $indent])''';
149149
}
150150
}
151151

152-
RegExp unescapedString = RegExp(r'[^{}]+');
152+
RegExp escapedString = RegExp(r"'[^']*'");
153+
RegExp unescapedString = RegExp(r"[^{}']+");
154+
RegExp normalString = RegExp(r'[^{}]+');
155+
153156
RegExp brace = RegExp(r'{|}');
154157

155158
RegExp whitespace = RegExp(r'\s+');
@@ -174,11 +177,17 @@ Map<ST, RegExp> matchers = <ST, RegExp>{
174177
};
175178

176179
class Parser {
177-
Parser(this.messageId, this.filename, this.messageString);
180+
Parser(
181+
this.messageId,
182+
this.filename,
183+
this.messageString,
184+
{ this.useEscaping = false }
185+
);
178186

179187
final String messageId;
180188
final String messageString;
181189
final String filename;
190+
final bool useEscaping;
182191

183192
static String indentForError(int position) {
184193
return '${List<String>.filled(position, ' ').join()}^';
@@ -201,20 +210,41 @@ class Parser {
201210
while (startIndex < messageString.length) {
202211
Match? match;
203212
if (isString) {
204-
// TODO(thkim1011): Uncomment this when we add escaping as an option.
205-
// See https://github.com/flutter/flutter/issues/113455.
206-
// match = escapedString.matchAsPrefix(message, startIndex);
207-
// if (match != null) {
208-
// final String string = match.group(0)!;
209-
// tokens.add(Node.string(startIndex, string == "''" ? "'" : string.substring(1, string.length - 1)));
210-
// startIndex = match.end;
211-
// continue;
212-
// }
213-
match = unescapedString.matchAsPrefix(messageString, startIndex);
214-
if (match != null) {
215-
tokens.add(Node.string(startIndex, match.group(0)!));
216-
startIndex = match.end;
217-
continue;
213+
if (useEscaping) {
214+
// This case is slightly involved. Essentially, wrapping any syntax in
215+
// single quotes escapes the syntax except when there are consecutive pair of single
216+
// quotes. For example, "Hello! 'Flutter''s amazing'. { unescapedPlaceholder }"
217+
// converts the '' in "Flutter's" to a single quote for convenience, since technically,
218+
// we should interpret this as two strings 'Flutter' and 's amazing'. To get around this,
219+
// we also check if the previous character is a ', and if so, add a single quote at the beginning
220+
// of the token.
221+
match = escapedString.matchAsPrefix(messageString, startIndex);
222+
if (match != null) {
223+
final String string = match.group(0)!;
224+
if (string == "''") {
225+
tokens.add(Node.string(startIndex, "'"));
226+
} else if (startIndex > 1 && messageString[startIndex - 1] == "'") {
227+
// Include a single quote in the beginning of the token.
228+
tokens.add(Node.string(startIndex, string.substring(0, string.length - 1)));
229+
} else {
230+
tokens.add(Node.string(startIndex, string.substring(1, string.length - 1)));
231+
}
232+
startIndex = match.end;
233+
continue;
234+
}
235+
match = unescapedString.matchAsPrefix(messageString, startIndex);
236+
if (match != null) {
237+
tokens.add(Node.string(startIndex, match.group(0)!));
238+
startIndex = match.end;
239+
continue;
240+
}
241+
} else {
242+
match = normalString.matchAsPrefix(messageString, startIndex);
243+
if (match != null) {
244+
tokens.add(Node.string(startIndex, match.group(0)!));
245+
startIndex = match.end;
246+
continue;
247+
}
218248
}
219249
match = brace.matchAsPrefix(messageString, startIndex);
220250
if (match != null) {

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

Lines changed: 57 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -187,32 +187,36 @@ void main() {
187187
]));
188188
});
189189

190-
// TODO(thkim1011): Uncomment when implementing escaping.
191-
// See https://github.com/flutter/flutter/issues/113455.
192-
// testWithoutContext('lexer escaping', () {
193-
// final List<Node> tokens1 = Parser("''").lexIntoTokens();
194-
// expect(tokens1, equals(<Node>[Node.string(0, "'")]));
190+
testWithoutContext('lexer escaping', () {
191+
final List<Node> tokens1 = Parser('escaping', 'app_en.arb', "''", useEscaping: true).lexIntoTokens();
192+
expect(tokens1, equals(<Node>[Node.string(0, "'")]));
195193

196-
// final List<Node> tokens2 = Parser("'hello world { name }'").lexIntoTokens();
197-
// expect(tokens2, equals(<Node>[Node.string(0, 'hello world { name }')]));
194+
final List<Node> tokens2 = Parser('escaping', 'app_en.arb', "'hello world { name }'", useEscaping: true).lexIntoTokens();
195+
expect(tokens2, equals(<Node>[Node.string(0, 'hello world { name }')]));
198196

199-
// final List<Node> tokens3 = Parser("'{ escaped string }' { not escaped }").lexIntoTokens();
200-
// expect(tokens3, equals(<Node>[
201-
// Node.string(0, '{ escaped string }'),
202-
// Node.string(20, ' '),
203-
// Node.openBrace(21),
204-
// Node.identifier(23, 'not'),
205-
// Node.identifier(27, 'escaped'),
206-
// Node.closeBrace(35),
207-
// ]));
197+
final List<Node> tokens3 = Parser('escaping', 'app_en.arb', "'{ escaped string }' { not escaped }", useEscaping: true).lexIntoTokens();
198+
expect(tokens3, equals(<Node>[
199+
Node.string(0, '{ escaped string }'),
200+
Node.string(20, ' '),
201+
Node.openBrace(21),
202+
Node.identifier(23, 'not'),
203+
Node.identifier(27, 'escaped'),
204+
Node.closeBrace(35),
205+
]));
208206

209-
// final List<Node> tokens4 = Parser("Flutter''s amazing!").lexIntoTokens();
210-
// expect(tokens4, equals(<Node>[
211-
// Node.string(0, 'Flutter'),
212-
// Node.string(7, "'"),
213-
// Node.string(9, 's amazing!'),
214-
// ]));
215-
// });
207+
final List<Node> tokens4 = Parser('escaping', 'app_en.arb', "Flutter''s amazing!", useEscaping: true).lexIntoTokens();
208+
expect(tokens4, equals(<Node>[
209+
Node.string(0, 'Flutter'),
210+
Node.string(7, "'"),
211+
Node.string(9, 's amazing!'),
212+
]));
213+
214+
final List<Node> tokens5 = Parser('escaping', 'app_en.arb', "'Flutter''s amazing!'", useEscaping: true).lexIntoTokens();
215+
expect(tokens5, equals(<Node>[
216+
Node(ST.string, 0, value: 'Flutter'),
217+
Node(ST.string, 9, value: "'s amazing!"),
218+
]));
219+
});
216220

217221
testWithoutContext('lexer: lexically correct but syntactically incorrect', () {
218222
final List<Node> tokens = Parser(
@@ -235,22 +239,20 @@ void main() {
235239
]));
236240
});
237241

238-
// TODO(thkim1011): Uncomment when implementing escaping.
239-
// See https://github.com/flutter/flutter/issues/113455.
240-
// testWithoutContext('lexer unmatched single quote', () {
241-
// const String message = "here''s an unmatched single quote: '";
242-
// const String expectedError = '''
243-
// ICU Lexing Error: Unmatched single quotes.
244-
// here''s an unmatched single quote: '
245-
// ^''';
246-
// expect(
247-
// () => Parser(message).lexIntoTokens(),
248-
// throwsA(isA<L10nException>().having(
249-
// (L10nException e) => e.message,
250-
// 'message',
251-
// contains(expectedError),
252-
// )));
253-
// });
242+
testWithoutContext('lexer unmatched single quote', () {
243+
const String message = "here''s an unmatched single quote: '";
244+
const String expectedError = '''
245+
ICU Lexing Error: Unmatched single quotes.
246+
[app_en.arb:escaping] here''s an unmatched single quote: '
247+
^''';
248+
expect(
249+
() => Parser('escaping', 'app_en.arb', message, useEscaping: true).lexIntoTokens(),
250+
throwsA(isA<L10nException>().having(
251+
(L10nException e) => e.message,
252+
'message',
253+
contains(expectedError),
254+
)));
255+
});
254256

255257
testWithoutContext('lexer unexpected character', () {
256258
const String message = '{ * }';
@@ -367,17 +369,22 @@ ICU Lexing Error: Unexpected character.
367369
));
368370
});
369371

370-
// TODO(thkim1011): Uncomment when implementing escaping.
371-
// See https://github.com/flutter/flutter/issues/113455.
372-
// testWithoutContext('parser basic 2', () {
373-
// expect(Parser("Flutter''s amazing!").parse(), equals(
374-
// Node(ST.message, 0, children: <Node>[
375-
// Node(ST.string, 0, value: 'Flutter'),
376-
// Node(ST.string, 7, value: "'"),
377-
// Node(ST.string, 9, value: 's amazing!'),
378-
// ])
379-
// ));
380-
// });
372+
testWithoutContext('parser escaping', () {
373+
expect(Parser('escaping', 'app_en.arb', "Flutter''s amazing!", useEscaping: true).parse(), equals(
374+
Node(ST.message, 0, children: <Node>[
375+
Node(ST.string, 0, value: 'Flutter'),
376+
Node(ST.string, 7, value: "'"),
377+
Node(ST.string, 9, value: 's amazing!'),
378+
])
379+
));
380+
381+
expect(Parser('escaping', 'app_en.arb', "'Flutter''s amazing!'", useEscaping: true).parse(), equals(
382+
Node(ST.message, 0, children: <Node> [
383+
Node(ST.string, 0, value: 'Flutter'),
384+
Node(ST.string, 9, value: "'s amazing!"),
385+
])
386+
));
387+
});
381388

382389
testWithoutContext('parser recursive', () {
383390
expect(Parser(

0 commit comments

Comments
 (0)