Skip to content

Commit

Permalink
Add support for dart formatter (#57075)
Browse files Browse the repository at this point in the history 
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`.
  • Loading branch information
@goderbauer
goderbauer authored Dec 11, 2024
1 parent 5fb04a6 commit a8f7556
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 3 deletions.
146 changes: 143 additions & 3 deletions ci/bin/format.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ enum MessageType {
}

enum FormatCheck {
dart,
gn,
java,
python,
Expand All @@ -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':
Expand All @@ -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:
Expand All @@ -87,8 +92,7 @@ String formatCheckToName(FormatCheck check) {

List<String> formatCheckNames() {
return FormatCheck.values
.map<String>((FormatCheck check) =>
check.toString().replaceFirst('$FormatCheck.', ''))
.map<String>((FormatCheck check) => check.name)
.toList();
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<bool> checkFormatting() async {
message('Checking Dart formatting...');
return (await _runDartFormat(fixing: false)) == 0;
}

@override
Future<bool> fixFormatting() async {
message('Fixing Dart formatting...');
await _runDartFormat(fixing: true);
// The dart formatter shouldn't fail when fixing errors.
return true;
}

Future<int> _runDartFormat({required bool fixing}) async {
final List<String> filesToCheck = await getFileList(<String>['*.dart']);

final List<String> cmd = <String>[
_dartBin,
'format',
'--set-exit-if-changed',
'--show=none',
if (!fixing) '--output=show',
if (fixing) '--output=write',
];
final List<WorkerJob> jobs = <WorkerJob>[];
for (final String file in filesToCheck) {
jobs.add(WorkerJob(<String>[...cmd, file]));
}
final ProcessPool dartFmt = ProcessPool(
processRunner: _processRunner,
printReport: namedReport('dart format'),
);

Iterable<WorkerJob> incorrect;
if (!fixing) {
final Stream<WorkerJob> completedJobs = dartFmt.startWorkers(jobs);
final List<WorkerJob> diffJobs = <WorkerJob>[];
await for (final WorkerJob completedJob in completedJobs) {
if (completedJob.result.exitCode == 1) {
diffJobs.add(
WorkerJob(
<String>[
'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<WorkerJob> completedDiffs = await diffPool.runToCompletion(diffJobs);
incorrect = completedDiffs.where((WorkerJob job) {
return job.result.exitCode != 0;
});
} else {
final List<WorkerJob> 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 <<DONE');
for (final WorkerJob job in incorrect) {
stdout.write(job.result.stdout
.replaceFirst('b/-', 'b/${job.command[job.command.length - 2]}')
.replaceFirst('b/-', 'b/${job.command[job.command.length - 2]}')
.replaceFirst(RegExp('\\+Formatted \\d+ files? \\(\\d+ changed\\) in \\d+.\\d+ seconds.\n'), '')
);
}
stdout.writeln('DONE');
stdout.writeln();
}
} else {
message('All dart files formatted correctly.');
}
return incorrect.length;
}
}

/// Checks the format of any .py files using the "yapf" command.
class PythonFormatChecker extends FormatChecker {
PythonFormatChecker({
Expand Down Expand Up @@ -1144,7 +1283,8 @@ Future<int> main(List<String> 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);
Expand Down
27 changes: 27 additions & 0 deletions ci/test/format_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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, <String>['--check', 'dart', '--fix'],
workingDirectory: repoDir.path);

final Iterable<FileContentPair> 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 {
Expand Down

0 comments on commit a8f7556

Please sign in to comment.