Skip to content

Commit e6cc4d4

Browse files
authored
Add support for dart formatter (flutter/engine#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`.
1 parent 53b543c commit e6cc4d4

File tree

2 files changed

+170
-3
lines changed

2 files changed

+170
-3
lines changed

engine/src/flutter/ci/bin/format.dart

Lines changed: 143 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ enum MessageType {
4040
}
4141

4242
enum FormatCheck {
43+
dart,
4344
gn,
4445
java,
4546
python,
@@ -53,6 +54,8 @@ FormatCheck nameToFormatCheck(String name) {
5354
switch (name) {
5455
case 'clang':
5556
return FormatCheck.clang;
57+
case 'dart':
58+
return FormatCheck.dart;
5659
case 'gn':
5760
return FormatCheck.gn;
5861
case 'java':
@@ -72,6 +75,8 @@ String formatCheckToName(FormatCheck check) {
7275
switch (check) {
7376
case FormatCheck.clang:
7477
return 'C++/ObjC/Shader';
78+
case FormatCheck.dart:
79+
return 'Dart';
7580
case FormatCheck.gn:
7681
return 'GN';
7782
case FormatCheck.java:
@@ -87,8 +92,7 @@ String formatCheckToName(FormatCheck check) {
8792

8893
List<String> formatCheckNames() {
8994
return FormatCheck.values
90-
.map<String>((FormatCheck check) =>
91-
check.toString().replaceFirst('$FormatCheck.', ''))
95+
.map<String>((FormatCheck check) => check.name)
9296
.toList();
9397
}
9498

@@ -141,6 +145,14 @@ abstract class FormatChecker {
141145
allFiles: allFiles,
142146
messageCallback: messageCallback,
143147
);
148+
case FormatCheck.dart:
149+
return DartFormatChecker(
150+
processManager: processManager,
151+
baseGitRef: baseGitRef,
152+
repoDir: repoDir,
153+
allFiles: allFiles,
154+
messageCallback: messageCallback,
155+
);
144156
case FormatCheck.gn:
145157
return GnFormatChecker(
146158
processManager: processManager,
@@ -786,6 +798,133 @@ class GnFormatChecker extends FormatChecker {
786798
}
787799
}
788800

801+
/// Checks the format of any .dart files using the "dart format" command.
802+
class DartFormatChecker extends FormatChecker {
803+
DartFormatChecker({
804+
super.processManager,
805+
required super.baseGitRef,
806+
required Directory repoDir,
807+
super.allFiles,
808+
super.messageCallback,
809+
}) : super(
810+
repoDir: repoDir,
811+
) {
812+
// $ENGINE/flutter/third_party/dart/tools/sdks/dart-sdk/bin/dart
813+
_dartBin = path.join(
814+
repoDir.absolute.parent.path,
815+
'flutter',
816+
'third_party',
817+
'dart',
818+
'tools',
819+
'sdks',
820+
'dart-sdk',
821+
'bin',
822+
Platform.isWindows ? 'dart.exe' : 'dart',
823+
);
824+
}
825+
826+
late final String _dartBin;
827+
828+
@override
829+
Future<bool> checkFormatting() async {
830+
message('Checking Dart formatting...');
831+
return (await _runDartFormat(fixing: false)) == 0;
832+
}
833+
834+
@override
835+
Future<bool> fixFormatting() async {
836+
message('Fixing Dart formatting...');
837+
await _runDartFormat(fixing: true);
838+
// The dart formatter shouldn't fail when fixing errors.
839+
return true;
840+
}
841+
842+
Future<int> _runDartFormat({required bool fixing}) async {
843+
final List<String> filesToCheck = await getFileList(<String>['*.dart']);
844+
845+
final List<String> cmd = <String>[
846+
_dartBin,
847+
'format',
848+
'--set-exit-if-changed',
849+
'--show=none',
850+
if (!fixing) '--output=show',
851+
if (fixing) '--output=write',
852+
];
853+
final List<WorkerJob> jobs = <WorkerJob>[];
854+
for (final String file in filesToCheck) {
855+
jobs.add(WorkerJob(<String>[...cmd, file]));
856+
}
857+
final ProcessPool dartFmt = ProcessPool(
858+
processRunner: _processRunner,
859+
printReport: namedReport('dart format'),
860+
);
861+
862+
Iterable<WorkerJob> incorrect;
863+
if (!fixing) {
864+
final Stream<WorkerJob> completedJobs = dartFmt.startWorkers(jobs);
865+
final List<WorkerJob> diffJobs = <WorkerJob>[];
866+
await for (final WorkerJob completedJob in completedJobs) {
867+
if (completedJob.result.exitCode == 1) {
868+
diffJobs.add(
869+
WorkerJob(
870+
<String>[
871+
'git',
872+
'diff',
873+
'--no-index',
874+
'--no-color',
875+
'--ignore-cr-at-eol',
876+
'--',
877+
completedJob.command.last,
878+
'-',
879+
],
880+
stdinRaw: codeUnitsAsStream(completedJob.result.stdoutRaw),
881+
),
882+
);
883+
}
884+
}
885+
final ProcessPool diffPool = ProcessPool(
886+
processRunner: _processRunner,
887+
printReport: namedReport('diff'),
888+
);
889+
final List<WorkerJob> completedDiffs = await diffPool.runToCompletion(diffJobs);
890+
incorrect = completedDiffs.where((WorkerJob job) {
891+
return job.result.exitCode != 0;
892+
});
893+
} else {
894+
final List<WorkerJob> completedJobs = await dartFmt.runToCompletion(jobs);
895+
incorrect = completedJobs.where((WorkerJob job) => job.result.exitCode == 1);
896+
}
897+
898+
reportDone();
899+
900+
if (incorrect.isNotEmpty) {
901+
final bool plural = incorrect.length > 1;
902+
if (fixing) {
903+
message('Fixing ${incorrect.length} dart file${plural ? 's' : ''}'
904+
' which ${plural ? 'were' : 'was'} formatted incorrectly.');
905+
} else {
906+
error('Found ${incorrect.length} Dart file${plural ? 's' : ''}'
907+
' which ${plural ? 'were' : 'was'} formatted incorrectly.');
908+
stdout.writeln('To fix, run `et format` or:');
909+
stdout.writeln();
910+
stdout.writeln('git apply <<DONE');
911+
for (final WorkerJob job in incorrect) {
912+
stdout.write(job.result.stdout
913+
.replaceFirst('b/-', 'b/${job.command[job.command.length - 2]}')
914+
.replaceFirst('b/-', 'b/${job.command[job.command.length - 2]}')
915+
.replaceFirst(RegExp('\\+Formatted \\d+ files? \\(\\d+ changed\\) in \\d+.\\d+ seconds.\n'), '')
916+
);
917+
}
918+
stdout.writeln('DONE');
919+
stdout.writeln();
920+
}
921+
} else {
922+
message('All dart files formatted correctly.');
923+
}
924+
return incorrect.length;
925+
}
926+
}
927+
789928
/// Checks the format of any .py files using the "yapf" command.
790929
class PythonFormatChecker extends FormatChecker {
791930
PythonFormatChecker({
@@ -1144,7 +1283,8 @@ Future<int> main(List<String> arguments) async {
11441283
parser.addMultiOption('check',
11451284
abbr: 'c',
11461285
allowed: formatCheckNames(),
1147-
defaultsTo: formatCheckNames(),
1286+
// TODO(goderbauer): Enable dart by default when we turned on the formatter.
1287+
defaultsTo: formatCheckNames()..remove(FormatCheck.dart.name),
11481288
help: 'Specifies which checks will be performed. Defaults to all checks. '
11491289
'May be specified more than once to perform multiple types of checks. ');
11501290
parser.addFlag('verbose', help: 'Print verbose output.', defaultsTo: verbose);

engine/src/flutter/ci/test/format_test.dart

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ final FileContentPair ccContentPair = FileContentPair(
2323
'int main(){return 0;}\n', 'int main() {\n return 0;\n}\n');
2424
final FileContentPair hContentPair =
2525
FileContentPair('int\nmain\n()\n;\n', 'int main();\n');
26+
final FileContentPair dartContentPair = FileContentPair(
27+
'enum \n\nfoo {\n entry1,\n entry2,\n}', 'enum foo { entry1, entry2 }\n');
2628
final FileContentPair gnContentPair = FileContentPair(
2729
'test\n(){testvar=true}\n', 'test() {\n testvar = true\n}\n');
2830
final FileContentPair javaContentPair = FileContentPair(
@@ -60,6 +62,10 @@ class TestFileFixture {
6062
final io.File hFile = io.File('${repoDir.path}/format_test.h');
6163
hFile.writeAsStringSync(hContentPair.original);
6264
files.add(hFile);
65+
case target.FormatCheck.dart:
66+
final io.File dartFile = io.File('${repoDir.path}/format_test.dart');
67+
dartFile.writeAsStringSync(dartContentPair.original);
68+
files.add(dartFile);
6369
case target.FormatCheck.gn:
6470
final io.File gnFile = io.File('${repoDir.path}/format_test.gn');
6571
gnFile.writeAsStringSync(gnContentPair.original);
@@ -116,6 +122,11 @@ class TestFileFixture {
116122
? ccContentPair.formatted
117123
: hContentPair.formatted,
118124
);
125+
case target.FormatCheck.dart:
126+
return FileContentPair(
127+
content,
128+
dartContentPair.formatted,
129+
);
119130
case target.FormatCheck.gn:
120131
return FileContentPair(
121132
content,
@@ -166,6 +177,22 @@ void main() {
166177
}
167178
});
168179

180+
test('Can fix Dart formatting errors', () {
181+
final TestFileFixture fixture = TestFileFixture(target.FormatCheck.dart);
182+
try {
183+
fixture.gitAdd();
184+
io.Process.runSync(formatterPath, <String>['--check', 'dart', '--fix'],
185+
workingDirectory: repoDir.path);
186+
187+
final Iterable<FileContentPair> files = fixture.getFileContents();
188+
for (final FileContentPair pair in files) {
189+
expect(pair.original, equals(pair.formatted));
190+
}
191+
} finally {
192+
fixture.gitRemove();
193+
}
194+
});
195+
169196
test('Can fix GN formatting errors', () {
170197
final TestFileFixture fixture = TestFileFixture(target.FormatCheck.gn);
171198
try {

0 commit comments

Comments
 (0)