Skip to content

Commit 4e75d56

Browse files
AlexV525AlexV525
authored
🐛 Normalize generated file paths for the l10n generator (#169467)
Multiple places use non-normalized file paths when generating the localization files. The PR normalizes file paths when generating the file list JSON file and the dependency file. Fixes #163591 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 8e76fb2 commit 4e75d56

7 files changed

Lines changed: 107 additions & 16 deletions

File tree

‎packages/flutter_tools/lib/src/build_system/targets/localizations.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class GenerateLocalizationsTarget extends Target {
5858
final LocalizationOptions options = parseLocalizationsOptionsFromYAML(
5959
file: configFile,
6060
logger: environment.logger,
61+
fileSystem: environment.fileSystem,
6162
defaultArbDir: defaultArbDir,
6263
defaultSyntheticPackage: !featureFlags.isExplicitPackageDependenciesEnabled,
6364
);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ class GenerateLocalizationsCommand extends FlutterCommand {
260260
options = parseLocalizationsOptionsFromYAML(
261261
file: _fileSystem.file('l10n.yaml'),
262262
logger: _logger,
263+
fileSystem: _fileSystem,
263264
defaultArbDir: defaultArbDir,
264265
defaultSyntheticPackage: !featureFlags.isExplicitPackageDependenciesEnabled,
265266
);

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,14 @@ class LocalizationsGenerator {
10001000
return true;
10011001
}
10021002

1003+
void _addToFileList(List<String> fileList, String path) {
1004+
fileList.add(_fs.path.normalize(path));
1005+
}
1006+
1007+
void _addAllToFileList(List<String> fileList, Iterable<String> paths) {
1008+
fileList.addAll(paths.map(_fs.path.normalize));
1009+
}
1010+
10031011
// Load _allMessages from templateArbFile and _allBundles from all of the ARB
10041012
// files in inputDirectory. Also initialized: supportedLocales.
10051013
void loadResources() {
@@ -1030,7 +1038,8 @@ class LocalizationsGenerator {
10301038
.toList();
10311039
hadErrors = _allMessages.any((Message message) => message.hadErrors);
10321040
if (inputsAndOutputsListFile != null) {
1033-
_inputFileList.addAll(
1041+
_addAllToFileList(
1042+
_inputFileList,
10341043
_allBundles.bundles.map((AppResourceBundle bundle) {
10351044
return bundle.file.absolute.path;
10361045
}),
@@ -1492,7 +1501,7 @@ The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "
14921501
// Generate the required files for localizations.
14931502
_languageFileMap.forEach((File file, String contents) {
14941503
file.writeAsStringSync(useCRLF ? contents.replaceAll('\n', '\r\n') : contents);
1495-
_outputFileList.add(file.absolute.path);
1504+
_addToFileList(_outputFileList, file.absolute.path);
14961505
});
14971506

14981507
baseOutputFile.writeAsStringSync(
@@ -1527,7 +1536,7 @@ The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "
15271536
);
15281537
}
15291538
final File? inputsAndOutputsListFileLocal = inputsAndOutputsListFile;
1530-
_outputFileList.add(baseOutputFile.absolute.path);
1539+
_addToFileList(_outputFileList, baseOutputFile.absolute.path);
15311540
if (inputsAndOutputsListFileLocal != null) {
15321541
// Generate a JSON file containing the inputs and outputs of the gen_l10n script.
15331542
if (!inputsAndOutputsListFileLocal.existsSync()) {
@@ -1548,7 +1557,7 @@ The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "
15481557
void _generateUntranslatedMessagesFile(Logger logger, File untranslatedMessagesFile) {
15491558
if (_unimplementedMessages.isEmpty) {
15501559
untranslatedMessagesFile.writeAsStringSync('{}');
1551-
_outputFileList.add(untranslatedMessagesFile.absolute.path);
1560+
_addToFileList(_outputFileList, untranslatedMessagesFile.absolute.path);
15521561
return;
15531562
}
15541563

@@ -1576,6 +1585,6 @@ The plural cases must be one of "=0", "=1", "=2", "zero", "one", "two", "few", "
15761585

15771586
resultingFile += '}\n';
15781587
untranslatedMessagesFile.writeAsStringSync(resultingFile);
1579-
_outputFileList.add(untranslatedMessagesFile.absolute.path);
1588+
_addToFileList(_outputFileList, untranslatedMessagesFile.absolute.path);
15801589
}
15811590
}

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,7 @@ class LocalizationOptions {
479479
LocalizationOptions parseLocalizationsOptionsFromYAML({
480480
required File file,
481481
required Logger logger,
482+
required FileSystem fileSystem,
482483
required String defaultArbDir,
483484
required bool defaultSyntheticPackage,
484485
}) {
@@ -497,14 +498,24 @@ LocalizationOptions parseLocalizationsOptionsFromYAML({
497498
throw Exception();
498499
}
499500
return LocalizationOptions(
500-
arbDir: _tryReadUri(yamlNode, 'arb-dir', logger)?.path ?? defaultArbDir,
501-
outputDir: _tryReadUri(yamlNode, 'output-dir', logger)?.path,
502-
templateArbFile: _tryReadUri(yamlNode, 'template-arb-file', logger)?.path,
503-
outputLocalizationFile: _tryReadUri(yamlNode, 'output-localization-file', logger)?.path,
504-
untranslatedMessagesFile: _tryReadUri(yamlNode, 'untranslated-messages-file', logger)?.path,
501+
arbDir: _tryReadFilePath(yamlNode, 'arb-dir', logger, fileSystem) ?? defaultArbDir,
502+
outputDir: _tryReadFilePath(yamlNode, 'output-dir', logger, fileSystem),
503+
templateArbFile: _tryReadFilePath(yamlNode, 'template-arb-file', logger, fileSystem),
504+
outputLocalizationFile: _tryReadFilePath(
505+
yamlNode,
506+
'output-localization-file',
507+
logger,
508+
fileSystem,
509+
),
510+
untranslatedMessagesFile: _tryReadFilePath(
511+
yamlNode,
512+
'untranslated-messages-file',
513+
logger,
514+
fileSystem,
515+
),
505516
outputClass: _tryReadString(yamlNode, 'output-class', logger),
506517
header: _tryReadString(yamlNode, 'header', logger),
507-
headerFile: _tryReadUri(yamlNode, 'header-file', logger)?.path,
518+
headerFile: _tryReadFilePath(yamlNode, 'header-file', logger, fileSystem),
508519
useDeferredLoading: _tryReadBool(yamlNode, 'use-deferred-loading', logger),
509520
preferredSupportedLocales: _tryReadStringList(yamlNode, 'preferred-supported-locales', logger),
510521
syntheticPackage:
@@ -596,8 +607,8 @@ List<String>? _tryReadStringList(YamlMap yamlMap, String key, Logger logger) {
596607
throw Exception();
597608
}
598609

599-
// Try to read a valid `Uri` or null from `yamlMap`, otherwise throw.
600-
Uri? _tryReadUri(YamlMap yamlMap, String key, Logger logger) {
610+
// Try to read a valid file `Uri` or null from `yamlMap` to file path, otherwise throw.
611+
String? _tryReadFilePath(YamlMap yamlMap, String key, Logger logger, FileSystem fileSystem) {
601612
final String? value = _tryReadString(yamlMap, key, logger);
602613
if (value == null) {
603614
return null;
@@ -606,5 +617,5 @@ Uri? _tryReadUri(YamlMap yamlMap, String key, Logger logger) {
606617
if (uri == null) {
607618
logger.printError('"$value" must be a relative file URI');
608619
}
609-
return uri;
620+
return uri != null ? fileSystem.path.normalize(uri.path) : null;
610621
}

‎packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:flutter_tools/src/artifacts.dart';
77
import 'package:flutter_tools/src/base/file_system.dart';
88
import 'package:flutter_tools/src/base/logger.dart';
99
import 'package:flutter_tools/src/build_system/build_system.dart';
10+
import 'package:flutter_tools/src/build_system/depfile.dart' show Depfile;
1011
import 'package:flutter_tools/src/build_system/targets/localizations.dart';
1112
import 'package:flutter_tools/src/cache.dart';
1213
import 'package:flutter_tools/src/commands/generate_localizations.dart';
@@ -516,6 +517,42 @@ format: true
516517
},
517518
);
518519

520+
testUsingContext('generates normalized input & output file paths', () async {
521+
final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
522+
..createSync(recursive: true);
523+
arbFile.writeAsStringSync('''
524+
{
525+
"helloWorld": "Hello, World!"
526+
}''');
527+
final File configFile = fileSystem.file('l10n.yaml')..createSync();
528+
// Writing both forward and backward slashes to test both cases.
529+
configFile.writeAsStringSync(r'''
530+
arb-dir: lib/l10n
531+
output-dir: lib\l10n
532+
format: false
533+
''');
534+
final File pubspecFile = fileSystem.file('pubspec.yaml')..createSync();
535+
pubspecFile.writeAsStringSync(BasicProjectWithFlutterGen().pubspec);
536+
537+
processManager.addCommand(const FakeCommand(command: <String>[]));
538+
final Environment environment = Environment.test(
539+
fileSystem.currentDirectory,
540+
artifacts: artifacts,
541+
processManager: processManager,
542+
fileSystem: fileSystem,
543+
logger: BufferLogger.test(),
544+
);
545+
const Target buildTarget = GenerateLocalizationsTarget();
546+
await buildTarget.build(environment);
547+
548+
final File dependencyFile = environment.buildDir.childFile(buildTarget.depfiles.single);
549+
final Depfile depfile = environment.depFileService.parse(dependencyFile);
550+
551+
final String oppositeSeparator = fileSystem.path.separator == '/' ? r'\' : '/';
552+
expect(depfile.inputs, everyElement(isNot(contains(oppositeSeparator))));
553+
expect(depfile.outputs, everyElement(isNot(contains(oppositeSeparator))));
554+
});
555+
519556
testUsingContext(
520557
'nullable-getter defaults to true',
521558
() async {

‎packages/flutter_tools/test/general.shard/build_system/targets/localizations_test.dart‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ nullable-getter: false
5151
final LocalizationOptions options = parseLocalizationsOptionsFromYAML(
5252
file: configFile,
5353
logger: BufferLogger.test(),
54+
fileSystem: fileSystem,
5455
defaultArbDir: fileSystem.path.join('lib', 'l10n'),
5556
defaultSyntheticPackage: true,
5657
);
@@ -90,6 +91,7 @@ nullable-getter: false
9091
final LocalizationOptions options = parseLocalizationsOptionsFromYAML(
9192
file: configFile,
9293
logger: BufferLogger.test(),
94+
fileSystem: fileSystem,
9395
defaultArbDir: fileSystem.path.join('lib', 'l10n'),
9496
defaultSyntheticPackage: true,
9597
);
@@ -118,6 +120,7 @@ nullable-getter: false
118120
final LocalizationOptions options = parseLocalizationsOptionsFromYAML(
119121
file: configFile,
120122
logger: BufferLogger.test(),
123+
fileSystem: fileSystem,
121124
defaultArbDir: fileSystem.path.join('lib', 'l10n'),
122125
defaultSyntheticPackage: false,
123126
);
@@ -136,6 +139,7 @@ preferred-supported-locales: ['en_US', 'de']
136139
final LocalizationOptions options = parseLocalizationsOptionsFromYAML(
137140
file: configFile,
138141
logger: BufferLogger.test(),
142+
fileSystem: fileSystem,
139143
defaultArbDir: fileSystem.path.join('lib', 'l10n'),
140144
defaultSyntheticPackage: true,
141145
);
@@ -156,6 +160,7 @@ use-deferred-loading: string
156160
() => parseLocalizationsOptionsFromYAML(
157161
file: configFile,
158162
logger: BufferLogger.test(),
163+
fileSystem: fileSystem,
159164
defaultArbDir: fileSystem.path.join('lib', 'l10n'),
160165
defaultSyntheticPackage: true,
161166
),
@@ -174,6 +179,7 @@ template-arb-file: {name}_en.arb
174179
() => parseLocalizationsOptionsFromYAML(
175180
file: configFile,
176181
logger: BufferLogger.test(),
182+
fileSystem: fileSystem,
177183
defaultArbDir: fileSystem.path.join('lib', 'l10n'),
178184
defaultSyntheticPackage: true,
179185
),

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:flutter_tools/src/artifacts.dart';
77
import 'package:flutter_tools/src/base/file_system.dart';
88
import 'package:flutter_tools/src/base/logger.dart';
99
import 'package:flutter_tools/src/convert.dart';
10+
import 'package:flutter_tools/src/globals.dart' as globals show platform;
1011
import 'package:flutter_tools/src/localizations/gen_l10n.dart';
1112
import 'package:flutter_tools/src/localizations/gen_l10n_types.dart';
1213
import 'package:flutter_tools/src/localizations/localizations_utils.dart';
@@ -89,16 +90,18 @@ void main() {
8990
bool relaxSyntax = false,
9091
bool useNamedParameters = false,
9192
void Function(Directory)? setup,
93+
FileSystem? fileSystem,
9294
}) {
93-
final Directory l10nDirectory = fs.directory(defaultL10nPath)..createSync(recursive: true);
95+
final Directory l10nDirectory = (fileSystem ?? fs).directory(defaultL10nPath)
96+
..createSync(recursive: true);
9497
for (final String locale in localeToArbFile.keys) {
9598
l10nDirectory.childFile('app_$locale.arb').writeAsStringSync(localeToArbFile[locale]!);
9699
}
97100
if (setup != null) {
98101
setup(l10nDirectory);
99102
}
100103
return LocalizationsGenerator(
101-
fileSystem: fs,
104+
fileSystem: fileSystem ?? fs,
102105
inputPathString: l10nDirectory.path,
103106
outputPathString: outputPathString ?? l10nDirectory.path,
104107
templateArbFileName: defaultTemplateArbFileName,
@@ -613,6 +616,29 @@ void main() {
613616
);
614617
});
615618

619+
testUsingContext('generates normalized input & output file paths', () {
620+
final FileSystem fs = MemoryFileSystem.test(
621+
style: globals.platform.isWindows ? FileSystemStyle.windows : FileSystemStyle.posix,
622+
);
623+
setupLocalizations(
624+
<String, String>{'en': singleMessageArbFileString, 'es': singleEsMessageArbFileString},
625+
fileSystem: fs,
626+
inputsAndOutputsListPath: defaultL10nPath,
627+
);
628+
final File inputsAndOutputsList = fs.file(
629+
fs.path.join(defaultL10nPath, 'gen_l10n_inputs_and_outputs.json'),
630+
);
631+
expect(inputsAndOutputsList.existsSync(), isTrue);
632+
633+
final Map<String, dynamic> jsonResult =
634+
json.decode(inputsAndOutputsList.readAsStringSync()) as Map<String, dynamic>;
635+
final String oppositeSeparator = globals.platform.isWindows ? '/' : r'\';
636+
final List<dynamic> inputList = jsonResult['inputs'] as List<dynamic>;
637+
expect(inputList, everyElement(isNot(contains(oppositeSeparator))));
638+
final List<dynamic> outputList = jsonResult['outputs'] as List<dynamic>;
639+
expect(outputList, everyElement(isNot(contains(oppositeSeparator))));
640+
});
641+
616642
testWithoutContext('setting both a headerString and a headerFile should fail', () {
617643
expect(
618644
() {

0 commit comments

Comments
 (0)