[et] Adds a format command (#50747)

For flutter/flutter#132807

Author: zanderso

ci/bin/format.dart

@@ -269,7 +269,9 @@ abstract class FormatChecker {
         ...types,
       ]);
     }
-    return output.split('\n').where((String line) => line.isNotEmpty).toList();
+    return output.split('\n').where(
+      (String line) => line.isNotEmpty && !line.contains('third_party')
+    ).toList();
   }
 
   /// Generates a reporting function to supply to ProcessRunner to use instead
@@ -285,7 +287,7 @@ abstract class FormatChecker {
       final String pendingStr = pending.toString().padLeft(3);
       final String failedStr = failed.toString().padLeft(3);
 
-      stderr.write('$name Jobs: $percent% done, '
+      stdout.write('$name Jobs: $percent% done, '
           '$completedStr/$totalStr completed, '
           '$inProgressStr in progress, '
           '$pendingStr pending, '
@@ -296,7 +298,7 @@ abstract class FormatChecker {
   /// Clears the last printed report line so garbage isn't left on the terminal.
   @protected
   void reportDone() {
-    stderr.write('\r${' ' * 100}\r');
+    stdout.write('\r${' ' * 100}\r');
   }
 }
 
@@ -436,7 +438,7 @@ class ClangFormatChecker extends FormatChecker {
       } else {
         error('Found ${failed.length} C++/ObjC/Shader file${plural ? 's' : ''}'
             ' which ${plural ? 'were' : 'was'} formatted incorrectly.');
-        stdout.writeln('To fix, run:');
+        stdout.writeln('To fix, run `et format` or:');
         stdout.writeln();
         stdout.writeln('git apply <<DONE');
         for (final WorkerJob job in failed) {
@@ -594,7 +596,7 @@ class JavaFormatChecker extends FormatChecker {
       } else {
         error('Found ${failed.length} Java file${plural ? 's' : ''}'
             ' which ${plural ? 'were' : 'was'} formatted incorrectly.');
-        stdout.writeln('To fix, run:');
+        stdout.writeln('To fix, run `et format` or:');
         stdout.writeln();
         stdout.writeln('git apply <<DONE');
         for (final WorkerJob job in failed) {
@@ -727,7 +729,7 @@ class GnFormatChecker extends FormatChecker {
       } else {
         error('Found ${failed.length} GN file${plural ? 's' : ''}'
             ' which ${plural ? 'were' : 'was'} formatted incorrectly.');
-        stdout.writeln('To fix, run:');
+        stdout.writeln('To fix, run `et format` or:');
         stdout.writeln();
         stdout.writeln('git apply <<DONE');
         for (final WorkerJob job in failed) {
@@ -822,7 +824,12 @@ class PythonFormatChecker extends FormatChecker {
       } else {
         error('Found ${incorrect.length} python file${plural ? 's' : ''}'
             ' which ${plural ? 'were' : 'was'} formatted incorrectly:');
-        incorrect.forEach(stderr.writeln);
+        stdout.writeln('To fix, run `et format` or:');
+        stdout.writeln();
+        stdout.writeln('git apply <<DONE');
+        incorrect.forEach(stdout.writeln);
+        stdout.writeln('DONE');
+        stdout.writeln();
       }
     } else {
       message('All python files formatted correctly.');
@@ -1129,7 +1136,7 @@ Future<int> main(List<String> arguments) async {
     message ??= '';
     switch (type) {
       case MessageType.message:
-        stderr.writeln(message);
+        stdout.writeln(message);
       case MessageType.error:
         stderr.writeln('ERROR: $message');
       case MessageType.warning:
@@ -1160,11 +1167,7 @@ Future<int> main(List<String> arguments) async {
           message('Unable to apply $humanCheckName format fixes.');
         }
       } else {
-        message('Performing $humanCheckName format check');
         stepResult = await checker.checkFormatting();
-        if (!stepResult) {
-          message('Found $humanCheckName format problems.');
-        }
       }
       result = result && stepResult;
     }

tools/engine_tool/README.md

@@ -14,6 +14,8 @@ before it will work.
 The tool has the following commands.
 
 * `help` - Prints helpful information about commands and usage.
+* `format` - Formats files in the engine tree using various off-the-shelf
+formatters.
 * `query builds` - Lists the CI builds described under `ci/builders` that the
 host platform is capable of executing.
 

tools/engine_tool/lib/src/commands/command_runner.dart

@@ -7,6 +7,8 @@ import 'package:args/command_runner.dart';
 import 'package:engine_build_configs/engine_build_configs.dart';
 
 import '../environment.dart';
+import 'command.dart';
+import 'format_command.dart';
 import 'query_command.dart';
 
 /// The root command runner.
@@ -17,10 +19,16 @@ final class ToolCommandRunner extends CommandRunner<int> {
     required this.environment,
     required this.configs,
   }) : super(toolName, toolDescription) {
-    addCommand(QueryCommand(
-      environment: environment,
-      configs: configs,
-    ));
+    final List<CommandBase> commands = <CommandBase>[
+      FormatCommand(
+        environment: environment,
+      ),
+      QueryCommand(
+        environment: environment,
+        configs: configs,
+      ),
+    ];
+    commands.forEach(addCommand);
   }
 
   /// The name of the tool as reported in the tool's usage and help

tools/engine_tool/lib/src/commands/flags.dart

@@ -0,0 +1,18 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// ignore_for_file: public_member_api_docs
+
+// The purpose of this list of flags in a file separate from the command
+// definitions is to ensure that flags are named consistently across
+// subcommands. For example, unless there's a compelling reason to have both,
+// we'd avoid having one subcommand define an --all flag while another defines
+// an --everything flag.
+
+// Keep this list alphabetized.
+const String allFlag = 'all';
+const String builderFlag = 'builder';
+const String dryRunFlag = 'dry-run';
+const String quietFlag = 'quiet';
+const String verboseFlag = 'verbose';

tools/engine_tool/lib/src/commands/format_command.dart

@@ -0,0 +1,165 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io' as io;
+
+import 'package:path/path.dart' as p;
+
+import '../logger.dart';
+import 'command.dart';
+import 'flags.dart';
+
+/// The 'format' command.
+///
+/// The format command implementation below works by spawning another Dart VM to
+/// run the program under ci/bin/format.dart.
+///
+// TODO(team-engine): Part of https://github.com/flutter/flutter/issues/132807.
+// Instead, format.dart should be moved under the engine_tool package and
+// invoked by a function call. The file ci/bin/format.dart should be split up so
+// that each of its `FormatCheckers` is in a separate file under src/formatters,
+// and they should be unit-tested.
+final class FormatCommand extends CommandBase {
+  // ignore: public_member_api_docs
+  FormatCommand({
+    required super.environment,
+  }) {
+    argParser
+      ..addFlag(
+        allFlag,
+        abbr: 'a',
+        help: 'By default only dirty files are checked. This flag causes all '
+              'files to be checked. (Slow)',
+        negatable: false,
+      )
+      ..addFlag(
+        dryRunFlag,
+        abbr: 'd',
+        help: 'Do not fix formatting in-place. Instead, print file diffs to '
+              'the logs.',
+        negatable: false,
+      )
+      ..addFlag(
+        quietFlag,
+        abbr: 'q',
+        help: 'Silences all log messages except for errors and warnings',
+        negatable: false,
+      )
+      ..addFlag(
+        verboseFlag,
+        abbr: 'v',
+        help: 'Prints verbose output',
+        negatable: false,
+      );
+  }
+
+  @override
+  String get name => 'format';
+
+  @override
+  String get description => 'Formats files using standard formatters and styles.';
+
+  @override
+  Future<int> run() async {
+    final bool all = argResults![allFlag]! as bool;
+    final bool dryRun = argResults![dryRunFlag]! as bool;
+    final bool quiet = argResults![quietFlag]! as bool;
+    final bool verbose = argResults![verboseFlag]! as bool;
+    final String formatPath = p.join(
+      environment.engine.flutterDir.path, 'ci', 'bin', 'format.dart',
+    );
+
+    final io.Process process = await environment.processRunner.processManager.start(
+      <String>[
+        environment.platform.resolvedExecutable,
+        formatPath,
+        if (all) '--all-files',
+        if (!dryRun) '--fix',
+        if (verbose) '--verbose',
+      ],
+      workingDirectory: environment.engine.flutterDir.path,
+    );
+    final Completer<void> stdoutComplete = Completer<void>();
+    final Completer<void> stderrComplete = Completer<void>();
+
+    final _FormatStreamer streamer = _FormatStreamer(
+      environment.logger,
+      dryRun,
+      quiet,
+    );
+    process.stdout
+      .transform<String>(const Utf8Decoder())
+      .transform(const LineSplitter())
+      .listen(
+        streamer.nextStdout,
+        onDone: () async => stdoutComplete.complete(),
+      );
+    process.stderr
+      .transform<String>(const Utf8Decoder())
+      .transform(const LineSplitter())
+      .listen(
+        streamer.nextStderr,
+        onDone: () async => stderrComplete.complete(),
+      );
+
+    await Future.wait<void>(<Future<void>>[
+      stdoutComplete.future, stderrComplete.future,
+    ]);
+    final int exitCode = await process.exitCode;
+
+    return exitCode;
+  }
+}
+
+class _FormatStreamer {
+  _FormatStreamer(this.logger, this.dryRun, this.quiet);
+
+  final Logger logger;
+  final bool dryRun;
+  final bool quiet;
+
+  bool inADiff = false;
+  bool inProgress = false;
+
+  void nextStdout(String line) {
+    if (quiet) {
+      return;
+    }
+    final String l = line.trim();
+    if (l == 'To fix, run `et format` or:') {
+      inADiff = true;
+    }
+    if (l.isNotEmpty && (!inADiff || dryRun)) {
+      if (_isProgressLine(l)) {
+        inProgress = true;
+        logger.clearLine();
+        logger.status('$l\r', newline: false);
+      } else {
+        if (inProgress) {
+          logger.clearLine();
+          inProgress = false;
+        }
+        logger.status(l);
+      }
+    }
+    if (l == 'DONE') {
+      inADiff = false;
+    }
+  }
+
+  void nextStderr(String line) {
+    final String l = line.trim();
+    if (l.isEmpty) {
+      return;
+    }
+    logger.error(l);
+  }
+
+  bool _isProgressLine(String l) {
+    final List<String> words = l.split(',');
+    return words.isNotEmpty && words[0].endsWith('% done');
+  }
+}

tools/engine_tool/lib/src/commands/query_command.dart

@@ -5,28 +5,25 @@
 import 'package:engine_build_configs/engine_build_configs.dart';
 
 import 'command.dart';
+import 'flags.dart';
 
-const String _allFlag = 'all';
-const String _builderFlag = 'builder';
-const String _verboseFlag = 'verbose';
-
-/// The root 'query' command.
+// ignore: public_member_api_docs
 final class QueryCommand extends CommandBase {
-  /// Constructs the 'query' command.
+  // ignore: public_member_api_docs
   QueryCommand({
     required super.environment,
     required this.configs,
   }) {
     // Add options here that are common to all queries.
     argParser
       ..addFlag(
-        _allFlag,
+        allFlag,
         abbr: 'a',
         help: 'List all results, even when not relevant on this platform',
         negatable: false,
       )
       ..addOption(
-        _builderFlag,
+        builderFlag,
         abbr: 'b',
         help: 'Restrict the query to a single builder.',
         allowed: <String>[
@@ -42,7 +39,7 @@ final class QueryCommand extends CommandBase {
         },
       )
       ..addFlag(
-        _verboseFlag,
+        verboseFlag,
         abbr: 'v',
         help: 'Respond to queries with extra information',
         negatable: false,
@@ -65,9 +62,9 @@ final class QueryCommand extends CommandBase {
                             'and tests.';
 }
 
-/// The 'query builds' command.
+// ignore: public_member_api_docs
 final class QueryBuildsCommand extends CommandBase {
-  /// Constructs the 'query build' command.
+  // ignore: public_member_api_docs
   QueryBuildsCommand({
     required super.environment,
     required this.configs,
@@ -87,9 +84,9 @@ final class QueryBuildsCommand extends CommandBase {
   Future<int> run() async {
     // Loop through all configs, and log those that are compatible with the
     // current platform.
-    final bool all = parent!.argResults![_allFlag]! as bool;
-    final String? builderName = parent!.argResults![_builderFlag] as String?;
-    final bool verbose = parent!.argResults![_verboseFlag] as bool;
+    final bool all = parent!.argResults![allFlag]! as bool;
+    final String? builderName = parent!.argResults![builderFlag] as String?;
+    final bool verbose = parent!.argResults![verboseFlag] as bool;
     if (!verbose) {
       environment.logger.status(
         'Add --verbose to see detailed information about each builder',