From a8f755647275e46a5842e55be7f2db714b601d84 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Tue, 10 Dec 2024 16:02:05 -0800 Subject: [PATCH] Add support for dart formatter (#57075) Currently it is off by default since we haven't migrated any files over to the new format. To try it out, run `dart ci/bin/format.dart -c dart -f`. Once we turn it on by default `et` will automatically format dart files as it calls into `format.dart`. --- ci/bin/format.dart | 146 ++++++++++++++++++++++++++++++++++++++- ci/test/format_test.dart | 27 ++++++++ 2 files changed, 170 insertions(+), 3 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index 88390bd826256..5f4c94b3345b3 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -40,6 +40,7 @@ enum MessageType { } enum FormatCheck { + dart, gn, java, python, @@ -53,6 +54,8 @@ FormatCheck nameToFormatCheck(String name) { switch (name) { case 'clang': return FormatCheck.clang; + case 'dart': + return FormatCheck.dart; case 'gn': return FormatCheck.gn; case 'java': @@ -72,6 +75,8 @@ String formatCheckToName(FormatCheck check) { switch (check) { case FormatCheck.clang: return 'C++/ObjC/Shader'; + case FormatCheck.dart: + return 'Dart'; case FormatCheck.gn: return 'GN'; case FormatCheck.java: @@ -87,8 +92,7 @@ String formatCheckToName(FormatCheck check) { List formatCheckNames() { return FormatCheck.values - .map((FormatCheck check) => - check.toString().replaceFirst('$FormatCheck.', '')) + .map((FormatCheck check) => check.name) .toList(); } @@ -141,6 +145,14 @@ abstract class FormatChecker { allFiles: allFiles, messageCallback: messageCallback, ); + case FormatCheck.dart: + return DartFormatChecker( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + allFiles: allFiles, + messageCallback: messageCallback, + ); case FormatCheck.gn: return GnFormatChecker( processManager: processManager, @@ -786,6 +798,133 @@ class GnFormatChecker extends FormatChecker { } } +/// Checks the format of any .dart files using the "dart format" command. +class DartFormatChecker extends FormatChecker { + DartFormatChecker({ + super.processManager, + required super.baseGitRef, + required Directory repoDir, + super.allFiles, + super.messageCallback, + }) : super( + repoDir: repoDir, + ) { + // $ENGINE/flutter/third_party/dart/tools/sdks/dart-sdk/bin/dart + _dartBin = path.join( + repoDir.absolute.parent.path, + 'flutter', + 'third_party', + 'dart', + 'tools', + 'sdks', + 'dart-sdk', + 'bin', + Platform.isWindows ? 'dart.exe' : 'dart', + ); + } + + late final String _dartBin; + + @override + Future checkFormatting() async { + message('Checking Dart formatting...'); + return (await _runDartFormat(fixing: false)) == 0; + } + + @override + Future fixFormatting() async { + message('Fixing Dart formatting...'); + await _runDartFormat(fixing: true); + // The dart formatter shouldn't fail when fixing errors. + return true; + } + + Future _runDartFormat({required bool fixing}) async { + final List filesToCheck = await getFileList(['*.dart']); + + final List cmd = [ + _dartBin, + 'format', + '--set-exit-if-changed', + '--show=none', + if (!fixing) '--output=show', + if (fixing) '--output=write', + ]; + final List jobs = []; + for (final String file in filesToCheck) { + jobs.add(WorkerJob([...cmd, file])); + } + final ProcessPool dartFmt = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('dart format'), + ); + + Iterable incorrect; + if (!fixing) { + final Stream completedJobs = dartFmt.startWorkers(jobs); + final List diffJobs = []; + await for (final WorkerJob completedJob in completedJobs) { + if (completedJob.result.exitCode == 1) { + diffJobs.add( + WorkerJob( + [ + 'git', + 'diff', + '--no-index', + '--no-color', + '--ignore-cr-at-eol', + '--', + completedJob.command.last, + '-', + ], + stdinRaw: codeUnitsAsStream(completedJob.result.stdoutRaw), + ), + ); + } + } + final ProcessPool diffPool = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('diff'), + ); + final List completedDiffs = await diffPool.runToCompletion(diffJobs); + incorrect = completedDiffs.where((WorkerJob job) { + return job.result.exitCode != 0; + }); + } else { + final List completedJobs = await dartFmt.runToCompletion(jobs); + incorrect = completedJobs.where((WorkerJob job) => job.result.exitCode == 1); + } + + reportDone(); + + if (incorrect.isNotEmpty) { + final bool plural = incorrect.length > 1; + if (fixing) { + message('Fixing ${incorrect.length} dart file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly.'); + } else { + error('Found ${incorrect.length} Dart file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly.'); + stdout.writeln('To fix, run `et format` or:'); + stdout.writeln(); + stdout.writeln('git apply < main(List arguments) async { parser.addMultiOption('check', abbr: 'c', allowed: formatCheckNames(), - defaultsTo: formatCheckNames(), + // TODO(goderbauer): Enable dart by default when we turned on the formatter. + defaultsTo: formatCheckNames()..remove(FormatCheck.dart.name), help: 'Specifies which checks will be performed. Defaults to all checks. ' 'May be specified more than once to perform multiple types of checks. '); parser.addFlag('verbose', help: 'Print verbose output.', defaultsTo: verbose); diff --git a/ci/test/format_test.dart b/ci/test/format_test.dart index da06a2d5f3e38..361644044aa02 100644 --- a/ci/test/format_test.dart +++ b/ci/test/format_test.dart @@ -23,6 +23,8 @@ final FileContentPair ccContentPair = FileContentPair( 'int main(){return 0;}\n', 'int main() {\n return 0;\n}\n'); final FileContentPair hContentPair = FileContentPair('int\nmain\n()\n;\n', 'int main();\n'); +final FileContentPair dartContentPair = FileContentPair( + 'enum \n\nfoo {\n entry1,\n entry2,\n}', 'enum foo { entry1, entry2 }\n'); final FileContentPair gnContentPair = FileContentPair( 'test\n(){testvar=true}\n', 'test() {\n testvar = true\n}\n'); final FileContentPair javaContentPair = FileContentPair( @@ -60,6 +62,10 @@ class TestFileFixture { final io.File hFile = io.File('${repoDir.path}/format_test.h'); hFile.writeAsStringSync(hContentPair.original); files.add(hFile); + case target.FormatCheck.dart: + final io.File dartFile = io.File('${repoDir.path}/format_test.dart'); + dartFile.writeAsStringSync(dartContentPair.original); + files.add(dartFile); case target.FormatCheck.gn: final io.File gnFile = io.File('${repoDir.path}/format_test.gn'); gnFile.writeAsStringSync(gnContentPair.original); @@ -116,6 +122,11 @@ class TestFileFixture { ? ccContentPair.formatted : hContentPair.formatted, ); + case target.FormatCheck.dart: + return FileContentPair( + content, + dartContentPair.formatted, + ); case target.FormatCheck.gn: return FileContentPair( content, @@ -166,6 +177,22 @@ void main() { } }); + test('Can fix Dart formatting errors', () { + final TestFileFixture fixture = TestFileFixture(target.FormatCheck.dart); + try { + fixture.gitAdd(); + io.Process.runSync(formatterPath, ['--check', 'dart', '--fix'], + workingDirectory: repoDir.path); + + final Iterable files = fixture.getFileContents(); + for (final FileContentPair pair in files) { + expect(pair.original, equals(pair.formatted)); + } + } finally { + fixture.gitRemove(); + } + }); + test('Can fix GN formatting errors', () { final TestFileFixture fixture = TestFileFixture(target.FormatCheck.gn); try {