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

Refactor Message class to hold all translations (#115506)

* init

* more fixing

* finish

* fix lint

* address pr comments

* redo checks
parent 4a5dd9c9
......@@ -92,14 +92,14 @@ String _syntheticL10nPackagePath(FileSystem fileSystem) => fileSystem.path.join(
// TODO(thkim1011): Let's store the output of this function in the Message class, so that we don't
// recompute this. See https://github.com/flutter/flutter/issues/112709
List<String> generateMethodParameters(Message message) {
return message.placeholders.map((Placeholder placeholder) {
return message.placeholders.values.map((Placeholder placeholder) {
return '${placeholder.type} ${placeholder.name}';
}).toList();
}
// Similar to above, but is used for passing arguments into helper functions.
List<String> generateMethodArguments(Message message) {
return message.placeholders.map((Placeholder placeholder) => placeholder.name).toList();
return message.placeholders.values.map((Placeholder placeholder) => placeholder.name).toList();
}
String generateDateFormattingLogic(Message message) {
......@@ -107,7 +107,7 @@ String generateDateFormattingLogic(Message message) {
return '@(none)';
}
final Iterable<String> formatStatements = message.placeholders
final Iterable<String> formatStatements = message.placeholders.values
.where((Placeholder placeholder) => placeholder.requiresDateFormatting)
.map((Placeholder placeholder) {
final String? placeholderFormat = placeholder.format;
......@@ -150,7 +150,7 @@ String generateNumberFormattingLogic(Message message) {
return '@(none)';
}
final Iterable<String> formatStatements = message.placeholders
final Iterable<String> formatStatements = message.placeholders.values
.where((Placeholder placeholder) => placeholder.requiresNumFormatting)
.map((Placeholder placeholder) {
final String? placeholderFormat = placeholder.format;
......@@ -502,8 +502,10 @@ class LocalizationsGenerator {
final FileSystem _fs;
Iterable<Message> _allMessages = <Message>[];
late final AppResourceBundleCollection _allBundles = AppResourceBundleCollection(inputDirectory);
late final AppResourceBundle _templateBundle = AppResourceBundle(templateArbFile);
late final Map<LocaleInfo, String> _inputFileNames = Map<LocaleInfo, String>.fromEntries(
_allBundles.bundles.map((AppResourceBundle bundle) => MapEntry<LocaleInfo, String>(bundle.locale, bundle.file.basename))
);
late final LocaleInfo _templateArbLocale = _templateBundle.locale;
@visibleForTesting
......@@ -843,7 +845,7 @@ class LocalizationsGenerator {
// files in inputDirectory. Also initialized: supportedLocales.
void loadResources() {
_allMessages = _templateBundle.resourceIds.map((String id) => Message(
_templateBundle.resources, id, areResourceAttributesRequired,
_templateBundle, _allBundles, id, areResourceAttributesRequired,
));
for (final String resourceId in _templateBundle.resourceIds) {
if (!_isValidGetterAndMethodName(resourceId)) {
......@@ -891,21 +893,19 @@ class LocalizationsGenerator {
String className,
String fileName,
String header,
AppResourceBundle bundle,
AppResourceBundle templateBundle,
Iterable<Message> messages,
final LocaleInfo locale,
) {
final LocaleInfo locale = bundle.locale;
final Iterable<String> methods = messages.map((Message message) {
if (bundle.translationFor(message) == null) {
final Iterable<String> methods = _allMessages.map((Message message) {
if (message.messages[locale] == null) {
_addUnimplementedMessage(locale, message.resourceId);
return _generateMethod(
message,
_templateArbLocale,
);
}
return _generateMethod(
message,
bundle.file.basename,
bundle.translationFor(message) ?? templateBundle.translationFor(message)!,
locale,
);
});
......@@ -923,20 +923,19 @@ class LocalizationsGenerator {
String _generateSubclass(
String className,
AppResourceBundle bundle,
Iterable<Message> messages,
) {
final LocaleInfo locale = bundle.locale;
final String baseClassName = '$className${LocaleInfo.fromString(locale.languageCode).camelCase()}';
messages
.where((Message message) => bundle.translationFor(message) == null)
_allMessages
.where((Message message) => message.messages[locale] == null)
.forEach((Message message) {
_addUnimplementedMessage(locale, message.resourceId);
});
final Iterable<String> methods = messages
.where((Message message) => bundle.translationFor(message) != null)
.map((Message message) => _generateMethod(message, bundle.file.basename, bundle.translationFor(message)!));
final Iterable<String> methods = _allMessages
.where((Message message) => message.messages[locale] != null)
.map((Message message) => _generateMethod(message, locale));
return subclassTemplate
.replaceAll('@(language)', describeLocale(locale.toString()))
......@@ -1016,9 +1015,7 @@ class LocalizationsGenerator {
className,
outputFileName,
header,
_allBundles.bundleFor(locale)!,
_allBundles.bundleFor(_templateArbLocale)!,
_allMessages,
locale,
);
// Every locale for the language except the base class.
......@@ -1029,7 +1026,6 @@ class LocalizationsGenerator {
return _generateSubclass(
className,
_allBundles.bundleFor(locale)!,
_allMessages,
);
});
......@@ -1079,13 +1075,14 @@ class LocalizationsGenerator {
.replaceAll('\n\n\n', '\n\n');
}
String _generateMethod(Message message, String filename, String translationForMessage) {
String _generateMethod(Message message, LocaleInfo locale) {
// Determine if we must import intl for date or number formatting.
if (message.placeholdersRequireFormatting) {
requiresIntlImport = true;
}
final Node node = Parser(message.resourceId, filename, translationForMessage).parse();
final String translationForMessage = message.messages[locale]!;
final Node node = message.parsedMessages[locale]!;
// If parse tree is only a string, then return a getter method.
if (node.children.every((Node child) => child.type == ST.string)) {
// Use the parsed translation to handle escaping with the same behavior.
......@@ -1150,17 +1147,16 @@ class LocalizationsGenerator {
assert(node.children[1].type == ST.identifier);
final Node identifier = node.children[1];
// Check that placeholders exist.
// TODO(thkim1011): Make message.placeholders a map so that we don't need to do linear time search.
// See https://github.com/flutter/flutter/issues/112709
final Placeholder placeholder = message.placeholders.firstWhere(
(Placeholder placeholder) => placeholder.name == identifier.value,
orElse: () {
throw L10nException('''
Make sure that the specified placeholder is defined in your arb file.
$translationForMessage
${Parser.indentForError(identifier.positionInMessage)}''');
}
final Placeholder? placeholder = message.placeholders[identifier.value];
if (placeholder == null) {
throw L10nParserException(
'Make sure that the specified placeholder is defined in your arb file.',
_inputFileNames[locale]!,
message.resourceId,
translationForMessage,
identifier.positionInMessage,
);
}
dependentPlaceholders.add(placeholder);
return HelperMethod(dependentPlaceholders, placeholder: placeholder);
......@@ -1175,25 +1171,27 @@ ${Parser.indentForError(identifier.positionInMessage)}''');
final Node identifier = node.children[1];
final Node pluralParts = node.children[5];
// Check that identifier exists and is of type int or num.
final Placeholder placeholder = message.placeholders.firstWhere(
(Placeholder placeholder) => placeholder.name == identifier.value,
orElse: () {
throw L10nException('''
Make sure that the specified plural placeholder is defined in your arb file.
$translationForMessage
${List<String>.filled(identifier.positionInMessage, ' ').join()}^''');
// Check that placeholders exist and is of type int or num.
final Placeholder? placeholder = message.placeholders[identifier.value];
if (placeholder == null) {
throw L10nParserException(
'Make sure that the specified placeholder is defined in your arb file.',
_inputFileNames[locale]!,
message.resourceId,
translationForMessage,
identifier.positionInMessage,
);
}
if (placeholder.type != 'num' && placeholder.type != 'int') {
throw L10nParserException(
'The specified placeholder must be of type int or num.',
_inputFileNames[locale]!,
message.resourceId,
translationForMessage,
identifier.positionInMessage,
);
}
dependentPlaceholders.add(placeholder);
// TODO(thkim1011): Uncomment the following lines after Message refactor.
// See https://github.com/flutter/flutter/issues/112709.
// if (placeholder.type != 'num' && placeholder.type != 'int') {
// throw L10nException('''
// The specified placeholder must be of type int or num.
// $translationForMessage
// ${List<String>.filled(identifier.positionInMessage, ' ').join()}^''');
// }
for (final Node pluralPart in pluralParts.children.reversed) {
String pluralCase;
......@@ -1239,16 +1237,17 @@ ${Parser.indentForError(pluralPart.positionInMessage)}
assert(node.children[5].type == ST.selectParts);
final Node identifier = node.children[1];
// Check that identifier exists
final Placeholder placeholder = message.placeholders.firstWhere(
(Placeholder placeholder) => placeholder.name == identifier.value,
orElse: () {
throw L10nException('''
Make sure that the specified select placeholder is defined in your arb file.
$translationForMessage
${Parser.indentForError(identifier.positionInMessage)}''');
}
// Check that placeholders exist.
final Placeholder? placeholder = message.placeholders[identifier.value];
if (placeholder == null) {
throw L10nParserException(
'Make sure that the specified placeholder is defined in your arb file.',
_inputFileNames[locale]!,
message.resourceId,
translationForMessage,
identifier.positionInMessage,
);
}
dependentPlaceholders.add(placeholder);
final List<String> selectLogicArgs = <String>[];
final Node selectParts = node.children[5];
......
......@@ -7,6 +7,7 @@ import 'package:intl/locale.dart';
import '../base/file_system.dart';
import '../convert.dart';
import 'localizations_utils.dart';
import 'message_parser.dart';
// The set of date formats that can be automatically localized.
//
......@@ -213,7 +214,7 @@ class Placeholder {
: assert(resourceId != null),
assert(name != null),
example = _stringAttribute(resourceId, name, attributes, 'example'),
type = _stringAttribute(resourceId, name, attributes, 'type') ?? 'Object',
type = _stringAttribute(resourceId, name, attributes, 'type'),
format = _stringAttribute(resourceId, name, attributes, 'format'),
optionalParameters = _optionalParameters(resourceId, name, attributes),
isCustomDateFormat = _boolAttribute(resourceId, name, attributes, 'isCustomDateFormat');
......@@ -221,10 +222,13 @@ class Placeholder {
final String resourceId;
final String name;
final String? example;
String? type;
final String? format;
final List<OptionalParameter> optionalParameters;
final bool? isCustomDateFormat;
// The following will be initialized after all messages are parsed in the Message constructor.
String? type;
bool isPlural = false;
bool isSelect = false;
bool get requiresFormatting => requiresDateFormatting || requiresNumFormatting;
bool get requiresDateFormatting => type == 'DateTime';
......@@ -294,7 +298,7 @@ class Placeholder {
}
}
// One translation: one pair of foo,@foo entries from the template ARB file.
// All translations for a given message specified by a resource id.
//
// The template ARB file must contain an entry called @myResourceId for each
// message named myResourceId. The @ entry describes message parameters
......@@ -309,48 +313,95 @@ class Placeholder {
// The value of this Message is "Hello World". The Message's value is the
// localized string to be shown for the template ARB file's locale.
// The docs for the Placeholder explain how placeholder entries are defined.
// TODO(thkim1011): We need to refactor this Message class to own all the messages in each language.
// See https://github.com/flutter/flutter/issues/112709.
class Message {
Message(Map<String, Object?> bundle, this.resourceId, bool isResourceAttributeRequired)
: assert(bundle != null),
Message(
AppResourceBundle templateBundle,
AppResourceBundleCollection allBundles,
this.resourceId,
bool isResourceAttributeRequired
) : assert(templateBundle != null),
assert(allBundles != null),
assert(resourceId != null && resourceId.isNotEmpty),
value = _value(bundle, resourceId),
description = _description(bundle, resourceId, isResourceAttributeRequired),
placeholders = _placeholders(bundle, resourceId, isResourceAttributeRequired),
_pluralMatch = _pluralRE.firstMatch(_value(bundle, resourceId)),
_selectMatch = _selectRE.firstMatch(_value(bundle, resourceId)) {
if (isPlural) {
final Placeholder placeholder = getCountPlaceholder();
value = _value(templateBundle.resources, resourceId),
description = _description(templateBundle.resources, resourceId, isResourceAttributeRequired),
placeholders = _placeholders(templateBundle.resources, resourceId, isResourceAttributeRequired),
messages = <LocaleInfo, String?>{},
parsedMessages = <LocaleInfo, Node?>{} {
// Filenames for error handling.
final Map<LocaleInfo, String> filenames = <LocaleInfo, String>{};
// Collect all translations from allBundles and parse them.
for (final AppResourceBundle bundle in allBundles.bundles) {
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();
}
// 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';
}
}
static final RegExp _pluralRE = RegExp(r'\s*\{([\w\s,]*),\s*plural\s*,');
static final RegExp _selectRE = RegExp(r'\s*\{([\w\s,]*),\s*select\s*,');
final String resourceId;
final String value;
final String? description;
final List<Placeholder> placeholders;
final RegExpMatch? _pluralMatch;
final RegExpMatch? _selectMatch;
late final Map<LocaleInfo, String?> messages;
final Map<LocaleInfo, Node?> parsedMessages;
final Map<String, Placeholder> placeholders;
bool get isPlural => _pluralMatch != null && _pluralMatch!.groupCount == 1;
bool get isSelect => _selectMatch != null && _selectMatch!.groupCount == 1;
bool get placeholdersRequireFormatting => placeholders.any((Placeholder p) => p.requiresFormatting);
Placeholder getCountPlaceholder() {
assert(isPlural);
final String countPlaceholderName = _pluralMatch![1]!;
return placeholders.firstWhere(
(Placeholder p) => p.name == countPlaceholderName,
orElse: () {
throw L10nException('Cannot find the $countPlaceholderName placeholder in plural message "$resourceId".');
}
);
}
bool get placeholdersRequireFormatting => placeholders.values.any((Placeholder p) => p.requiresFormatting);
static String _value(Map<String, Object?> bundle, String resourceId) {
final Object? value = bundle[resourceId];
......@@ -385,23 +436,6 @@ class Message {
);
}
if (attributes == null) {
void throwEmptyAttributes(final RegExp regExp, final String type) {
final RegExpMatch? match = regExp.firstMatch(_value(bundle, resourceId));
final bool isMatch = match != null && match.groupCount == 1;
if (isMatch) {
throw L10nException(
'Resource attribute "@$resourceId" was not found. Please '
'ensure that $type resources have a corresponding @resource.'
);
}
}
throwEmptyAttributes(_pluralRE, 'plural');
throwEmptyAttributes(_selectRE, 'select');
}
return attributes as Map<String, Object?>?;
}
......@@ -427,18 +461,18 @@ class Message {
return value;
}
static List<Placeholder> _placeholders(
static Map<String, Placeholder> _placeholders(
Map<String, Object?> bundle,
String resourceId,
bool isResourceAttributeRequired,
) {
final Map<String, Object?>? resourceAttributes = _attributes(bundle, resourceId, isResourceAttributeRequired);
if (resourceAttributes == null) {
return <Placeholder>[];
return <String, Placeholder>{};
}
final Object? allPlaceholdersMap = resourceAttributes['placeholders'];
if (allPlaceholdersMap == null) {
return <Placeholder>[];
return <String, Placeholder>{};
}
if (allPlaceholdersMap is! Map<String, Object?>) {
throw L10nException(
......@@ -446,7 +480,8 @@ class Message {
'properly formatted. Ensure that it is a map with string valued keys.'
);
}
return allPlaceholdersMap.keys.map<Placeholder>((String placeholderName) {
return Map<String, Placeholder>.fromEntries(
allPlaceholdersMap.keys.map((String placeholderName) {
final Object? value = allPlaceholdersMap[placeholderName];
if (value is! Map<String, Object?>) {
throw L10nException(
......@@ -455,8 +490,9 @@ class Message {
'with string valued keys.'
);
}
return Placeholder(resourceId, placeholderName, value);
}).toList();
return MapEntry<String, Placeholder>(placeholderName, Placeholder(resourceId, placeholderName, value));
}),
);
}
}
......@@ -532,10 +568,11 @@ class AppResourceBundle {
final File file;
final LocaleInfo locale;
/// JSON representation of the contents of the ARB file.
final Map<String, Object?> resources;
final Iterable<String> resourceIds;
String? translationFor(Message message) => resources[message.resourceId] as String?;
String? translationFor(String resourceId) => resources[resourceId] as String?;
@override
String toString() {
......
......@@ -1618,7 +1618,7 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e
'message',
contains('''
Make sure that the specified placeholder is defined in your arb file.
Hello {name}
[app_en.arb:helloWorld] Hello {name}
^'''),
)),
);
......@@ -1940,13 +1940,10 @@ Hello {name}
throwsA(isA<L10nException>().having(
(L10nException e) => e.message,
'message',
// TODO(thkim1011): Uncomment after work on refactoring the Message class.
// See https://github.com/flutter/flutter/issues/112709.
// contains('''
// Make sure that the specified plural placeholder is defined in your arb file.
// {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}}
// ^'''),
contains('Cannot find the count placeholder in plural message "helloWorlds".'),
contains('''
Make sure that the specified plural placeholder is defined in your arb file.
[app_en.arb:helloWorlds] {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}}
^'''),
)),
);
});
......@@ -1983,13 +1980,10 @@ Hello {name}
throwsA(isA<L10nException>().having(
(L10nException e) => e.message,
'message',
// TODO(thkim1011): Uncomment after work on refactoring the Message class.
// See https://github.com/flutter/flutter/issues/112709.
// contains('''
// Make sure that the specified plural placeholder is defined in your arb file.
// {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}}
// ^'''),
contains('Cannot find the count placeholder in plural message "helloWorlds".'),
contains('''
Make sure that the specified plural placeholder is defined in your arb file.
[app_en.arb:helloWorlds] {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}}
^'''),
)),
);
});
......@@ -2022,13 +2016,10 @@ Hello {name}
throwsA(isA<L10nException>().having(
(L10nException e) => e.message,
'message',
// TODO(thkim1011): Uncomment after work on refactoring the Message class.
// See https://github.com/flutter/flutter/issues/112709.
// contains('''
// Make sure that the specified plural placeholder is defined in your arb file.
// {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}}
// ^'''),
contains('Resource attribute "@helloWorlds" was not found. Please ensure that plural resources have a corresponding @resource.'),
contains('''
Make sure that the specified plural placeholder is defined in your arb file.
[app_en.arb:helloWorlds] {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}}
^'''),
)),
);
});
......@@ -2107,7 +2098,7 @@ Hello {name}
'message',
contains('''
Make sure that the specified select placeholder is defined in your arb file.
{gender, select, female {She} male {He} other {they} }
[app_en.arb:genderSelect] {gender, select, female {She} male {He} other {they} }
^'''),
)),
);
......@@ -2147,7 +2138,7 @@ Make sure that the specified select placeholder is defined in your arb file.
'message',
contains('''
Make sure that the specified select placeholder is defined in your arb file.
{gender, select, female {She} male {He} other {they} }
[app_en.arb:genderSelect] {gender, select, female {She} male {He} other {they} }
^'''),
)),
);
......@@ -2181,13 +2172,10 @@ Make sure that the specified select placeholder is defined in your arb file.
throwsA(isA<L10nException>().having(
(L10nException e) => e.message,
'message',
// TODO(thkim1011): Uncomment after work on refactoring the Message class.
// See https://github.com/flutter/flutter/issues/112709.
// contains('''
// Make sure that the specified select placeholder is defined in your arb file.
// {gender, select, female {She} male {He} other {they} }
// ^'''),
contains('Resource attribute "@genderSelect" was not found. Please ensure that select resources have a corresponding @resource.'),
contains('''
Make sure that the specified select placeholder is defined in your arb file.
[app_en.arb:genderSelect] {gender, select, female {She} male {He} other {they} }
^'''),
)),
);
});
......@@ -2996,50 +2984,6 @@ AppLocalizations lookupAppLocalizations(Locale locale) {
'''));
});
// Regression test for https://github.com/flutter/flutter/pull/93228
testWithoutContext('should use num type for plural', () {
const String arbFile = '''
{
"tryToPollute": "{count, plural, =0{零} =1{一} other{其他}}",
"@tryToPollute": {
"placeholders": {
"count": {
"type": "int"
}
}
},
"withoutType": "{count, plural, =0{零} =1{一} other{其他}}",
"@withoutType": {
"placeholders": {
"count": {}
}
}
}''';
final Directory l10nDirectory = fs.currentDirectory.childDirectory('lib').childDirectory('l10n')
..createSync(recursive: true);
l10nDirectory.childFile(defaultTemplateArbFileName)
.writeAsStringSync(arbFile);
LocalizationsGenerator(
fileSystem: fs,
inputPathString: defaultL10nPathString,
outputPathString: defaultL10nPathString,
templateArbFileName: defaultTemplateArbFileName,
outputFileString: defaultOutputFileString,
classNameString: defaultClassNameString,
logger: logger,
)
..loadResources()
..writeOutputFiles();
final String localizationsFile = fs.file(
fs.path.join(syntheticL10nPackagePath, 'output-localization-file_en.dart'),
).readAsStringSync();
expect(localizationsFile, containsIgnoringWhitespace(r'String tryToPollute(num count) {'));
expect(localizationsFile, containsIgnoringWhitespace(r'String withoutType(num count) {'));
});
// TODO(thkim1011): Uncomment when implementing escaping.
// See https://github.com/flutter/flutter/issues/113455.
// testWithoutContext('escaping with single quotes', () {
......
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