Fix gen-l10n format: true so that it applies to when it gets called via build target (#127886)

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<String, Object?> dependencies = json.decode(

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<String> outputFileList = generator.outputFileList;
-    final File? untranslatedMessagesFile = generator.untranslatedMessagesFile;
-
-    // All other post processing.
-    if (options.format) {
-      if (outputFileList.isEmpty) {
-        return FlutterCommandResult.success();
-      }
-      final List<String> 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<String> command = <String>[dartBinary, 'format', ...formatFileList];
-      final ProcessResult result = await _processManager.run(command);
-      if (result.exitCode != 0) {
-        throwToolExit('Formatting failed: $result', exitCode: result.exitCode);
-      }
-    }
 
     return FlutterCommandResult.success();
   }

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<LocalizationsGenerator> 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<String> outputFileList = generator.outputFileList;
+  final File? untranslatedMessagesFile = generator.untranslatedMessagesFile;
+
+  // All other post processing.
+  if (options.format) {
+    final List<String> 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<String> command = <String>[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;
 }
 

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: <String>[
+          '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);

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(), '''