Skip to content

Commit 6806f6b

Browse files
[CP-stable][Gen-l10n] Infer placeholder types on both templates and localizations (flutter#166337)
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? flutter#163627 ### Changelog Description: Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples [flutter/163627](flutter#163627): Fix issue where placeholder types in ARB localizations weren't used for type inference, causing a possible type mismatch with the placeholder field defined in the template. ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch) Developers are unable to successfully generate localization code without making changes across all of their ARB files. ### Workaround: Is there a workaround for this issue? Downgrade to 3.27 or explicitly define types for all impacted placeholders across all template and localization files. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? See https://github.com/benthillerkus/i18n_repro
1 parent 00a5e71 commit 6806f6b

File tree

3 files changed

+50
-4
lines changed

3 files changed

+50
-4
lines changed

CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ INTERNAL NOTE
3030
- [flutter/163421](https://github.com/flutter/flutter/issues/163421) - Impeller,
3131
Android, Fixes Android Emulator crash when navigating to routes with backdrop
3232
blurs.
33-
- [flutter/165166](https://github.com/flutter/flutter/pull/165166) - Impeller,
34-
All platforms, Text that is scaled over 48x renders incorrectly.
33+
- [flutter/165166](https://github.com/flutter/flutter/pull/165166) - Impeller, All platforms, Text that is scaled over 48x renders incorrectly.
34+
- [flutter/163627](https://github.com/flutter/flutter/pull/163627) - Fix issue where placeholder types in ARB localizations weren't used for type inference, causing a possible type mismatch with the placeholder field defined in the template.
3535
- [flutter/165166](https://github.com/flutter/flutter/pull/165166) - Update CI configurations and tests to use Xcode 16 and iOS 18 simulator.
3636

3737
### [3.29.2](https://github.com/flutter/flutter/releases/tag/3.29.2)

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,9 @@ class Message {
584584
return x && !y && !z || !x && y && !z || !x && !y && z || !x && !y && !z;
585585
}
586586

587-
for (final Placeholder placeholder in templatePlaceholders.values) {
587+
for (final Placeholder placeholder in templatePlaceholders.values.followedBy(
588+
localePlaceholders.values.expand((Map<String, Placeholder> e) => e.values),
589+
)) {
588590
if (!atMostOneOf(placeholder.isPlural, placeholder.isDateTime, placeholder.isSelect)) {
589591
throw L10nException('Placeholder is used as plural/select/datetime in certain languages.');
590592
} else if (placeholder.isPlural) {

packages/flutter_tools/test/general.shard/generate_localizations_test.dart

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1659,6 +1659,50 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e
16591659
expect(content, contains("String get helloWorld => 'Hello {name}'"));
16601660
},
16611661
);
1662+
1663+
// Regression test for https://github.com/flutter/flutter/issues/163627
1664+
//
1665+
// If placeholders have no explicit type (like `int` or `String`) set
1666+
// their type can be inferred.
1667+
//
1668+
// Later in the pipeline it is ensured that each locales placeholder types
1669+
// matches the definitions in the template.
1670+
//
1671+
// If only the types of the template had been inferred,
1672+
// and not for the translation there would be a mismatch:
1673+
// in this case `num` for count and `null` (the default), which is incompatible
1674+
// and `getGeneratedFileContent` would throw an exception.
1675+
//
1676+
// This test ensures that both template and locale can be equally partially defined
1677+
// in the arb.
1678+
testWithoutContext(
1679+
'translation placeholder type definitions can be inferred for plurals',
1680+
() {
1681+
setupLocalizations(<String, String>{
1682+
'en': '''
1683+
{
1684+
"helloWorld": "{count, plural, one{Hello World!} other{Hello Worlds!}}",
1685+
"@helloWorld": {
1686+
"description": "The conventional newborn programmer greeting",
1687+
"placeholders": {
1688+
"count": {}
1689+
}
1690+
}
1691+
}''',
1692+
'de': '''
1693+
{
1694+
"helloWorld": "{count, plural, one{Hallo Welt!} other{Hallo Welten!}}",
1695+
"@helloWorld": {
1696+
"description": "The conventional newborn programmer greeting",
1697+
"placeholders": {
1698+
"count": {}
1699+
}
1700+
}
1701+
}''',
1702+
});
1703+
expect(getGeneratedFileContent(locale: 'en'), isA<String>());
1704+
},
1705+
);
16621706
});
16631707

16641708
group('DateTime tests', () {
@@ -2128,7 +2172,7 @@ import 'output-localization-file_en.dart' deferred as output-localization-file_e
21282172
(L10nException e) => e.message,
21292173
'message',
21302174
contains(
2131-
'The placeholder, springStartDate, has its "type" resource attribute set to the "null" type in locale "ja", but it is "DateTime" in the template placeholder.',
2175+
'The placeholder, springStartDate, has its "type" resource attribute set to the "Object" type in locale "ja", but it is "DateTime" in the template placeholder.',
21322176
),
21332177
),
21342178
),

0 commit comments

Comments
 (0)