diff --git a/packages/flutter_tools/lib/src/build_system/targets/localizations.dart b/packages/flutter_tools/lib/src/build_system/targets/localizations.dart index 609d8e03752..01e4792a32d 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/localizations.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/localizations.dart @@ -57,12 +57,14 @@ class GenerateLocalizationsTarget extends Target { logger: environment.logger, defaultArbDir: defaultArbDir, ); - generateLocalizations( + await generateLocalizations( logger: environment.logger, options: options, projectDir: environment.projectDir, dependenciesDir: environment.buildDir, fileSystem: environment.fileSystem, + artifacts: environment.artifacts, + processManager: environment.processManager, ); final Map dependencies = json.decode( diff --git a/packages/flutter_tools/lib/src/commands/generate_localizations.dart b/packages/flutter_tools/lib/src/commands/generate_localizations.dart index a51eaf78cd1..a7931f0618f 100644 --- a/packages/flutter_tools/lib/src/commands/generate_localizations.dart +++ b/packages/flutter_tools/lib/src/commands/generate_localizations.dart @@ -5,9 +5,7 @@ import 'package:process/process.dart'; import '../artifacts.dart'; -import '../base/common.dart'; import '../base/file_system.dart'; -import '../base/io.dart'; import '../base/logger.dart'; import '../localizations/gen_l10n.dart'; import '../localizations/localizations_utils.dart'; @@ -247,35 +245,14 @@ class GenerateLocalizationsCommand extends FlutterCommand { } // Run the localizations generator. - final LocalizationsGenerator generator = generateLocalizations( + await generateLocalizations( logger: _logger, options: options, projectDir: _fileSystem.currentDirectory, fileSystem: _fileSystem, + artifacts: _artifacts, + processManager: _processManager, ); - final List outputFileList = generator.outputFileList; - final File? untranslatedMessagesFile = generator.untranslatedMessagesFile; - - // All other post processing. - if (options.format) { - if (outputFileList.isEmpty) { - return FlutterCommandResult.success(); - } - final List formatFileList = outputFileList.toList(); - if (untranslatedMessagesFile != null) { - // Don't format the messages file using `dart format`. - formatFileList.remove(untranslatedMessagesFile.absolute.path); - } - if (formatFileList.isEmpty) { - return FlutterCommandResult.success(); - } - final String dartBinary = _artifacts.getArtifactPath(Artifact.engineDartBinary); - final List command = [dartBinary, 'format', ...formatFileList]; - final ProcessResult result = await _processManager.run(command); - if (result.exitCode != 0) { - throwToolExit('Formatting failed: $result', exitCode: result.exitCode); - } - } return FlutterCommandResult.success(); } diff --git a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart index bd8ea9f03c4..01f97aa91d8 100644 --- a/packages/flutter_tools/lib/src/localizations/gen_l10n.dart +++ b/packages/flutter_tools/lib/src/localizations/gen_l10n.dart @@ -3,26 +3,30 @@ // found in the LICENSE file. import 'package:meta/meta.dart'; +import 'package:process/process.dart'; +import '../artifacts.dart'; import '../base/common.dart'; import '../base/file_system.dart'; +import '../base/io.dart'; import '../base/logger.dart'; import '../convert.dart'; import '../flutter_manifest.dart'; - import 'gen_l10n_templates.dart'; import 'gen_l10n_types.dart'; import 'localizations_utils.dart'; import 'message_parser.dart'; /// Run the localizations generation script with the configuration [options]. -LocalizationsGenerator generateLocalizations({ +Future generateLocalizations({ required Directory projectDir, Directory? dependenciesDir, required LocalizationOptions options, required Logger logger, required FileSystem fileSystem, -}) { + required Artifacts artifacts, + required ProcessManager processManager, +}) async { // If generating a synthetic package, generate a warning if // flutter: generate is not set. final FlutterManifest? flutterManifest = FlutterManifest.createFromPath( @@ -71,6 +75,37 @@ LocalizationsGenerator generateLocalizations({ } on L10nException catch (e) { throwToolExit(e.message); } + + final List outputFileList = generator.outputFileList; + final File? untranslatedMessagesFile = generator.untranslatedMessagesFile; + + // All other post processing. + if (options.format) { + final List formatFileList = outputFileList.toList(); + if (untranslatedMessagesFile != null) { + // Don't format the messages file using `dart format`. + formatFileList.remove(untranslatedMessagesFile.absolute.path); + } + if (formatFileList.isEmpty) { + return generator; + } + final String dartBinary = artifacts.getArtifactPath(Artifact.engineDartBinary); + final List command = [dartBinary, 'format', ...formatFileList]; + final ProcessResult result = await processManager.run(command); + if (result.exitCode != 0) { + throw ProcessException( + dartBinary, + command, + ''' +`dart format` failed with exit code ${result.exitCode} + +stdout:\n${result.stdout}\n +stderr:\n${result.stderr}''', + result.exitCode, + ); + } + } + return generator; } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart index a1ac916a80e..552dd37470a 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart @@ -6,6 +6,8 @@ import 'package:file/memory.dart'; import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/build_system/build_system.dart'; +import 'package:flutter_tools/src/build_system/targets/localizations.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/generate_localizations.dart'; @@ -324,6 +326,50 @@ untranslated-messages-file: lib/l10n/untranslated.json ProcessManager: () => FakeProcessManager.any(), }); + // Regression test for https://github.com/flutter/flutter/issues/120530. + testWithoutContext('dart format is run when generateLocalizations is called through build target', () async { + final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb')) + ..createSync(recursive: true); + arbFile.writeAsStringSync(''' +{ + "helloWorld": "Hello, World!", + "@helloWorld": { + "description": "Sample description" + } +}'''); + final File configFile = fileSystem.file('l10n.yaml')..createSync(); + configFile.writeAsStringSync(''' +format: true +'''); + const Target buildTarget = GenerateLocalizationsTarget(); + final File pubspecFile = fileSystem.file('pubspec.yaml')..createSync(); + pubspecFile.writeAsStringSync(BasicProjectWithFlutterGen().pubspec); + processManager.addCommand( + const FakeCommand( + command: [ + 'Artifact.engineDartBinary', + 'format', + '/.dart_tool/flutter_gen/gen_l10n/app_localizations_en.dart', + '/.dart_tool/flutter_gen/gen_l10n/app_localizations.dart', + ] + ) + ); + final Environment environment = Environment.test( + fileSystem.currentDirectory, + artifacts: artifacts, + processManager: processManager, + fileSystem: fileSystem, + logger: BufferLogger.test(), + ); + await buildTarget.build(environment); + + final Directory outputDirectory = fileSystem.directory(fileSystem.path.join('.dart_tool', 'flutter_gen', 'gen_l10n')); + expect(outputDirectory.existsSync(), true); + expect(outputDirectory.childFile('app_localizations_en.dart').existsSync(), true); + expect(outputDirectory.childFile('app_localizations.dart').existsSync(), true); + expect(processManager, hasNoRemainingExpectations); + }); + testUsingContext('nullable-getter defaults to true', () async { final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb')) ..createSync(recursive: true); diff --git a/packages/flutter_tools/test/general.shard/generate_localizations_test.dart b/packages/flutter_tools/test/general.shard/generate_localizations_test.dart index 340cca2f830..4e6070d689a 100644 --- a/packages/flutter_tools/test/general.shard/generate_localizations_test.dart +++ b/packages/flutter_tools/test/general.shard/generate_localizations_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:file/memory.dart'; +import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/convert.dart'; @@ -12,6 +13,7 @@ import 'package:flutter_tools/src/localizations/localizations_utils.dart'; import 'package:yaml/yaml.dart'; import '../src/common.dart'; +import '../src/fake_process_manager.dart'; const String defaultTemplateArbFileName = 'app_en.arb'; const String defaultOutputFileString = 'output-localization-file.dart'; @@ -62,6 +64,8 @@ void _standardFlutterDirectoryL10nSetup(FileSystem fs) { void main() { late MemoryFileSystem fs; late BufferLogger logger; + late Artifacts artifacts; + late ProcessManager processManager; late String defaultL10nPathString; late String syntheticPackagePath; late String syntheticL10nPackagePath; @@ -69,6 +73,8 @@ void main() { setUp(() { fs = MemoryFileSystem.test(); logger = BufferLogger.test(); + artifacts = Artifacts.test(); + processManager = FakeProcessManager.empty(); defaultL10nPathString = fs.path.join('lib', 'l10n'); syntheticPackagePath = fs.path.join('.dart_tool', 'flutter_gen'); @@ -793,7 +799,7 @@ void main() { logger.printError('An error output from a different tool in flutter_tools'); // Should run without error. - generateLocalizations( + await generateLocalizations( fileSystem: fs, options: LocalizationOptions( arbDir: Uri.directory(defaultL10nPathString).path, @@ -804,6 +810,8 @@ void main() { logger: logger, projectDir: fs.currentDirectory, dependenciesDir: fs.currentDirectory, + artifacts: artifacts, + processManager: processManager, ); }); @@ -825,12 +833,14 @@ void main() { ); // Verify that values are correctly passed through the localizations target. - final LocalizationsGenerator generator = generateLocalizations( + final LocalizationsGenerator generator = await generateLocalizations( fileSystem: fs, options: options, logger: logger, projectDir: fs.currentDirectory, dependenciesDir: fs.currentDirectory, + artifacts: artifacts, + processManager: processManager, ); expect(generator.inputDirectory.path, '/lib/l10n/'); @@ -894,6 +904,8 @@ flutter: logger: BufferLogger.test(), projectDir: fs.currentDirectory, dependenciesDir: fs.currentDirectory, + artifacts: artifacts, + processManager: processManager, ), throwsToolExit( message: 'Attempted to generate localizations code without having the ' @@ -906,7 +918,7 @@ flutter: _standardFlutterDirectoryL10nSetup(fs); // Test without headers. - generateLocalizations( + await generateLocalizations( fileSystem: fs, options: LocalizationOptions( arbDir: Uri.directory(defaultL10nPathString).path, @@ -917,6 +929,8 @@ flutter: logger: BufferLogger.test(), projectDir: fs.currentDirectory, dependenciesDir: fs.currentDirectory, + artifacts: artifacts, + processManager: processManager, ); expect(fs.file('/lib/l10n/app_localizations_en.dart').readAsStringSync(), ''' @@ -932,7 +946,7 @@ class AppLocalizationsEn extends AppLocalizations { '''); // Test with headers. - generateLocalizations( + await generateLocalizations( fileSystem: fs, options: LocalizationOptions( header: 'HEADER', @@ -944,6 +958,8 @@ class AppLocalizationsEn extends AppLocalizations { logger: logger, projectDir: fs.currentDirectory, dependenciesDir: fs.currentDirectory, + artifacts: artifacts, + processManager: processManager, ); expect(fs.file('/lib/l10n/app_localizations_en.dart').readAsStringSync(), '''