Unverified Commit c6b93b2d authored by Tae Hyung Kim's avatar Tae Hyung Kim Committed by GitHub

Relax syntax for gen-l10n (#130736)

To preserve backward compatibility with the old parser which would
ignore syntax errors, this PR introduces a way to treat the special
characters `{` and `}` in the following way:
1. If we encounter a `{` which searching for a string token and this `{`
is not followed by a valid placeholder, then we treat the `{` as a
string and continue lexing for strings.
2. If we encounter a `}` while not within some expression (i.e.
placeholders, arguments, plurals, or selects), then we treat the `}` as
a string and continue lexing for strings.

This makes it so that
```
"helloWorld": "{ } { placeholder }",
"@@helloWorld": {
  "placeholders": {
    "placeholder" {}
  }
}
```
treats the `{ }` as a string while `{ placeholder } ` is treated as a
placeholder.

Fixes https://github.com/flutter/flutter/issues/122404.
parent bb798e2a
...@@ -200,6 +200,13 @@ class GenerateLocalizationsCommand extends FlutterCommand { ...@@ -200,6 +200,13 @@ class GenerateLocalizationsCommand extends FlutterCommand {
'suppress-warnings', 'suppress-warnings',
help: 'When specified, all warnings will be suppressed.\n' help: 'When specified, all warnings will be suppressed.\n'
); );
argParser.addFlag(
'relax-syntax',
help: 'When specified, the syntax will be relaxed so that the special character '
'"{" is treated as a string if it is not followed by a valid placeholder '
'and "}" is treated as a string if it does not close any previous "{" '
'that is treated as a special character.',
);
} }
final FileSystem _fileSystem; final FileSystem _fileSystem;
......
...@@ -72,6 +72,7 @@ Future<LocalizationsGenerator> generateLocalizations({ ...@@ -72,6 +72,7 @@ Future<LocalizationsGenerator> generateLocalizations({
useEscaping: options.useEscaping, useEscaping: options.useEscaping,
logger: logger, logger: logger,
suppressWarnings: options.suppressWarnings, suppressWarnings: options.suppressWarnings,
useRelaxedSyntax: options.relaxSyntax,
) )
..loadResources() ..loadResources()
..writeOutputFiles(isFromYaml: true, useCRLF: useCRLF); ..writeOutputFiles(isFromYaml: true, useCRLF: useCRLF);
...@@ -494,6 +495,7 @@ class LocalizationsGenerator { ...@@ -494,6 +495,7 @@ class LocalizationsGenerator {
bool useEscaping = false, bool useEscaping = false,
required Logger logger, required Logger logger,
bool suppressWarnings = false, bool suppressWarnings = false,
bool useRelaxedSyntax = false,
}) { }) {
final Directory? projectDirectory = projectDirFromPath(fileSystem, projectPathString); final Directory? projectDirectory = projectDirFromPath(fileSystem, projectPathString);
final Directory inputDirectory = inputDirectoryFromPath(fileSystem, inputPathString, projectDirectory); final Directory inputDirectory = inputDirectoryFromPath(fileSystem, inputPathString, projectDirectory);
...@@ -517,6 +519,7 @@ class LocalizationsGenerator { ...@@ -517,6 +519,7 @@ class LocalizationsGenerator {
useEscaping: useEscaping, useEscaping: useEscaping,
logger: logger, logger: logger,
suppressWarnings: suppressWarnings, suppressWarnings: suppressWarnings,
useRelaxedSyntax: useRelaxedSyntax,
); );
} }
...@@ -541,6 +544,7 @@ class LocalizationsGenerator { ...@@ -541,6 +544,7 @@ class LocalizationsGenerator {
required this.logger, required this.logger,
this.useEscaping = false, this.useEscaping = false,
this.suppressWarnings = false, this.suppressWarnings = false,
this.useRelaxedSyntax = false,
}); });
final FileSystem _fs; final FileSystem _fs;
...@@ -617,6 +621,9 @@ class LocalizationsGenerator { ...@@ -617,6 +621,9 @@ class LocalizationsGenerator {
/// from calling [_generateMethod]. /// from calling [_generateMethod].
bool hadErrors = false; bool hadErrors = false;
/// Whether to use relaxed syntax.
bool useRelaxedSyntax = 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();
...@@ -908,7 +915,13 @@ class LocalizationsGenerator { ...@@ -908,7 +915,13 @@ class LocalizationsGenerator {
} }
// The call to .toList() is absolutely necessary. Otherwise, it is an iterator and will call Message's constructor again. // 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( _allMessages = _templateBundle.resourceIds.map((String id) => Message(
_templateBundle, _allBundles, id, areResourceAttributesRequired, useEscaping: useEscaping, logger: logger, _templateBundle,
_allBundles,
id,
areResourceAttributesRequired,
useEscaping: useEscaping,
logger: logger,
useRelaxedSyntax: useRelaxedSyntax,
)).toList(); )).toList();
hadErrors = _allMessages.any((Message message) => message.hadErrors); hadErrors = _allMessages.any((Message message) => message.hadErrors);
if (inputsAndOutputsListFile != null) { if (inputsAndOutputsListFile != null) {
......
...@@ -336,6 +336,7 @@ class Message { ...@@ -336,6 +336,7 @@ class Message {
this.resourceId, this.resourceId,
bool isResourceAttributeRequired, bool isResourceAttributeRequired,
{ {
this.useRelaxedSyntax = false,
this.useEscaping = false, this.useEscaping = false,
this.logger, this.logger,
} }
...@@ -352,13 +353,18 @@ class Message { ...@@ -352,13 +353,18 @@ 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;
List<String>? validPlaceholders;
if (useRelaxedSyntax) {
validPlaceholders = placeholders.entries.map((MapEntry<String, Placeholder> e) => e.key).toList();
}
try { try {
parsedMessages[bundle.locale] = translation == null ? null : Parser( parsedMessages[bundle.locale] = translation == null ? null : Parser(
resourceId, resourceId,
bundle.file.basename, bundle.file.basename,
translation, translation,
useEscaping: useEscaping, useEscaping: useEscaping,
logger: logger placeholders: validPlaceholders,
logger: logger,
).parse(); ).parse();
} on L10nParserException catch (error) { } on L10nParserException catch (error) {
logger?.printError(error.toString()); logger?.printError(error.toString());
...@@ -378,6 +384,7 @@ class Message { ...@@ -378,6 +384,7 @@ class Message {
final Map<LocaleInfo, Node?> parsedMessages; final Map<LocaleInfo, Node?> parsedMessages;
final Map<String, Placeholder> placeholders; final Map<String, Placeholder> placeholders;
final bool useEscaping; final bool useEscaping;
final bool useRelaxedSyntax;
final Logger? logger; final Logger? logger;
bool hadErrors = false; bool hadErrors = false;
......
...@@ -354,6 +354,7 @@ class LocalizationOptions { ...@@ -354,6 +354,7 @@ class LocalizationOptions {
bool? format, bool? format,
bool? useEscaping, bool? useEscaping,
bool? suppressWarnings, bool? suppressWarnings,
bool? relaxSyntax,
}) : templateArbFile = templateArbFile ?? 'app_en.arb', }) : templateArbFile = templateArbFile ?? 'app_en.arb',
outputLocalizationFile = outputLocalizationFile ?? 'app_localizations.dart', outputLocalizationFile = outputLocalizationFile ?? 'app_localizations.dart',
outputClass = outputClass ?? 'AppLocalizations', outputClass = outputClass ?? 'AppLocalizations',
...@@ -363,7 +364,8 @@ class LocalizationOptions { ...@@ -363,7 +364,8 @@ class LocalizationOptions {
nullableGetter = nullableGetter ?? true, nullableGetter = nullableGetter ?? true,
format = format ?? false, format = format ?? false,
useEscaping = useEscaping ?? false, useEscaping = useEscaping ?? false,
suppressWarnings = suppressWarnings ?? false; suppressWarnings = suppressWarnings ?? false,
relaxSyntax = relaxSyntax ?? false;
/// The `--arb-dir` argument. /// The `--arb-dir` argument.
/// ///
...@@ -455,6 +457,16 @@ class LocalizationOptions { ...@@ -455,6 +457,16 @@ class LocalizationOptions {
/// ///
/// Whether or not to suppress warnings. /// Whether or not to suppress warnings.
final bool suppressWarnings; final bool suppressWarnings;
/// The `relax-syntax` argument.
///
/// Whether or not to relax the syntax. When specified, the syntax will be
/// relaxed so that the special character "{" is treated as a string if it is
/// not followed by a valid placeholder and "}" is treated as a string if it
/// does not close any previous "{" that is treated as a special character.
/// This was added in for backward compatibility and is not recommended
/// as it may mask errors.
final bool relaxSyntax;
} }
/// Parse the localizations configuration options from [file]. /// Parse the localizations configuration options from [file].
...@@ -498,6 +510,7 @@ LocalizationOptions parseLocalizationsOptionsFromYAML({ ...@@ -498,6 +510,7 @@ LocalizationOptions parseLocalizationsOptionsFromYAML({
format: _tryReadBool(yamlNode, 'format', logger), format: _tryReadBool(yamlNode, 'format', logger),
useEscaping: _tryReadBool(yamlNode, 'use-escaping', logger), useEscaping: _tryReadBool(yamlNode, 'use-escaping', logger),
suppressWarnings: _tryReadBool(yamlNode, 'suppress-warnings', logger), suppressWarnings: _tryReadBool(yamlNode, 'suppress-warnings', logger),
relaxSyntax: _tryReadBool(yamlNode, 'relax-syntax', logger),
); );
} }
......
...@@ -198,7 +198,8 @@ class Parser { ...@@ -198,7 +198,8 @@ class Parser {
this.messageString, this.messageString,
{ {
this.useEscaping = false, this.useEscaping = false,
this.logger this.logger,
this.placeholders,
} }
); );
...@@ -207,6 +208,7 @@ class Parser { ...@@ -207,6 +208,7 @@ class Parser {
final String filename; final String filename;
final bool useEscaping; final bool useEscaping;
final Logger? logger; final Logger? logger;
final List<String>? placeholders;
static String indentForError(int position) { static String indentForError(int position) {
return '${List<String>.filled(position, ' ').join()}^'; return '${List<String>.filled(position, ' ').join()}^';
...@@ -216,12 +218,16 @@ class Parser { ...@@ -216,12 +218,16 @@ class Parser {
// every instance of "{" and "}" toggles the isString boolean and every // every instance of "{" and "}" toggles the isString boolean and every
// instance of "'" toggles the isEscaped boolean (and treats a double // instance of "'" toggles the isEscaped boolean (and treats a double
// single quote "''" as a single quote "'"). When !isString and !isEscaped // single quote "''" as a single quote "'"). When !isString and !isEscaped
// delimit tokens by whitespace and special characters. // delimit tokens by whitespace and special characters. When placeholders
// is passed, relax the syntax so that "{" and "}" can be used as strings in
// certain cases.
List<Node> lexIntoTokens() { List<Node> lexIntoTokens() {
final bool useRelaxedLexer = placeholders != null;
final List<Node> tokens = <Node>[]; final List<Node> tokens = <Node>[];
bool isString = true; bool isString = true;
// Index specifying where to match from // Index specifying where to match from
int startIndex = 0; int startIndex = 0;
int depth = 0;
// At every iteration, we should be able to match a new token until we // At every iteration, we should be able to match a new token until we
// reach the end of the string. If for some reason we don't match a // reach the end of the string. If for some reason we don't match a
...@@ -267,9 +273,28 @@ class Parser { ...@@ -267,9 +273,28 @@ class Parser {
} }
match = brace.matchAsPrefix(messageString, startIndex); match = brace.matchAsPrefix(messageString, startIndex);
if (match != null) { if (match != null) {
final String matchedBrace = match.group(0)!;
if (useRelaxedLexer) {
final Match? whitespaceMatch = whitespace.matchAsPrefix(messageString, match.end);
final int endOfWhitespace = whitespaceMatch?.group(0) == null ? match.end : whitespaceMatch!.end;
final Match? identifierMatch = alphanumeric.matchAsPrefix(messageString, endOfWhitespace);
// If we match a "}" and the depth is 0, treat it as a string.
// If we match a "{" and the next token is not a valid placeholder, treat it as a string.
if (matchedBrace == '}' && depth == 0) {
tokens.add(Node.string(startIndex, matchedBrace));
startIndex = match.end;
continue;
}
if (matchedBrace == '{' && (identifierMatch == null || !placeholders!.contains(identifierMatch.group(0)))) {
tokens.add(Node.string(startIndex, matchedBrace));
startIndex = match.end;
continue;
}
}
tokens.add(Node.brace(startIndex, match.group(0)!)); tokens.add(Node.brace(startIndex, match.group(0)!));
isString = false; isString = false;
startIndex = match.end; startIndex = match.end;
depth += 1;
continue; continue;
} }
// Theoretically, we only reach this point because of unmatched single quotes because // Theoretically, we only reach this point because of unmatched single quotes because
...@@ -299,9 +324,15 @@ class Parser { ...@@ -299,9 +324,15 @@ class Parser {
if (match == null) { if (match == null) {
match = brace.matchAsPrefix(messageString, startIndex); match = brace.matchAsPrefix(messageString, startIndex);
if (match != null) { if (match != null) {
tokens.add(Node.brace(startIndex, match.group(0)!)); final String matchedBrace = match.group(0)!;
tokens.add(Node.brace(startIndex, matchedBrace));
isString = true; isString = true;
startIndex = match.end; startIndex = match.end;
if (matchedBrace == '{') {
depth += 1;
} else {
depth -= 1;
}
continue; continue;
} }
// This should only happen when there are special characters we are unable to match. // This should only happen when there are special characters we are unable to match.
......
...@@ -95,6 +95,7 @@ void main() { ...@@ -95,6 +95,7 @@ void main() {
bool useEscaping = false, bool useEscaping = false,
bool areResourceAttributeRequired = false, bool areResourceAttributeRequired = false,
bool suppressWarnings = false, bool suppressWarnings = false,
bool relaxSyntax = false,
void Function(Directory)? setup, void Function(Directory)? setup,
} }
) { ) {
...@@ -126,6 +127,7 @@ void main() { ...@@ -126,6 +127,7 @@ void main() {
useEscaping: useEscaping, useEscaping: useEscaping,
areResourceAttributesRequired: areResourceAttributeRequired, areResourceAttributesRequired: areResourceAttributeRequired,
suppressWarnings: suppressWarnings, suppressWarnings: suppressWarnings,
useRelaxedSyntax: relaxSyntax,
) )
..loadResources() ..loadResources()
..writeOutputFiles(isFromYaml: isFromYaml); ..writeOutputFiles(isFromYaml: isFromYaml);
...@@ -1475,6 +1477,22 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e ...@@ -1475,6 +1477,22 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e
expect(getGeneratedFileContent(locale: 'en'), contains('String helloWorld(Object name) {')); expect(getGeneratedFileContent(locale: 'en'), contains('String helloWorld(Object name) {'));
expect(getGeneratedFileContent(locale: 'es'), contains('String helloWorld(Object name) {')); expect(getGeneratedFileContent(locale: 'es'), contains('String helloWorld(Object name) {'));
}); });
testWithoutContext('braces are ignored as special characters if placeholder does not exist', () {
setupLocalizations(<String, String>{
'en': '''
{
"helloWorld": "Hello {name}",
"@@helloWorld": {
"placeholders": {
"names": {}
}
}
}'''
}, relaxSyntax: true);
final String content = getGeneratedFileContent(locale: 'en');
expect(content, contains("String get helloWorld => 'Hello {name}'"));
});
}); });
group('DateTime tests', () { group('DateTime tests', () {
......
...@@ -293,6 +293,54 @@ void main() { ...@@ -293,6 +293,54 @@ void main() {
))); )));
}); });
testWithoutContext('relaxed lexer', () {
final List<Node> tokens1 = Parser(
'string',
'app_en.arb',
'{ }',
placeholders: <String>[],
).lexIntoTokens();
expect(tokens1, equals(<Node>[
Node(ST.string, 0, value: '{'),
Node(ST.string, 1, value: ' '),
Node(ST.string, 2, value: '}')
]));
final List<Node> tokens2 = Parser(
'string',
'app_en.arb',
'{ notAPlaceholder }',
placeholders: <String>['isAPlaceholder'],
).lexIntoTokens();
expect(tokens2, equals(<Node>[
Node(ST.string, 0, value: '{'),
Node(ST.string, 1, value: ' notAPlaceholder '),
Node(ST.string, 18, value: '}')
]));
final List<Node> tokens3 = Parser(
'string',
'app_en.arb',
'{ isAPlaceholder }',
placeholders: <String>['isAPlaceholder'],
).lexIntoTokens();
expect(tokens3, equals(<Node>[
Node(ST.openBrace, 0, value: '{'),
Node(ST.identifier, 2, value: 'isAPlaceholder'),
Node(ST.closeBrace, 17, value: '}')
]));
});
testWithoutContext('relaxed lexer complex', () {
const String message = '{ notPlaceholder } {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 List<Node> tokens = Parser(
'string',
'app_en.arb',
message,
placeholders: <String>['count'],
).lexIntoTokens();
expect(tokens[0].type, equals(ST.string));
});
testWithoutContext('parser basic', () { testWithoutContext('parser basic', () {
expect(Parser('helloWorld', 'app_en.arb', 'Hello {name}').parse(), equals( expect(Parser('helloWorld', 'app_en.arb', 'Hello {name}').parse(), equals(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment