[gen_l10n] Add option to format generated localizations files (#109171)

* init

* fix

* fix 2

* fix 3

* tests

* fix tests

* clarify help text

* fix all tests

* fix formatting?

* add second test

* unused import

* remove print

* trailing spaces

* artifacts is never null

* fix

packages/flutter_tools/lib/executable.dart

@@ -178,6 +178,8 @@ List<FlutterCommand> generateCommands({
   GenerateLocalizationsCommand(
     fileSystem: globals.fs,
     logger: globals.logger,
+    artifacts: globals.artifacts!,
+    processManager: globals.processManager,
   ),
   InstallCommand(),
   LogsCommand(),

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

@@ -2,8 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+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 '../globals.dart' as globals;
 import '../localizations/gen_l10n.dart';
@@ -21,9 +25,13 @@ class GenerateLocalizationsCommand extends FlutterCommand {
   GenerateLocalizationsCommand({
     required FileSystem fileSystem,
     required Logger logger,
+    required Artifacts artifacts,
+    required ProcessManager processManager,
   }) :
     _fileSystem = fileSystem,
-    _logger = logger {
+    _logger = logger,
+    _artifacts = artifacts,
+    _processManager = processManager {
     argParser.addOption(
       'arb-dir',
       defaultsTo: globals.fs.path.join('lib', 'l10n'),
@@ -180,10 +188,16 @@ class GenerateLocalizationsCommand extends FlutterCommand {
             'Localizations.of(context), removing the need for null checking in '
             'user code.'
     );
+    argParser.addFlag(
+      'format',
+      help: 'When specified, the "dart format" command is run after generating the localization files.'
+    );
   }
 
   final FileSystem _fileSystem;
   final Logger _logger;
+  final Artifacts _artifacts;
+  final ProcessManager _processManager;
 
   @override
   String get description => 'Generate localizations for the current project.';
@@ -196,6 +210,10 @@ class GenerateLocalizationsCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
+    List<String> outputFileList;
+
+    bool format = boolArg('format') ?? false;
+
     if (_fileSystem.file('l10n.yaml').existsSync()) {
       final LocalizationOptions options = parseLocalizationsOptions(
         file: _fileSystem.file('l10n.yaml'),
@@ -207,57 +225,71 @@ class GenerateLocalizationsCommand extends FlutterCommand {
         'To use the command line arguments, delete the l10n.yaml file in the '
         'Flutter project.\n\n'
       );
-      generateLocalizations(
+      outputFileList = generateLocalizations(
         logger: _logger,
         options: options,
         projectDir: _fileSystem.currentDirectory,
         fileSystem: _fileSystem,
-      );
-      return FlutterCommandResult.success();
-    }
+      ).outputFileList;
+      format = format || options.format;
+    } else {
+      final String inputPathString = stringArgDeprecated('arb-dir')!; // Has default value, cannot be null.
+      final String? outputPathString = stringArgDeprecated('output-dir');
+      final String outputFileString = stringArgDeprecated('output-localization-file')!; // Has default value, cannot be null.
+      final String templateArbFileName = stringArgDeprecated('template-arb-file')!; // Has default value, cannot be null.
+      final String? untranslatedMessagesFile = stringArgDeprecated('untranslated-messages-file');
+      final String classNameString = stringArgDeprecated('output-class')!; // Has default value, cannot be null.
+      final List<String> preferredSupportedLocales = stringsArg('preferred-supported-locales');
+      final String? headerString = stringArgDeprecated('header');
+      final String? headerFile = stringArgDeprecated('header-file');
+      final bool useDeferredLoading = boolArgDeprecated('use-deferred-loading');
+      final String? inputsAndOutputsListPath = stringArgDeprecated('gen-inputs-and-outputs-list');
+      final bool useSyntheticPackage = boolArgDeprecated('synthetic-package');
+      final String? projectPathString = stringArgDeprecated('project-dir');
+      final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes');
+      final bool usesNullableGetter = boolArgDeprecated('nullable-getter');
 
-    final String inputPathString = stringArgDeprecated('arb-dir')!; // Has default value, cannot be null.
-    final String? outputPathString = stringArgDeprecated('output-dir');
-    final String outputFileString = stringArgDeprecated('output-localization-file')!; // Has default value, cannot be null.
-    final String templateArbFileName = stringArgDeprecated('template-arb-file')!; // Has default value, cannot be null.
-    final String? untranslatedMessagesFile = stringArgDeprecated('untranslated-messages-file');
-    final String classNameString = stringArgDeprecated('output-class')!; // Has default value, cannot be null.
-    final List<String> preferredSupportedLocales = stringsArg('preferred-supported-locales');
-    final String? headerString = stringArgDeprecated('header');
-    final String? headerFile = stringArgDeprecated('header-file');
-    final bool useDeferredLoading = boolArgDeprecated('use-deferred-loading');
-    final String? inputsAndOutputsListPath = stringArgDeprecated('gen-inputs-and-outputs-list');
-    final bool useSyntheticPackage = boolArgDeprecated('synthetic-package');
-    final String? projectPathString = stringArgDeprecated('project-dir');
-    final bool areResourceAttributesRequired = boolArgDeprecated('required-resource-attributes');
-    final bool usesNullableGetter = boolArgDeprecated('nullable-getter');
+      precacheLanguageAndRegionTags();
 
-    precacheLanguageAndRegionTags();
+      try {
+        outputFileList = (LocalizationsGenerator(
+          fileSystem: _fileSystem,
+          inputPathString: inputPathString,
+          outputPathString: outputPathString,
+          templateArbFileName: templateArbFileName,
+          outputFileString: outputFileString,
+          classNameString: classNameString,
+          preferredSupportedLocales: preferredSupportedLocales,
+          headerString: headerString,
+          headerFile: headerFile,
+          useDeferredLoading: useDeferredLoading,
+          inputsAndOutputsListPath: inputsAndOutputsListPath,
+          useSyntheticPackage: useSyntheticPackage,
+          projectPathString: projectPathString,
+          areResourceAttributesRequired: areResourceAttributesRequired,
+          untranslatedMessagesFile: untranslatedMessagesFile,
+          usesNullableGetter: usesNullableGetter,
+          logger: _logger,
+        )
+          ..loadResources()
+          ..writeOutputFiles())
+          .outputFileList;
+      } on L10nException catch (e) {
+        throwToolExit(e.message);
+      }
+    }
 
-    try {
-      LocalizationsGenerator(
-        fileSystem: _fileSystem,
-        inputPathString: inputPathString,
-        outputPathString: outputPathString,
-        templateArbFileName: templateArbFileName,
-        outputFileString: outputFileString,
-        classNameString: classNameString,
-        preferredSupportedLocales: preferredSupportedLocales,
-        headerString: headerString,
-        headerFile: headerFile,
-        useDeferredLoading: useDeferredLoading,
-        inputsAndOutputsListPath: inputsAndOutputsListPath,
-        useSyntheticPackage: useSyntheticPackage,
-        projectPathString: projectPathString,
-        areResourceAttributesRequired: areResourceAttributesRequired,
-        untranslatedMessagesFile: untranslatedMessagesFile,
-        usesNullableGetter: usesNullableGetter,
-        logger: _logger,
-      )
-        ..loadResources()
-        ..writeOutputFiles();
-    } on L10nException catch (e) {
-      throwToolExit(e.message);
+    // All other post processing.
+    if (format) {
+      if (outputFileList.isEmpty) {
+        return FlutterCommandResult.success();
+      }
+      final String dartBinary = _artifacts.getHostArtifact(HostArtifact.engineDartBinary).path;
+      final List<String> command = <String>[dartBinary, 'format', ...outputFileList];
+      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

@@ -811,6 +811,10 @@ class LocalizationsGenerator {
     return _allBundles.bundles.map((AppResourceBundle bundle) => bundle.file.path).toList();
   }
 
+  List<String> get outputFileList {
+    return _outputFileList;
+  }
+
   /// The supported language codes as found in the arb files located in
   /// [inputDirectory].
   final Set<String> supportedLanguageCodes = <String>{};
@@ -1339,7 +1343,7 @@ class LocalizationsGenerator {
         || message.placeholdersRequireFormatting;
   });
 
-  void writeOutputFiles({ bool isFromYaml = false }) {
+  List<String> writeOutputFiles({ bool isFromYaml = false }) {
     // First, generate the string contents of all necessary files.
     final String generatedLocalizationsFile = _generateCode();
 
@@ -1372,9 +1376,7 @@ class LocalizationsGenerator {
     // Generate the required files for localizations.
     _languageFileMap.forEach((File file, String contents) {
       file.writeAsStringSync(contents);
-      if (inputsAndOutputsListFile != null) {
-        _outputFileList.add(file.absolute.path);
-      }
+      _outputFileList.add(file.absolute.path);
     });
 
     baseOutputFile.writeAsStringSync(generatedLocalizationsFile);
@@ -1407,9 +1409,8 @@ class LocalizationsGenerator {
       );
     }
     final File? inputsAndOutputsListFileLocal = inputsAndOutputsListFile;
+    _outputFileList.add(baseOutputFile.absolute.path);
     if (inputsAndOutputsListFileLocal != null) {
-      _outputFileList.add(baseOutputFile.absolute.path);
-
       // Generate a JSON file containing the inputs and outputs of the gen_l10n script.
       if (!inputsAndOutputsListFileLocal.existsSync()) {
         inputsAndOutputsListFileLocal.createSync(recursive: true);
@@ -1422,14 +1423,14 @@ class LocalizationsGenerator {
         }),
       );
     }
+
+    return _outputFileList;
   }
 
   void _generateUntranslatedMessagesFile(Logger logger, File untranslatedMessagesFile) {
     if (_unimplementedMessages.isEmpty) {
       untranslatedMessagesFile.writeAsStringSync('{}');
-      if (inputsAndOutputsListFile != null) {
-        _outputFileList.add(untranslatedMessagesFile.absolute.path);
-      }
+      _outputFileList.add(untranslatedMessagesFile.absolute.path);
       return;
     }
 
@@ -1457,8 +1458,6 @@ class LocalizationsGenerator {
 
     resultingFile += '}\n';
     untranslatedMessagesFile.writeAsStringSync(resultingFile);
-    if (inputsAndOutputsListFile != null) {
-      _outputFileList.add(untranslatedMessagesFile.absolute.path);
-    }
+    _outputFileList.add(untranslatedMessagesFile.absolute.path);
   }
 }

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

@@ -311,6 +311,7 @@ class LocalizationOptions {
     this.useSyntheticPackage = true,
     this.areResourceAttributesRequired = false,
     this.usesNullableGetter = true,
+    this.format = false,
   }) : assert(useSyntheticPackage != null);
 
   /// The `--arb-dir` argument.
@@ -377,6 +378,11 @@ class LocalizationOptions {
   ///
   /// Whether or not the localizations class getter is nullable.
   final bool usesNullableGetter;
+
+  /// The `format` argument.
+  ///
+  /// Whether or not to format the generated files.
+  final bool format;
 }
 
 /// Parse the localizations configuration options from [file].
@@ -411,6 +417,7 @@ LocalizationOptions parseLocalizationsOptions({
     useSyntheticPackage: _tryReadBool(yamlNode, 'synthetic-package', logger) ?? true,
     areResourceAttributesRequired: _tryReadBool(yamlNode, 'required-resource-attributes', logger) ?? false,
     usesNullableGetter: _tryReadBool(yamlNode, 'nullable-getter', logger) ?? true,
+    format: _tryReadBool(yamlNode, 'format', logger) ?? true,
   );
 }
 

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

@@ -5,19 +5,23 @@
 // @dart = 2.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/cache.dart';
 import 'package:flutter_tools/src/commands/generate_localizations.dart';
-import 'package:flutter_tools/src/runner/flutter_command.dart';
 
 import '../../integration.shard/test_data/basic_project.dart';
 import '../../src/common.dart';
 import '../../src/context.dart';
+import '../../src/fake_process_manager.dart';
 import '../../src/test_flutter_command_runner.dart';
 
 void main() {
   FileSystem fileSystem;
+  BufferLogger logger;
+  Artifacts artifacts;
+  FakeProcessManager processManager;
 
   setUpAll(() {
     Cache.disableLocking();
@@ -25,10 +29,12 @@ void main() {
 
   setUp(() {
     fileSystem = MemoryFileSystem.test();
+    logger = BufferLogger.test();
+    artifacts = Artifacts.test();
+    processManager = FakeProcessManager.empty();
   });
 
   testUsingContext('default l10n settings', () async {
-    final BufferLogger logger = BufferLogger.test();
     final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
       ..createSync(recursive: true);
     arbFile.writeAsStringSync('''
@@ -41,11 +47,11 @@ void main() {
     final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
       fileSystem: fileSystem,
       logger: logger,
+      artifacts: artifacts,
+      processManager: processManager,
     );
     await createTestCommandRunner(command).run(<String>['gen-l10n']);
 
-    final FlutterCommandResult result = await command.runCommand();
-    expect(result.exitStatus, ExitStatus.success);
     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);
@@ -56,7 +62,6 @@ void main() {
   });
 
   testUsingContext('not using synthetic packages', () async {
-    final BufferLogger logger = BufferLogger.test();
     final Directory l10nDirectory = fileSystem.directory(
       fileSystem.path.join('lib', 'l10n'),
     );
@@ -75,14 +80,14 @@ void main() {
     final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
       fileSystem: fileSystem,
       logger: logger,
+      artifacts: artifacts,
+      processManager: processManager,
     );
     await createTestCommandRunner(command).run(<String>[
       'gen-l10n',
       '--no-synthetic-package',
     ]);
 
-    final FlutterCommandResult result = await command.runCommand();
-    expect(result.exitStatus, ExitStatus.success);
     expect(l10nDirectory.existsSync(), true);
     expect(l10nDirectory.childFile('app_localizations_en.dart').existsSync(), true);
     expect(l10nDirectory.childFile('app_localizations.dart').existsSync(), true);
@@ -92,7 +97,6 @@ void main() {
   });
 
   testUsingContext('throws error when arguments are invalid', () async {
-    final BufferLogger logger = BufferLogger.test();
     final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
       ..createSync(recursive: true);
     arbFile.writeAsStringSync('''
@@ -107,6 +111,8 @@ void main() {
     final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
       fileSystem: fileSystem,
       logger: logger,
+      artifacts: artifacts,
+      processManager: processManager,
     );
     expect(
       () => createTestCommandRunner(command).run(<String>[
@@ -122,7 +128,6 @@ void main() {
   });
 
   testUsingContext('l10n yaml file takes precedence over command line arguments', () async {
-    final BufferLogger logger = BufferLogger.test();
     final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
       ..createSync(recursive: true);
     arbFile.writeAsStringSync('''
@@ -138,11 +143,11 @@ void main() {
     final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
       fileSystem: fileSystem,
       logger: logger,
+      artifacts: artifacts,
+      processManager: processManager,
     );
     await createTestCommandRunner(command).run(<String>['gen-l10n']);
 
-    final FlutterCommandResult result = await command.runCommand();
-    expect(result.exitStatus, ExitStatus.success);
     expect(logger.statusText, contains('Because l10n.yaml exists, the options defined there will be used instead.'));
     final Directory outputDirectory = fileSystem.directory(fileSystem.path.join('.dart_tool', 'flutter_gen', 'gen_l10n'));
     expect(outputDirectory.existsSync(), true);
@@ -154,7 +159,6 @@ void main() {
   });
 
   testUsingContext('nullable-getter help message is expected string', () async {
-    final BufferLogger logger = BufferLogger.test();
     final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
       ..createSync(recursive: true);
     arbFile.writeAsStringSync('''
@@ -170,6 +174,8 @@ void main() {
     final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
       fileSystem: fileSystem,
       logger: logger,
+      artifacts: artifacts,
+      processManager: processManager,
     );
     await createTestCommandRunner(command).run(<String>['gen-l10n']);
     expect(command.usage, contains(' If this value is set to false, then '));
@@ -177,4 +183,88 @@ void main() {
     FileSystem: () => fileSystem,
     ProcessManager: () => FakeProcessManager.any(),
   });
+
+  testUsingContext('dart format is run when --format is passed', () 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"
+  }
+}''');
+    processManager.addCommand(
+      const FakeCommand(
+        command: <String>[
+          'HostArtifact.engineDartBinary',
+          'format',
+          '/.dart_tool/flutter_gen/gen_l10n/app_localizations_en.dart',
+          '/.dart_tool/flutter_gen/gen_l10n/app_localizations.dart',
+        ]
+      )
+    );
+
+    final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
+      fileSystem: fileSystem,
+      logger: logger,
+      artifacts: artifacts,
+      processManager: processManager,
+    );
+
+    await createTestCommandRunner(command).run(<String>['gen-l10n', '--format']);
+
+    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);
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+  });
+
+  testUsingContext('dart format is run when format: true is passed into l10n.yaml', () 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
+''');
+    final File pubspecFile = fileSystem.file('pubspec.yaml')..createSync();
+    pubspecFile.writeAsStringSync(BasicProjectWithFlutterGen().pubspec);
+    processManager.addCommand(
+      const FakeCommand(
+        command: <String>[
+          'HostArtifact.engineDartBinary',
+          'format',
+          '/.dart_tool/flutter_gen/gen_l10n/app_localizations_en.dart',
+          '/.dart_tool/flutter_gen/gen_l10n/app_localizations.dart',
+        ]
+      )
+    );
+    final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
+      fileSystem: fileSystem,
+      logger: logger,
+      artifacts: artifacts,
+      processManager: processManager,
+    );
+    await createTestCommandRunner(command).run(<String>['gen-l10n']);
+
+    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);
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+  });
 }