Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly report dart format errors #57206

Merged
merged 7 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 43 additions & 19 deletions ci/bin/format.dart
Original file line number Diff line number Diff line change
Expand Up @@ -834,9 +834,7 @@ class DartFormatChecker extends FormatChecker {
@override
Future<bool> fixFormatting() async {
message('Fixing Dart formatting...');
await _runDartFormat(fixing: true);
// The dart formatter shouldn't fail when fixing errors.
return true;
return (await _runDartFormat(fixing: true)) == 0;
}

Future<int> _runDartFormat({required bool fixing}) async {
Expand All @@ -860,25 +858,26 @@ class DartFormatChecker extends FormatChecker {
);

Iterable<WorkerJob> incorrect;
final List<WorkerJob> errorJobs = [];
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) {
if (completedJob.result.exitCode != 0 && (completedJob.result.stderr.isNotEmpty || completedJob.result.exitCode != 1)) {
// The formatter had a problem formatting the file.
errorJobs.add(completedJob);
} else 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),
),
WorkerJob(<String>[
'git',
'diff',
'--no-index',
'--no-color',
'--ignore-cr-at-eol',
'--',
completedJob.command.last,
'-',
], stdinRaw: codeUnitsAsStream(completedJob.result.stdoutRaw)),
);
}
}
Expand All @@ -892,7 +891,15 @@ class DartFormatChecker extends FormatChecker {
});
} else {
final List<WorkerJob> completedJobs = await dartFmt.runToCompletion(jobs);
incorrect = completedJobs.where((WorkerJob job) => job.result.exitCode == 1);
final List<WorkerJob> incorrectJobs = incorrect = [];
for (final WorkerJob job in completedJobs) {
if (job.result.exitCode != 0 && (job.result.stderr.isNotEmpty || job.result.exitCode != 1)) {
// The formatter had a problem formatting the file.
errorJobs.add(job);
} else if (job.result.exitCode == 1) {
incorrectJobs.add(job);
}
}
}

reportDone();
Expand All @@ -905,6 +912,7 @@ class DartFormatChecker extends FormatChecker {
} else {
error('Found ${incorrect.length} Dart file${plural ? 's' : ''}'
' which ${plural ? 'were' : 'was'} formatted incorrectly.');
stdout.writeln();
stdout.writeln('To fix, run `et format` or:');
stdout.writeln();
stdout.writeln('git apply <<DONE');
Expand All @@ -918,10 +926,26 @@ class DartFormatChecker extends FormatChecker {
stdout.writeln('DONE');
stdout.writeln();
}
_printErrorJobs(errorJobs);
} else if (errorJobs.isNotEmpty) {
_printErrorJobs(errorJobs);
} else {
message('All dart files formatted correctly.');
}
return incorrect.length;
return fixing ? errorJobs.length : (incorrect.length + errorJobs.length);
}

void _printErrorJobs(List<WorkerJob> errorJobs) {
if (errorJobs.isNotEmpty) {
final bool plural = errorJobs.length > 1;
error('The formatter failed to run on ${errorJobs.length} Dart file${plural ? 's' : ''}.');
stdout.writeln();
for (final WorkerJob job in errorJobs) {
stdout.writeln('--> ${job.command.last} produced the following error:');
stdout.write(job.result.stderr);
stdout.writeln();
}
}
}
}

Expand Down
19 changes: 19 additions & 0 deletions ci/test/format_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,25 @@ void main() {
}
});

test('Prints error if dart formatter fails', () {
final TestFileFixture fixture = TestFileFixture(target.FormatCheck.dart);
final io.File dartFile = io.File('${repoDir.path}/format_test2.dart');
dartFile.writeAsStringSync('P\n');
fixture.files.add(dartFile);

try {
fixture.gitAdd();
final io.ProcessResult result = io.Process.runSync(
formatterPath, <String>['--check', 'dart', '--fix'],
workingDirectory: repoDir.path,
);
expect(result.stdout, contains('format_test2.dart produced the following error'));
expect(result.exitCode, isNot(0));
} finally {
fixture.gitRemove();
}
});

test('Can fix GN formatting errors', () {
final TestFileFixture fixture = TestFileFixture(target.FormatCheck.gn);
try {
Expand Down
Loading