From a18f84e6759fbed65544abb2bf81a52bfa84504e Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 13 Dec 2024 15:17:19 -0800 Subject: [PATCH 1/7] ++ --- ci/bin/format.dart | 65 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index 5f4c94b3345b3..e298bc3c0c238 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -860,26 +860,32 @@ class DartFormatChecker extends FormatChecker { ); Iterable incorrect; + final List errorJobs = []; 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), - ), - ); + if (completedJob.result.stderr.isNotEmpty) { + // The formatter had a problem formatting the file. + errorJobs.add(completedJob); + } else { + 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( @@ -892,7 +898,17 @@ class DartFormatChecker extends FormatChecker { }); } else { final List completedJobs = await dartFmt.runToCompletion(jobs); - incorrect = completedJobs.where((WorkerJob job) => job.result.exitCode == 1); + final List incorrectList = incorrect = []; + for (final WorkerJob job in completedJobs) { + if (job.result.exitCode == 1) { + if (job.result.stderr.isNotEmpty) { + // The formatter had a problem formatting the file. + errorJobs.add(job); + } else { + incorrectList.add(job); + } + } + } } reportDone(); @@ -905,6 +921,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 < 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.write('${job.command[job.command.length - 2]} produced the following error:'); + stdout.write(job.result.stderr); + stdout.writeln(); + } + } + } } /// Checks the format of any .py files using the "yapf" command. From f1c2855e9c134f27b88c3600681a88fa9080f061 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 13 Dec 2024 15:37:59 -0800 Subject: [PATCH 2/7] ++ --- ci/bin/format.dart | 51 +++++++++++++++++----------------------- ci/test/format_test.dart | 18 ++++++++++++++ 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index e298bc3c0c238..03fd62ae499d3 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -865,27 +865,22 @@ class DartFormatChecker extends FormatChecker { final Stream completedJobs = dartFmt.startWorkers(jobs); final List diffJobs = []; await for (final WorkerJob completedJob in completedJobs) { - if (completedJob.result.exitCode == 1) { - if (completedJob.result.stderr.isNotEmpty) { - // The formatter had a problem formatting the file. - errorJobs.add(completedJob); - } else { - diffJobs.add( - WorkerJob( - [ - 'git', - 'diff', - '--no-index', - '--no-color', - '--ignore-cr-at-eol', - '--', - completedJob.command.last, - '-', - ], - stdinRaw: codeUnitsAsStream(completedJob.result.stdoutRaw), - ), - ); - } + if ((completedJob.result.exitCode == 1 && 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([ + 'git', + 'diff', + '--no-index', + '--no-color', + '--ignore-cr-at-eol', + '--', + completedJob.command.last, + '-', + ], stdinRaw: codeUnitsAsStream(completedJob.result.stdoutRaw)), + ); } } final ProcessPool diffPool = ProcessPool( @@ -900,13 +895,11 @@ class DartFormatChecker extends FormatChecker { final List completedJobs = await dartFmt.runToCompletion(jobs); final List incorrectList = incorrect = []; for (final WorkerJob job in completedJobs) { - if (job.result.exitCode == 1) { - if (job.result.stderr.isNotEmpty) { - // The formatter had a problem formatting the file. - errorJobs.add(job); - } else { - incorrectList.add(job); - } + if ((job.result.exitCode == 1 && 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) { + incorrectList.add(job); } } } @@ -950,7 +943,7 @@ class DartFormatChecker extends FormatChecker { error('The formatter failed to run on ${errorJobs.length} Dart file${plural ? 's' : ''}.'); stdout.writeln(); for (final WorkerJob job in errorJobs) { - stdout.write('${job.command[job.command.length - 2]} produced the following error:'); + stdout.writeln('--> ${job.command.last} produced the following error:'); stdout.write(job.result.stderr); stdout.writeln(); } diff --git a/ci/test/format_test.dart b/ci/test/format_test.dart index 361644044aa02..b6b55612cefc8 100644 --- a/ci/test/format_test.dart +++ b/ci/test/format_test.dart @@ -193,6 +193,24 @@ 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, ['--check', 'dart', '--fix'], + workingDirectory: repoDir.path, + ); + expect(result.stderr, contains('format_test2.dart produced the following error')); + } finally { + fixture.gitRemove(); + } + }); + test('Can fix GN formatting errors', () { final TestFileFixture fixture = TestFileFixture(target.FormatCheck.gn); try { From 8e304fb02e3f765f45e8a5a442aa51d831beb0f3 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 13 Dec 2024 15:40:24 -0800 Subject: [PATCH 3/7] ++ --- ci/bin/format.dart | 2 +- ci/test/format_test.dart | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index 03fd62ae499d3..19bc2ecb90528 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -934,7 +934,7 @@ class DartFormatChecker extends FormatChecker { } else { message('All dart files formatted correctly.'); } - return incorrect.length; + return incorrect.length + errorJobs.length; } void _printErrorJobs(List errorJobs) { diff --git a/ci/test/format_test.dart b/ci/test/format_test.dart index b6b55612cefc8..a63e129d43b18 100644 --- a/ci/test/format_test.dart +++ b/ci/test/format_test.dart @@ -205,7 +205,8 @@ void main() { formatterPath, ['--check', 'dart', '--fix'], workingDirectory: repoDir.path, ); - expect(result.stderr, contains('format_test2.dart produced the following error')); + expect(result.stdout, contains('format_test2.dart produced the following error')); + expect(result.exitCode, isNot(0)); } finally { fixture.gitRemove(); } From bf084290aa6a7fe83d615a45faeb67fa09ee98ce Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 13 Dec 2024 15:44:08 -0800 Subject: [PATCH 4/7] ++ --- ci/bin/format.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index 19bc2ecb90528..64687f2661947 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -834,9 +834,7 @@ class DartFormatChecker extends FormatChecker { @override Future 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 _runDartFormat({required bool fixing}) async { @@ -934,7 +932,7 @@ class DartFormatChecker extends FormatChecker { } else { message('All dart files formatted correctly.'); } - return incorrect.length + errorJobs.length; + return fixing ? errorJobs.length : (incorrect.length + errorJobs.length); } void _printErrorJobs(List errorJobs) { From 56ec4d84047fde54a7f4699ab04ed7b00717bf11 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 13 Dec 2024 15:55:48 -0800 Subject: [PATCH 5/7] better name --- ci/bin/format.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index 64687f2661947..fd040d6f282b9 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -891,13 +891,13 @@ class DartFormatChecker extends FormatChecker { }); } else { final List completedJobs = await dartFmt.runToCompletion(jobs); - final List incorrectList = incorrect = []; + final List incorrectJobs = incorrect = []; for (final WorkerJob job in completedJobs) { if ((job.result.exitCode == 1 && 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) { - incorrectList.add(job); + incorrectJobs.add(job); } } } From 10ba6985aa365b59d47c0f579a4830d285aee156 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Mon, 16 Dec 2024 09:30:48 -0800 Subject: [PATCH 6/7] adjust exit code handeling --- ci/bin/format.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index fd040d6f282b9..ac9a4e6102b9c 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -863,7 +863,7 @@ class DartFormatChecker extends FormatChecker { final Stream completedJobs = dartFmt.startWorkers(jobs); final List diffJobs = []; await for (final WorkerJob completedJob in completedJobs) { - if ((completedJob.result.exitCode == 1 && completedJob.result.stderr.isNotEmpty) || 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) { @@ -893,7 +893,7 @@ class DartFormatChecker extends FormatChecker { final List completedJobs = await dartFmt.runToCompletion(jobs); final List incorrectJobs = incorrect = []; for (final WorkerJob job in completedJobs) { - if ((job.result.exitCode == 1 && job.result.stderr.isNotEmpty) || job.result.exitCode > 1) { + 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) { From a7b84432202a109fdd694ae966e4bc4859cb21fd Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Mon, 16 Dec 2024 11:01:34 -0800 Subject: [PATCH 7/7] Remove stderr check - not reliable. --- ci/bin/format.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/bin/format.dart b/ci/bin/format.dart index ac9a4e6102b9c..fbe7b61938285 100644 --- a/ci/bin/format.dart +++ b/ci/bin/format.dart @@ -863,7 +863,7 @@ class DartFormatChecker extends FormatChecker { final Stream completedJobs = dartFmt.startWorkers(jobs); final List diffJobs = []; await for (final WorkerJob completedJob in completedJobs) { - if (completedJob.result.exitCode != 0 && (completedJob.result.stderr.isNotEmpty || completedJob.result.exitCode != 1)) { + if (completedJob.result.exitCode != 0 && completedJob.result.exitCode != 1) { // The formatter had a problem formatting the file. errorJobs.add(completedJob); } else if (completedJob.result.exitCode == 1) { @@ -893,7 +893,7 @@ class DartFormatChecker extends FormatChecker { final List completedJobs = await dartFmt.runToCompletion(jobs); final List incorrectJobs = incorrect = []; for (final WorkerJob job in completedJobs) { - if (job.result.exitCode != 0 && (job.result.stderr.isNotEmpty || job.result.exitCode != 1)) { + if (job.result.exitCode != 0 && job.result.exitCode != 1) { // The formatter had a problem formatting the file. errorJobs.add(job); } else if (job.result.exitCode == 1) {