diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index b5e6086b5b3..f93f2212451 100644 --- a/script/tool/lib/src/format_command.dart +++ b/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 run() async { + Future 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 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 runForPackage(RepositoryPackage package) async { + final Iterable 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 completeRun() async { if (getBoolArg(_failonChangeArg)) { final bool modified = await _didModifyAnything(); if (modified) { @@ -291,13 +314,16 @@ class FormatCommand extends PackageCommand { } } - Future _formatDart(Iterable files) async { + Future _formatDart( + Iterable files, { + Directory? workingDir, + }) async { final Iterable dartFiles = _getPathsWithExtensions(files, {'.dart'}); if (dartFiles.isNotEmpty) { print('Formatting .dart files...'); - final int exitCode = - await _runBatched('dart', ['format'], files: dartFiles); + final int exitCode = await _runBatched('dart', ['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 _runBatched( - String command, - List arguments, { - required Iterable files, - }) async { + Future _runBatched(String command, List arguments, + {required Iterable 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, [...arguments, ...batch], - workingDir: packagesDir); + workingDir: workingDir ?? packagesDir); if (exitCode != 0) { return exitCode; } diff --git a/script/tool/test/format_command_test.dart b/script/tool/test/format_command_test.dart index 33f5104ce13..b2c82378a7d 100644 --- a/script/tool/test/format_command_test.dart +++ b/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 get99CharacterPathExtraFiles(String packageName, int count) { - final int padding = 99 - - packageName.length - - 1 - // the path separator after the package name + List 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( - 'dart', - ['format', ...getPackagesDirRelativePaths(plugin, files)], - packagesDir.path), + ProcessCall('dart', const ['format', ...files], plugin.path), ])); }); @@ -135,12 +130,7 @@ void main() { processRunner.recordedCalls, orderedEquals([ ProcessCall( - 'dart', - [ - 'format', - ...getPackagesDirRelativePaths(plugin, formattedFiles) - ], - packagesDir.path), + 'dart', const ['format', ...formattedFiles], plugin.path), ])); }); @@ -719,12 +709,7 @@ void main() { ], packagesDir.path), ProcessCall( - 'dart', - [ - 'format', - ...getPackagesDirRelativePaths(plugin, dartFiles) - ], - packagesDir.path), + 'dart', const ['format', ...dartFiles], plugin.path), ProcessCall( 'java', [ @@ -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 batch1 = - get99CharacterPathExtraFiles(pluginName, batchSize + 1); + final List batch1 = get99CharacterPathExtraFiles(batchSize + 1); final String extraFile = batch1.removeLast(); - createFakePlugin( + final RepositoryPackage package = createFakePlugin( pluginName, packagesDir, extraFiles: [...batch1, extraFile], @@ -921,9 +905,9 @@ void main() { 'dart', [ '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 batch = - get99CharacterPathExtraFiles(pluginName, batchSize + 1); + final List 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 batch1 = - get99CharacterPathExtraFiles(pluginName, batchSize + 1); + final List batch1 = get99CharacterPathExtraFiles(batchSize + 1); final String extraFile = batch1.removeLast(); - createFakePlugin( + final RepositoryPackage package = createFakePlugin( pluginName, packagesDir, extraFiles: [...batch1, extraFile], @@ -978,9 +960,9 @@ void main() { 'dart', [ 'format', - '$pluginName/$extraFile', + extraFile, ], - packagesDir.path), + package.path), )); }); }