Skip to content

Commit 7802c7a

Browse files
authored
[gen_l10n] Improvements to gen_l10n (#116202)
* 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 :(
1 parent 43d5cbb commit 7802c7a

File tree

8 files changed

+431
-581
lines changed

8 files changed

+431
-581
lines changed

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

+6
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,10 @@ class GenerateLocalizationsCommand extends FlutterCommand {
201201
'contained within pairs of single quotes as normal strings and treat all '
202202
'consecutive pairs of single quotes as a single quote character.',
203203
);
204+
argParser.addFlag(
205+
'suppress-warnings',
206+
help: 'When specified, all warnings will be suppressed.\n'
207+
);
204208
}
205209

206210
final FileSystem _fileSystem;
@@ -258,6 +262,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
258262
final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes');
259263
final bool usesNullableGetter = boolArgDeprecated('nullable-getter');
260264
final bool useEscaping = boolArgDeprecated('use-escaping');
265+
final bool suppressWarnings = boolArgDeprecated('suppress-warnings');
261266

262267
precacheLanguageAndRegionTags();
263268

@@ -281,6 +286,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
281286
usesNullableGetter: usesNullableGetter,
282287
useEscaping: useEscaping,
283288
logger: _logger,
289+
suppressWarnings: suppressWarnings,
284290
)
285291
..loadResources()
286292
..writeOutputFiles())

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

+68-125
Large diffs are not rendered by default.

packages/flutter_tools/lib/src/localizations/gen_l10n_templates.dart

+13-23
Original file line numberDiff line numberDiff line change
@@ -139,33 +139,23 @@ const String methodTemplate = '''
139139
String @(name)(@(parameters)) {
140140
@(dateFormatting)
141141
@(numberFormatting)
142-
@(helperMethods)
143-
return @(message);
142+
@(tempVars) return @(message);
144143
}''';
145144

146-
const String messageHelperTemplate = '''
147-
String @(name)(@(parameters)) {
148-
return @(message);
149-
}''';
150-
151-
const String pluralHelperTemplate = '''
152-
String @(name)(@(parameters)) {
153-
return intl.Intl.pluralLogic(
154-
@(count),
155-
locale: localeName,
145+
const String pluralVariableTemplate = '''
146+
String @(varName) = intl.Intl.pluralLogic(
147+
@(count),
148+
locale: localeName,
156149
@(pluralLogicArgs)
157-
);
158-
}''';
159-
160-
const String selectHelperTemplate = '''
161-
String @(name)(@(parameters)) {
162-
return intl.Intl.selectLogic(
163-
@(choice),
164-
{
150+
);''';
151+
152+
const String selectVariableTemplate = '''
153+
String @(varName) = intl.Intl.selectLogic(
154+
@(choice),
155+
{
165156
@(selectCases)
166-
},
167-
);
168-
}''';
157+
},
158+
);''';
169159

170160
const String classFileTemplate = '''
171161
@(header)@(requiresIntlImport)import '@(fileName)';

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

+89-108
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import 'package:intl/locale.dart';
66

77
import '../base/file_system.dart';
8+
import '../base/logger.dart';
89
import '../convert.dart';
910
import 'localizations_utils.dart';
1011
import 'message_parser.dart';
@@ -138,17 +139,31 @@ class L10nParserException extends L10nException {
138139
this.messageString,
139140
this.charNumber
140141
): super('''
141-
$error
142-
[$fileName:$messageId] $messageString
143-
${List<String>.filled(4 + fileName.length + messageId.length + charNumber, ' ').join()}^''');
142+
[$fileName:$messageId] $error
143+
$messageString
144+
${List<String>.filled(charNumber, ' ').join()}^''');
144145

145146
final String error;
146147
final String fileName;
147148
final String messageId;
148149
final String messageString;
150+
// Position of character within the "messageString" where the error is.
149151
final int charNumber;
150152
}
151153

154+
class L10nMissingPlaceholderException extends L10nParserException {
155+
L10nMissingPlaceholderException(
156+
super.error,
157+
super.fileName,
158+
super.messageId,
159+
super.messageString,
160+
super.charNumber,
161+
this.placeholderName,
162+
);
163+
164+
final String placeholderName;
165+
}
166+
152167
// One optional named parameter to be used by a NumberFormat.
153168
//
154169
// Some of the NumberFormat factory constructors have optional named parameters.
@@ -319,7 +334,10 @@ class Message {
319334
AppResourceBundleCollection allBundles,
320335
this.resourceId,
321336
bool isResourceAttributeRequired,
322-
{ this.useEscaping = false }
337+
{
338+
this.useEscaping = false,
339+
this.logger,
340+
}
323341
) : assert(templateBundle != null),
324342
assert(allBundles != null),
325343
assert(resourceId != null && resourceId.isNotEmpty),
@@ -335,64 +353,16 @@ class Message {
335353
filenames[bundle.locale] = bundle.file.basename;
336354
final String? translation = bundle.translationFor(resourceId);
337355
messages[bundle.locale] = translation;
338-
parsedMessages[bundle.locale] = translation == null ? null : Parser(resourceId, bundle.file.basename, translation, useEscaping: useEscaping).parse();
339-
}
340-
// Using parsed translations, attempt to infer types of placeholders used by plurals and selects.
341-
for (final LocaleInfo locale in parsedMessages.keys) {
342-
if (parsedMessages[locale] == null) {
343-
continue;
344-
}
345-
final List<Node> traversalStack = <Node>[parsedMessages[locale]!];
346-
while (traversalStack.isNotEmpty) {
347-
final Node node = traversalStack.removeLast();
348-
if (node.type == ST.pluralExpr) {
349-
final Placeholder? placeholder = placeholders[node.children[1].value!];
350-
if (placeholder == null) {
351-
throw L10nParserException(
352-
'Make sure that the specified plural placeholder is defined in your arb file.',
353-
filenames[locale]!,
354-
resourceId,
355-
messages[locale]!,
356-
node.children[1].positionInMessage
357-
);
358-
}
359-
placeholders[node.children[1].value!]!.isPlural = true;
360-
}
361-
if (node.type == ST.selectExpr) {
362-
final Placeholder? placeholder = placeholders[node.children[1].value!];
363-
if (placeholder == null) {
364-
throw L10nParserException(
365-
'Make sure that the specified select placeholder is defined in your arb file.',
366-
filenames[locale]!,
367-
resourceId,
368-
messages[locale]!,
369-
node.children[1].positionInMessage
370-
);
371-
}
372-
placeholders[node.children[1].value!]!.isSelect = true;
373-
}
374-
traversalStack.addAll(node.children);
375-
}
376-
}
377-
for (final Placeholder placeholder in placeholders.values) {
378-
if (placeholder.isPlural && placeholder.isSelect) {
379-
throw L10nException('Placeholder is used as both a plural and select in certain languages.');
380-
} else if (placeholder.isPlural) {
381-
if (placeholder.type == null) {
382-
placeholder.type = 'num';
383-
}
384-
else if (!<String>['num', 'int'].contains(placeholder.type)) {
385-
throw L10nException("Placeholders used in plurals must be of type 'num' or 'int'");
386-
}
387-
} else if (placeholder.isSelect) {
388-
if (placeholder.type == null) {
389-
placeholder.type = 'String';
390-
} else if (placeholder.type != 'String') {
391-
throw L10nException("Placeholders used in selects must be of type 'String'");
392-
}
393-
}
394-
placeholder.type ??= 'Object';
356+
parsedMessages[bundle.locale] = translation == null ? null : Parser(
357+
resourceId,
358+
bundle.file.basename,
359+
translation,
360+
useEscaping: useEscaping,
361+
logger: logger
362+
).parse();
395363
}
364+
// Infer the placeholders
365+
_inferPlaceholders(filenames);
396366
}
397367

398368
final String resourceId;
@@ -402,6 +372,7 @@ class Message {
402372
final Map<LocaleInfo, Node?> parsedMessages;
403373
final Map<String, Placeholder> placeholders;
404374
final bool useEscaping;
375+
final Logger? logger;
405376

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

@@ -496,6 +467,63 @@ class Message {
496467
}),
497468
);
498469
}
470+
471+
// Using parsed translations, attempt to infer types of placeholders used by plurals and selects.
472+
// For undeclared placeholders, create a new placeholder.
473+
void _inferPlaceholders(Map<LocaleInfo, String> filenames) {
474+
// We keep the undeclared placeholders separate so that we can sort them alphabetically afterwards.
475+
final Map<String, Placeholder> undeclaredPlaceholders = <String, Placeholder>{};
476+
// Helper for getting placeholder by name.
477+
Placeholder? getPlaceholder(String name) => placeholders[name] ?? undeclaredPlaceholders[name];
478+
for (final LocaleInfo locale in parsedMessages.keys) {
479+
if (parsedMessages[locale] == null) {
480+
continue;
481+
}
482+
final List<Node> traversalStack = <Node>[parsedMessages[locale]!];
483+
while (traversalStack.isNotEmpty) {
484+
final Node node = traversalStack.removeLast();
485+
if (<ST>[ST.placeholderExpr, ST.pluralExpr, ST.selectExpr].contains(node.type)) {
486+
final String identifier = node.children[1].value!;
487+
Placeholder? placeholder = getPlaceholder(identifier);
488+
if (placeholder == null) {
489+
placeholder = Placeholder(resourceId, identifier, <String, Object?>{});
490+
undeclaredPlaceholders[identifier] = placeholder;
491+
}
492+
if (node.type == ST.pluralExpr) {
493+
placeholder.isPlural = true;
494+
} else if (node.type == ST.selectExpr) {
495+
placeholder.isSelect = true;
496+
}
497+
}
498+
traversalStack.addAll(node.children);
499+
}
500+
}
501+
placeholders.addEntries(
502+
undeclaredPlaceholders.entries
503+
.toList()
504+
..sort((MapEntry<String, Placeholder> p1, MapEntry<String, Placeholder> p2) => p1.key.compareTo(p2.key))
505+
);
506+
507+
for (final Placeholder placeholder in placeholders.values) {
508+
if (placeholder.isPlural && placeholder.isSelect) {
509+
throw L10nException('Placeholder is used as both a plural and select in certain languages.');
510+
} else if (placeholder.isPlural) {
511+
if (placeholder.type == null) {
512+
placeholder.type = 'num';
513+
}
514+
else if (!<String>['num', 'int'].contains(placeholder.type)) {
515+
throw L10nException("Placeholders used in plurals must be of type 'num' or 'int'");
516+
}
517+
} else if (placeholder.isSelect) {
518+
if (placeholder.type == null) {
519+
placeholder.type = 'String';
520+
} else if (placeholder.type != 'String') {
521+
throw L10nException("Placeholders used in selects must be of type 'String'");
522+
}
523+
}
524+
placeholder.type ??= 'Object';
525+
}
526+
}
499527
}
500528

501529
// Represents the contents of one ARB file.
@@ -834,50 +862,3 @@ final Set<String> _iso639Languages = <String>{
834862
'zh',
835863
'zu',
836864
};
837-
838-
// Used in LocalizationsGenerator._generateMethod.generateHelperMethod.
839-
class HelperMethod {
840-
HelperMethod(this.dependentPlaceholders, {this.helper, this.placeholder, this.string }):
841-
assert((() {
842-
// At least one of helper, placeholder, string must be nonnull.
843-
final bool a = helper == null;
844-
final bool b = placeholder == null;
845-
final bool c = string == null;
846-
return (!a && b && c) || (a && !b && c) || (a && b && !c);
847-
})());
848-
849-
Set<Placeholder> dependentPlaceholders;
850-
String? helper;
851-
Placeholder? placeholder;
852-
String? string;
853-
854-
String get helperOrPlaceholder {
855-
if (helper != null) {
856-
return '$helper($methodArguments)';
857-
} else if (string != null) {
858-
return '$string';
859-
} else {
860-
if (placeholder!.requiresFormatting) {
861-
return '${placeholder!.name}String';
862-
} else {
863-
return placeholder!.name;
864-
}
865-
}
866-
}
867-
868-
String get methodParameters {
869-
assert(helper != null);
870-
return dependentPlaceholders.map((Placeholder placeholder) =>
871-
(placeholder.requiresFormatting)
872-
? 'String ${placeholder.name}String'
873-
: '${placeholder.type} ${placeholder.name}').join(', ');
874-
}
875-
876-
String get methodArguments {
877-
assert(helper != null);
878-
return dependentPlaceholders.map((Placeholder placeholder) =>
879-
(placeholder.requiresFormatting)
880-
? '${placeholder.name}String'
881-
: placeholder.name).join(', ');
882-
}
883-
}

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

+20-15
Original file line numberDiff line numberDiff line change
@@ -297,25 +297,23 @@ String generateString(String value) {
297297

298298
/// Given a list of strings, placeholders, or helper function calls, concatenate
299299
/// them into one expression to be returned.
300-
String generateReturnExpr(List<HelperMethod> helpers) {
301-
if (helpers.isEmpty) {
300+
/// If isSingleStringVar is passed, then we want to convert "'$expr'" to simply "expr".
301+
String generateReturnExpr(List<String> expressions, { bool isSingleStringVar = false }) {
302+
if (expressions.isEmpty) {
302303
return "''";
303-
} else if (
304-
helpers.length == 1
305-
&& helpers[0].string == null
306-
&& (helpers[0].placeholder?.type == 'String' || helpers[0].helper != null)
307-
) {
308-
return helpers[0].helperOrPlaceholder;
304+
} else if (isSingleStringVar) {
305+
// If our expression is "$varName" where varName is a String, this is equivalent to just varName.
306+
return expressions[0].substring(1);
309307
} else {
310-
final String string = helpers.reversed.fold<String>('', (String string, HelperMethod helper) {
311-
if (helper.string != null) {
312-
return generateString(helper.string!) + string;
308+
final String string = expressions.reversed.fold<String>('', (String string, String expression) {
309+
if (expression[0] != r'$') {
310+
return generateString(expression) + string;
313311
}
314312
final RegExp alphanumeric = RegExp(r'^([0-9a-zA-Z]|_)+$');
315-
if (alphanumeric.hasMatch(helper.helperOrPlaceholder) && !(string.isNotEmpty && alphanumeric.hasMatch(string[0]))) {
316-
return '\$${helper.helperOrPlaceholder}$string';
313+
if (alphanumeric.hasMatch(expression.substring(1)) && !(string.isNotEmpty && alphanumeric.hasMatch(string[0]))) {
314+
return '$expression$string';
317315
} else {
318-
return '\${${helper.helperOrPlaceholder}}$string';
316+
return '\${${expression.substring(1)}}$string';
319317
}
320318
});
321319
return "'$string'";
@@ -340,6 +338,7 @@ class LocalizationOptions {
340338
this.usesNullableGetter = true,
341339
this.format = false,
342340
this.useEscaping = false,
341+
this.suppressWarnings = false,
343342
}) : assert(useSyntheticPackage != null);
344343

345344
/// The `--arb-dir` argument.
@@ -416,6 +415,11 @@ class LocalizationOptions {
416415
///
417416
/// Whether or not the ICU escaping syntax is used.
418417
final bool useEscaping;
418+
419+
/// The `suppress-warnings` argument.
420+
///
421+
/// Whether or not to suppress warnings.
422+
final bool suppressWarnings;
419423
}
420424

421425
/// Parse the localizations configuration options from [file].
@@ -450,8 +454,9 @@ LocalizationOptions parseLocalizationsOptions({
450454
useSyntheticPackage: _tryReadBool(yamlNode, 'synthetic-package', logger) ?? true,
451455
areResourceAttributesRequired: _tryReadBool(yamlNode, 'required-resource-attributes', logger) ?? false,
452456
usesNullableGetter: _tryReadBool(yamlNode, 'nullable-getter', logger) ?? true,
453-
format: _tryReadBool(yamlNode, 'format', logger) ?? true,
457+
format: _tryReadBool(yamlNode, 'format', logger) ?? false,
454458
useEscaping: _tryReadBool(yamlNode, 'use-escaping', logger) ?? false,
459+
suppressWarnings: _tryReadBool(yamlNode, 'suppress-warnings', logger) ?? false,
455460
);
456461
}
457462

0 commit comments

Comments
 (0)