[tools] Format Dart per-package (flutter#8043)

Instead of running Dart formatting on the whole repo at once, run it per
package, from the package's directory. This is slower, but necessary
since the new formatter behaves differently depending on the package's
min SDK version.

Author: stuartmorgan-g

script/tool/lib/src/format_command.dart

@@ -11,7 +11,8 @@ import 'package:meta/meta.dart';
 
 import 'common/core.dart';
 import 'common/output_utils.dart';
-import 'common/package_command.dart';
+import 'common/package_looping_command.dart';
+import 'common/repository_package.dart';
 
 /// In theory this should be 8191, but in practice that was still resulting in
 /// "The input line is too long" errors. This was chosen as a value that worked
@@ -40,7 +41,7 @@ final Uri _kotlinFormatterUrl = Uri.https('maven.org',
     '/maven2/com/facebook/ktfmt/0.46/ktfmt-0.46-jar-with-dependencies.jar');
 
 /// A command to format all package code.
-class FormatCommand extends PackageCommand {
+class FormatCommand extends PackageLoopingCommand {
   /// Creates an instance of the format command.
   FormatCommand(
     super.packagesDir, {
@@ -85,18 +86,19 @@ class FormatCommand extends PackageCommand {
       'to be in your path.';
 
   @override
-  Future<void> run() async {
+  Future<void> initializeRun() async {
     final String javaFormatterPath = await _getJavaFormatterPath();
     final String kotlinFormatterPath = await _getKotlinFormatterPath();
 
-    // This class is not based on PackageLoopingCommand because running the
-    // formatters separately for each package is an order of magnitude slower,
-    // due to the startup overhead of the formatters.
+    // All but Dart is formatted here rather than in runForPackage because
+    // running the formatters separately for each package is an order of
+    // magnitude slower, due to the startup overhead of the formatters.
+    //
+    // Dart has to be run per-package because the formatter can have different
+    // behavior based on the package's SDK, which can't be determined if the
+    // formatter isn't running in the context of the package.
     final Iterable<String> files =
         await _getFilteredFilePaths(getFiles(), relativeTo: packagesDir);
-    if (getBoolArg(_dartArg)) {
-      await _formatDart(files);
-    }
     if (getBoolArg(_javaArg)) {
       await _formatJava(files, javaFormatterPath);
     }
@@ -109,7 +111,28 @@ class FormatCommand extends PackageCommand {
     if (getBoolArg(_swiftArg)) {
       await _formatAndLintSwift(files);
     }
+  }
+
+  @override
+  Future<PackageResult> runForPackage(RepositoryPackage package) async {
+    final Iterable<String> files = await _getFilteredFilePaths(
+      getFilesForPackage(package),
+      relativeTo: package.directory,
+    );
+    if (getBoolArg(_dartArg)) {
+      await _formatDart(files, workingDir: package.directory);
+    }
+    // Success or failure is determined overall in completeRun, since most code
+    // isn't being validated per-package, so just always return success at the
+    // package level.
+    // TODO(stuartmorgan): Consider doing _didModifyAnything checks per-package
+    //  instead, since the other languages are already formatted by the time
+    //  this code is being run.
+    return PackageResult.success();
+  }
 
+  @override
+  Future<void> completeRun() async {
     if (getBoolArg(_failonChangeArg)) {
       final bool modified = await _didModifyAnything();
       if (modified) {
@@ -291,13 +314,16 @@ class FormatCommand extends PackageCommand {
     }
   }
 
-  Future<void> _formatDart(Iterable<String> files) async {
+  Future<void> _formatDart(
+    Iterable<String> files, {
+    Directory? workingDir,
+  }) async {
     final Iterable<String> dartFiles =
         _getPathsWithExtensions(files, <String>{'.dart'});
     if (dartFiles.isNotEmpty) {
       print('Formatting .dart files...');
-      final int exitCode =
-          await _runBatched('dart', <String>['format'], files: dartFiles);
+      final int exitCode = await _runBatched('dart', <String>['format'],
+          files: dartFiles, workingDir: workingDir);
       if (exitCode != 0) {
         printError('Failed to format Dart files: exit code $exitCode.');
         throw ToolExit(_exitFlutterFormatFailed);
@@ -440,11 +466,8 @@ class FormatCommand extends PackageCommand {
   ///
   /// Returns the exit code of the first failure, which stops the run, or 0
   /// on success.
-  Future<int> _runBatched(
-    String command,
-    List<String> arguments, {
-    required Iterable<String> files,
-  }) async {
+  Future<int> _runBatched(String command, List<String> arguments,
+      {required Iterable<String> files, Directory? workingDir}) async {
     final int commandLineMax =
         platform.isWindows ? windowsCommandLineMax : nonWindowsCommandLineMax;
 
@@ -462,7 +485,7 @@ class FormatCommand extends PackageCommand {
       batch.sort(); // For ease of testing.
       final int exitCode = await processRunner.runAndStream(
           command, <String>[...arguments, ...batch],
-          workingDir: packagesDir);
+          workingDir: workingDir ?? packagesDir);
       if (exitCode != 0) {
         return exitCode;
       }

script/tool/test/format_command_test.dart

@@ -63,15 +63,13 @@ void main() {
   }
 
   /// Returns a list of [count] relative paths to pass to [createFakePlugin]
-  /// or [createFakePackage] with name [packageName] such that each path will
-  /// be 99 characters long relative to [packagesDir].
+  /// or [createFakePackage] such that each path will be 99 characters long
+  /// relative to the package directory.
   ///
   /// This is for each of testing batching, since it means each file will
   /// consume 100 characters of the batch length.
-  List<String> get99CharacterPathExtraFiles(String packageName, int count) {
-    final int padding = 99 -
-        packageName.length -
-        1 - // the path separator after the package name
+  List<String> get99CharacterPathExtraFiles(int count) {
+    const int padding = 99 -
         1 - // the path separator after the padding
         10; // the file name
     const int filenameBase = 10000;
@@ -100,10 +98,7 @@ void main() {
     expect(
         processRunner.recordedCalls,
         orderedEquals(<ProcessCall>[
-          ProcessCall(
-              'dart',
-              <String>['format', ...getPackagesDirRelativePaths(plugin, files)],
-              packagesDir.path),
+          ProcessCall('dart', const <String>['format', ...files], plugin.path),
         ]));
   });
 
@@ -135,12 +130,7 @@ void main() {
         processRunner.recordedCalls,
         orderedEquals(<ProcessCall>[
           ProcessCall(
-              'dart',
-              <String>[
-                'format',
-                ...getPackagesDirRelativePaths(plugin, formattedFiles)
-              ],
-              packagesDir.path),
+              'dart', const <String>['format', ...formattedFiles], plugin.path),
         ]));
   });
 
@@ -719,12 +709,7 @@ void main() {
               ],
               packagesDir.path),
           ProcessCall(
-              'dart',
-              <String>[
-                'format',
-                ...getPackagesDirRelativePaths(plugin, dartFiles)
-              ],
-              packagesDir.path),
+              'dart', const <String>['format', ...dartFiles], plugin.path),
           ProcessCall(
               'java',
               <String>[
@@ -899,11 +884,10 @@ void main() {
     const int batchSize = (windowsCommandLineMax ~/ 100) - 1;
 
     // Make the file list one file longer than would fit in the batch.
-    final List<String> batch1 =
-        get99CharacterPathExtraFiles(pluginName, batchSize + 1);
+    final List<String> batch1 = get99CharacterPathExtraFiles(batchSize + 1);
     final String extraFile = batch1.removeLast();
 
-    createFakePlugin(
+    final RepositoryPackage package = createFakePlugin(
       pluginName,
       packagesDir,
       extraFiles: <String>[...batch1, extraFile],
@@ -921,9 +905,9 @@ void main() {
               'dart',
               <String>[
                 'format',
-                '$pluginName\\$extraFile',
+                extraFile,
               ],
-              packagesDir.path),
+              package.path),
         ));
   });
 
@@ -936,8 +920,7 @@ void main() {
     const int batchSize = (windowsCommandLineMax ~/ 100) - 1;
 
     // Make the file list one file longer than would fit in a Windows batch.
-    final List<String> batch =
-        get99CharacterPathExtraFiles(pluginName, batchSize + 1);
+    final List<String> batch = get99CharacterPathExtraFiles(batchSize + 1);
 
     createFakePlugin(
       pluginName,
@@ -956,11 +939,10 @@ void main() {
     const int batchSize = (nonWindowsCommandLineMax ~/ 100) - 1;
 
     // Make the file list one file longer than would fit in the batch.
-    final List<String> batch1 =
-        get99CharacterPathExtraFiles(pluginName, batchSize + 1);
+    final List<String> batch1 = get99CharacterPathExtraFiles(batchSize + 1);
     final String extraFile = batch1.removeLast();
 
-    createFakePlugin(
+    final RepositoryPackage package = createFakePlugin(
       pluginName,
       packagesDir,
       extraFiles: <String>[...batch1, extraFile],
@@ -978,9 +960,9 @@ void main() {
               'dart',
               <String>[
                 'format',
-                '$pluginName/$extraFile',
+                extraFile,
               ],
-              packagesDir.path),
+              package.path),
         ));
   });
 }