Skip to content

Commit

Permalink
[gen_l10n] Improvements to gen_l10n (#116202)
Browse files Browse the repository at this point in the history 
* 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 :(
  • Loading branch information
@thkim1011
thkim1011 committed Nov 30, 2022
1 parent 43d5cbb commit 7802c7a
Show file tree
Hide file tree
Showing 8 changed files with 431 additions and 581 deletions.
6 changes: 6 additions & 0 deletions packages/flutter_tools/lib/src/commands/generate_localizations.dart
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand All @@ -281,6 +286,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
usesNullableGetter: usesNullableGetter,
useEscaping: useEscaping,
logger: _logger,
suppressWarnings: suppressWarnings,
)
..loadResources()
..writeOutputFiles())
Expand Down
193 changes: 68 additions & 125 deletions packages/flutter_tools/lib/src/localizations/gen_l10n.dart
View file

Large diffs are not rendered by default.

36 changes: 13 additions & 23 deletions packages/flutter_tools/lib/src/localizations/gen_l10n_templates.dart
View file
Expand Up @@ -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)';
Expand Down
197 changes: 89 additions & 108 deletions packages/flutter_tools/lib/src/localizations/gen_l10n_types.dart
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -138,17 +139,31 @@ class L10nParserException extends L10nException {
this.messageString,
this.charNumber
): super('''
$error
[$fileName:$messageId] $messageString
${List<String>.filled(4 + fileName.length + messageId.length + charNumber, ' ').join()}^''');
[$fileName:$messageId] $error
$messageString
${List<String>.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.
Expand Down Expand Up @@ -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),
Expand All @@ -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<Node> traversalStack = <Node>[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 (!<String>['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;
Expand All @@ -402,6 +372,7 @@ class Message {
final Map<LocaleInfo, Node?> parsedMessages;
final Map<String, Placeholder> placeholders;
final bool useEscaping;
final Logger? logger;

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

Expand Down Expand Up @@ -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<LocaleInfo, String> filenames) {
// We keep the undeclared placeholders separate so that we can sort them alphabetically afterwards.
final Map<String, Placeholder> undeclaredPlaceholders = <String, Placeholder>{};
// 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<Node> traversalStack = <Node>[parsedMessages[locale]!];
while (traversalStack.isNotEmpty) {
final Node node = traversalStack.removeLast();
if (<ST>[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, <String, Object?>{});
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<String, Placeholder> p1, MapEntry<String, Placeholder> 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 (!<String>['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.
Expand Down Expand Up @@ -834,50 +862,3 @@ final Set<String> _iso639Languages = <String>{
'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<Placeholder> 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(', ');
}
}
35 changes: 20 additions & 15 deletions packages/flutter_tools/lib/src/localizations/localizations_utils.dart
View file
Expand Up @@ -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<HelperMethod> helpers) {
if (helpers.isEmpty) {
/// If isSingleStringVar is passed, then we want to convert "'$expr'" to simply "expr".
String generateReturnExpr(List<String> 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 string, HelperMethod helper) {
if (helper.string != null) {
return generateString(helper.string!) + string;
final String string = expressions.reversed.fold<String>('', (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'";
Expand All @@ -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.
Expand Down Expand Up @@ -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].
Expand Down Expand Up @@ -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,
);
}

Expand Down

0 comments on commit 7802c7a

Please sign in to comment.