[go: nahoru, domu]
Skip to content

Commit

Permalink
Add Escaping Option for ICU MessageFormat Syntax (#116137)
Browse files Browse the repository at this point in the history 
* init

* make more descriptive

* fix lint
  • Loading branch information
@thkim1011
thkim1011 committed Nov 29, 2022
1 parent d6995aa commit 39a73ca
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 69 deletions.
11 changes: 11 additions & 0 deletions packages/flutter_tools/lib/src/commands/generate_localizations.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,15 @@ class GenerateLocalizationsCommand extends FlutterCommand {
'format',
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;
Expand Down Expand Up @@ -248,6 +257,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
final String? projectPathString = stringArgDeprecated('project-dir');
final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes');
final bool usesNullableGetter = boolArgDeprecated('nullable-getter');
final bool useEscaping = boolArgDeprecated('use-escaping');

precacheLanguageAndRegionTags();

Expand All @@ -269,6 +279,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
areResourceAttributesRequired: areResourceAttributesRequired,
untranslatedMessagesFile: untranslatedMessagesFile,
usesNullableGetter: usesNullableGetter,
useEscaping: useEscaping,
logger: _logger,
)
..loadResources()
Expand Down
9 changes: 8 additions & 1 deletion packages/flutter_tools/lib/src/localizations/gen_l10n.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ LocalizationsGenerator generateLocalizations({
areResourceAttributesRequired: options.areResourceAttributesRequired,
untranslatedMessagesFile: options.untranslatedMessagesFile?.toFilePath(),
usesNullableGetter: options.usesNullableGetter,
useEscaping: options.useEscaping,
logger: logger,
)
..loadResources()
Expand Down Expand Up @@ -453,6 +454,7 @@ class LocalizationsGenerator {
bool areResourceAttributesRequired = false,
String? untranslatedMessagesFile,
bool usesNullableGetter = true,
bool useEscaping = false,
required Logger logger,
}) {
final Directory? projectDirectory = projectDirFromPath(fileSystem, projectPathString);
Expand All @@ -474,6 +476,7 @@ class LocalizationsGenerator {
untranslatedMessagesFile: _untranslatedMessagesFileFromPath(fileSystem, untranslatedMessagesFile),
inputsAndOutputsListFile: _inputsAndOutputsListFileFromPath(fileSystem, inputsAndOutputsListPath),
areResourceAttributesRequired: areResourceAttributesRequired,
useEscaping: useEscaping,
logger: logger,
);
}
Expand All @@ -497,6 +500,7 @@ class LocalizationsGenerator {
this.untranslatedMessagesFile,
this.usesNullableGetter = true,
required this.logger,
this.useEscaping = false,
});

final FileSystem _fs;
Expand Down Expand Up @@ -566,6 +570,9 @@ class LocalizationsGenerator {
// all of the messages.
bool requiresIntlImport = false;

// Whether we want to use escaping for ICU messages.
bool useEscaping = false;

/// The list of all arb path strings in [inputDirectory].
List<String> get arbPathStrings {
return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList();
Expand Down Expand Up @@ -845,7 +852,7 @@ class LocalizationsGenerator {
// files in inputDirectory. Also initialized: supportedLocales.
void loadResources() {
_allMessages = _templateBundle.resourceIds.map((String id) => Message(
_templateBundle, _allBundles, id, areResourceAttributesRequired,
_templateBundle, _allBundles, id, areResourceAttributesRequired, useEscaping: useEscaping,
));
for (final String resourceId in _templateBundle.resourceIds) {
if (!_isValidGetterAndMethodName(resourceId)) {
Expand Down
6 changes: 4 additions & 2 deletions packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ class Message {
AppResourceBundle templateBundle,
AppResourceBundleCollection allBundles,
this.resourceId,
bool isResourceAttributeRequired
bool isResourceAttributeRequired,
{ this.useEscaping = false }
) : assert(templateBundle != null),
assert(allBundles != null),
assert(resourceId != null && resourceId.isNotEmpty),
Expand All @@ -334,7 +335,7 @@ 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).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.
for (final LocaleInfo locale in parsedMessages.keys) {
Expand Down Expand Up @@ -400,6 +401,7 @@ class Message {
late final Map<LocaleInfo, String?> messages;
final Map<LocaleInfo, Node?> parsedMessages;
final Map<String, Placeholder> placeholders;
final bool useEscaping;

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

Expand Down
7 changes: 7 additions & 0 deletions packages/flutter_tools/lib/src/localizations/localizations_utils.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ class LocalizationOptions {
this.areResourceAttributesRequired = false,
this.usesNullableGetter = true,
this.format = false,
this.useEscaping = false,
}) : assert(useSyntheticPackage != null);

/// The `--arb-dir` argument.
Expand Down Expand Up @@ -410,6 +411,11 @@ class LocalizationOptions {
///
/// Whether or not to format the generated files.
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].
Expand Down Expand Up @@ -445,6 +451,7 @@ LocalizationOptions parseLocalizationsOptions({
areResourceAttributesRequired: _tryReadBool(yamlNode, 'required-resource-attributes', logger) ?? false,
usesNullableGetter: _tryReadBool(yamlNode, 'nullable-getter', logger) ?? true,
format: _tryReadBool(yamlNode, 'format', logger) ?? true,
useEscaping: _tryReadBool(yamlNode, 'use-escaping', logger) ?? false,
);
}

Expand Down
62 changes: 46 additions & 16 deletions packages/flutter_tools/lib/src/localizations/message_parser.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 whitespace = RegExp(r'\s+');
Expand All @@ -174,11 +177,17 @@ Map<ST, RegExp> matchers = <ST, RegExp>{
};

class Parser {
Parser(this.messageId, this.filename, this.messageString);
Parser(
this.messageId,
this.filename,
this.messageString,
{ this.useEscaping = false }
);

final String messageId;
final String messageString;
final String filename;
final bool useEscaping;

static String indentForError(int position) {
return '${List<String>.filled(position, ' ').join()}^';
Expand All @@ -201,20 +210,41 @@ class Parser {
while (startIndex < messageString.length) {
Match? match;
if (isString) {
// TODO(thkim1011): Uncomment this when we add escaping as an option.
// See https://github.com/flutter/flutter/issues/113455.
// match = escapedString.matchAsPrefix(message, startIndex);
// if (match != null) {
// final String string = match.group(0)!;
// tokens.add(Node.string(startIndex, string == "''" ? "'" : 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;
if (useEscaping) {
// This case is slightly involved. Essentially, wrapping any syntax in
// single quotes escapes the syntax except when there are consecutive pair of single
// quotes. For example, "Hello! 'Flutter''s amazing'. { unescapedPlaceholder }"
// converts the '' in "Flutter's" to a single quote for convenience, since technically,
// we should interpret this as two strings 'Flutter' and 's amazing'. To get around this,
// we also check if the previous character is a ', and if so, add a single quote at the beginning
// of the token.
match = escapedString.matchAsPrefix(messageString, startIndex);
if (match != null) {
final String string = match.group(0)!;
if (string == "''") {
tokens.add(Node.string(startIndex, "'"));
} 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);
if (match != null) {
Expand Down
107 changes: 57 additions & 50 deletions packages/flutter_tools/test/general.shard/message_parser_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,32 +187,36 @@ void main() {
]));
});

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

// final List<Node> tokens2 = Parser("'hello world { name }'").lexIntoTokens();
// expect(tokens2, equals(<Node>[Node.string(0, 'hello world { name }')]));
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 }')]));

// final List<Node> tokens3 = Parser("'{ escaped string }' { not escaped }").lexIntoTokens();
// expect(tokens3, equals(<Node>[
// Node.string(0, '{ escaped string }'),
// Node.string(20, ' '),
// Node.openBrace(21),
// Node.identifier(23, 'not'),
// Node.identifier(27, 'escaped'),
// Node.closeBrace(35),
// ]));
final List<Node> tokens3 = Parser('escaping', 'app_en.arb', "'{ escaped string }' { not escaped }", useEscaping: true).lexIntoTokens();
expect(tokens3, equals(<Node>[
Node.string(0, '{ escaped string }'),
Node.string(20, ' '),
Node.openBrace(21),
Node.identifier(23, 'not'),
Node.identifier(27, 'escaped'),
Node.closeBrace(35),
]));

// final List<Node> tokens4 = Parser("Flutter''s amazing!").lexIntoTokens();
// expect(tokens4, equals(<Node>[
// Node.string(0, 'Flutter'),
// Node.string(7, "'"),
// Node.string(9, 's amazing!'),
// ]));
// });
final List<Node> tokens4 = Parser('escaping', 'app_en.arb', "Flutter''s amazing!", useEscaping: true).lexIntoTokens();
expect(tokens4, equals(<Node>[
Node.string(0, 'Flutter'),
Node.string(7, "'"),
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', () {
final List<Node> tokens = Parser(
Expand All @@ -235,22 +239,20 @@ void main() {
]));
});

// TODO(thkim1011): Uncomment when implementing escaping.
// See https://github.com/flutter/flutter/issues/113455.
// testWithoutContext('lexer unmatched single quote', () {
// const String message = "here''s an unmatched single quote: '";
// const String expectedError = '''
// ICU Lexing Error: Unmatched single quotes.
// here''s an unmatched single quote: '
// ^''';
// expect(
// () => Parser(message).lexIntoTokens(),
// throwsA(isA<L10nException>().having(
// (L10nException e) => e.message,
// 'message',
// contains(expectedError),
// )));
// });
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: '
^''';
expect(
() => Parser('escaping', 'app_en.arb', message, useEscaping: true).lexIntoTokens(),
throwsA(isA<L10nException>().having(
(L10nException e) => e.message,
'message',
contains(expectedError),
)));
});

testWithoutContext('lexer unexpected character', () {
const String message = '{ * }';
Expand Down Expand Up @@ -367,17 +369,22 @@ ICU Lexing Error: Unexpected character.
));
});

// TODO(thkim1011): Uncomment when implementing escaping.
// See https://github.com/flutter/flutter/issues/113455.
// testWithoutContext('parser basic 2', () {
// expect(Parser("Flutter''s amazing!").parse(), equals(
// Node(ST.message, 0, children: <Node>[
// Node(ST.string, 0, value: 'Flutter'),
// Node(ST.string, 7, value: "'"),
// Node(ST.string, 9, value: 's amazing!'),
// ])
// ));
// });
testWithoutContext('parser escaping', () {
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, 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', () {
expect(Parser(
Expand Down

0 comments on commit 39a73ca

Please sign in to comment.