Skip to content

Commit 8ab9b25

Browse files
jmagmanjmagman
authored
[ci] Add flags to formatter command to decide which formatters to run (flutter#5905)
Get ready for a world where `swift-format` is available on the `PATH` https://flutter-review.googlesource.com/c/recipes/+/54020 1. Add `format --clang-format --java --kotlin --swift --dart` flags to decide whether to run specific formatters, as opposed to using the `path`. Keep `swift-format` optional but default the others to run. This matches the current behavior on Linux. 2. Add `*-path` variants of each. This will allow us to run `format --swift --no-clang-format --no-java --no-kotlin --no-dart` on the macOS bot so it doesn't duplicate same `format` call run on Linux. Part of flutter#41129
1 parent ef8ccdb commit 8ab9b25

2 files changed

Lines changed: 140 additions & 32 deletions

File tree

‎script/tool/lib/src/format_command.dart‎

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,29 @@ class FormatCommand extends PackageCommand {
4848
super.platform,
4949
}) {
5050
argParser.addFlag('fail-on-change', hide: true);
51-
argParser.addOption(_clangFormatArg,
51+
argParser.addFlag(_dartArg, help: 'Format Dart files', defaultsTo: true);
52+
argParser.addFlag(_clangFormatArg,
53+
help: 'Format with "clang-format"', defaultsTo: true);
54+
argParser.addFlag(_kotlinArg,
55+
help: 'Format Kotlin files', defaultsTo: true);
56+
argParser.addFlag(_javaArg, help: 'Format Java files', defaultsTo: true);
57+
argParser.addFlag(_swiftArg, help: 'Format Swift files');
58+
argParser.addOption(_clangFormatPathArg,
5259
defaultsTo: 'clang-format', help: 'Path to "clang-format" executable.');
53-
argParser.addOption(_javaArg,
60+
argParser.addOption(_javaPathArg,
5461
defaultsTo: 'java', help: 'Path to "java" executable.');
55-
argParser.addOption(_swiftFormatArg,
56-
help: 'Path to "swift-format" executable.');
62+
argParser.addOption(_swiftFormatPathArg,
63+
defaultsTo: 'swift-format', help: 'Path to "swift-format" executable.');
5764
}
5865

66+
static const String _dartArg = 'dart';
5967
static const String _clangFormatArg = 'clang-format';
68+
static const String _kotlinArg = 'kotlin';
6069
static const String _javaArg = 'java';
61-
static const String _swiftFormatArg = 'swift-format';
70+
static const String _swiftArg = 'swift';
71+
static const String _clangFormatPathArg = 'clang-format-path';
72+
static const String _javaPathArg = 'java-path';
73+
static const String _swiftFormatPathArg = 'swift-format-path';
6274

6375
@override
6476
final String name = 'format';
@@ -80,13 +92,20 @@ class FormatCommand extends PackageCommand {
8092
// due to the startup overhead of the formatters.
8193
final Iterable<String> files =
8294
await _getFilteredFilePaths(getFiles(), relativeTo: packagesDir);
83-
await _formatDart(files);
84-
await _formatJava(files, javaFormatterPath);
85-
await _formatKotlin(files, kotlinFormatterPath);
86-
await _formatCppAndObjectiveC(files);
87-
final String? swiftFormat = getNullableStringArg(_swiftFormatArg);
88-
if (swiftFormat != null) {
89-
await _formatSwift(swiftFormat, files);
95+
if (getBoolArg(_dartArg)) {
96+
await _formatDart(files);
97+
}
98+
if (getBoolArg(_javaArg)) {
99+
await _formatJava(files, javaFormatterPath);
100+
}
101+
if (getBoolArg(_kotlinArg)) {
102+
await _formatKotlin(files, kotlinFormatterPath);
103+
}
104+
if (getBoolArg(_clangFormatArg)) {
105+
await _formatCppAndObjectiveC(files);
106+
}
107+
if (getBoolArg(_swiftArg)) {
108+
await _formatSwift(files);
90109
}
91110

92111
if (getBoolArg('fail-on-change')) {
@@ -158,7 +177,8 @@ class FormatCommand extends PackageCommand {
158177
}
159178
}
160179

161-
Future<void> _formatSwift(String swiftFormat, Iterable<String> files) async {
180+
Future<void> _formatSwift(Iterable<String> files) async {
181+
final String swiftFormat = await _findValidSwiftFormat();
162182
final Iterable<String> swiftFiles =
163183
_getPathsWithExtensions(files, <String>{'.swift'});
164184
if (swiftFiles.isNotEmpty) {
@@ -173,7 +193,7 @@ class FormatCommand extends PackageCommand {
173193
}
174194

175195
Future<String> _findValidClangFormat() async {
176-
final String clangFormat = getStringArg(_clangFormatArg);
196+
final String clangFormat = getStringArg(_clangFormatPathArg);
177197
if (await _hasDependency(clangFormat)) {
178198
return clangFormat;
179199
}
@@ -188,19 +208,30 @@ class FormatCommand extends PackageCommand {
188208
}
189209
}
190210
printError('Unable to run "clang-format". Make sure that it is in your '
191-
'path, or provide a full path with --clang-format.');
211+
'path, or provide a full path with --$_clangFormatPathArg.');
212+
throw ToolExit(_exitDependencyMissing);
213+
}
214+
215+
Future<String> _findValidSwiftFormat() async {
216+
final String swiftFormat = getStringArg(_swiftFormatPathArg);
217+
if (await _hasDependency(swiftFormat)) {
218+
return swiftFormat;
219+
}
220+
221+
printError('Unable to run "swift-format". Make sure that it is in your '
222+
'path, or provide a full path with --$_swiftFormatPathArg.');
192223
throw ToolExit(_exitDependencyMissing);
193224
}
194225

195226
Future<void> _formatJava(Iterable<String> files, String formatterPath) async {
196227
final Iterable<String> javaFiles =
197228
_getPathsWithExtensions(files, <String>{'.java'});
198229
if (javaFiles.isNotEmpty) {
199-
final String java = getStringArg('java');
230+
final String java = getStringArg(_javaPathArg);
200231
if (!await _hasDependency(java)) {
201232
printError(
202233
'Unable to run "java". Make sure that it is in your path, or '
203-
'provide a full path with --java.');
234+
'provide a full path with --$_javaPathArg.');
204235
throw ToolExit(_exitDependencyMissing);
205236
}
206237

@@ -220,11 +251,11 @@ class FormatCommand extends PackageCommand {
220251
final Iterable<String> kotlinFiles =
221252
_getPathsWithExtensions(files, <String>{'.kt'});
222253
if (kotlinFiles.isNotEmpty) {
223-
final String java = getStringArg('java');
254+
final String java = getStringArg(_javaPathArg);
224255
if (!await _hasDependency(java)) {
225256
printError(
226257
'Unable to run "java". Make sure that it is in your path, or '
227-
'provide a full path with --java.');
258+
'provide a full path with --$_javaPathArg.');
228259
throw ToolExit(_exitDependencyMissing);
229260
}
230261

‎script/tool/test/format_command_test.dart‎

Lines changed: 90 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,16 @@ void main() {
169169
]));
170170
});
171171

172+
test('skips dart if --no-dart flag is provided', () async {
173+
const List<String> files = <String>[
174+
'lib/a.dart',
175+
];
176+
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
177+
178+
await runCapturingPrint(runner, <String>['format', '--no-dart']);
179+
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
180+
});
181+
172182
test('formats .java files', () async {
173183
const List<String> files = <String>[
174184
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
@@ -220,7 +230,7 @@ void main() {
220230
containsAllInOrder(<Matcher>[
221231
contains(
222232
'Unable to run "java". Make sure that it is in your path, or '
223-
'provide a full path with --java.'),
233+
'provide a full path with --java-path.'),
224234
]));
225235
});
226236

@@ -250,7 +260,7 @@ void main() {
250260
]));
251261
});
252262

253-
test('honors --java flag', () async {
263+
test('honors --java-path flag', () async {
254264
const List<String> files = <String>[
255265
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
256266
'android/src/main/java/io/flutter/plugins/a_plugin/b.java',
@@ -261,7 +271,8 @@ void main() {
261271
extraFiles: files,
262272
);
263273

264-
await runCapturingPrint(runner, <String>['format', '--java=/path/to/java']);
274+
await runCapturingPrint(
275+
runner, <String>['format', '--java-path=/path/to/java']);
265276

266277
expect(
267278
processRunner.recordedCalls,
@@ -279,6 +290,16 @@ void main() {
279290
]));
280291
});
281292

293+
test('skips Java if --no-java flag is provided', () async {
294+
const List<String> files = <String>[
295+
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
296+
];
297+
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
298+
299+
await runCapturingPrint(runner, <String>['format', '--no-java']);
300+
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
301+
});
302+
282303
test('formats c-ish files', () async {
283304
const List<String> files = <String>[
284305
'ios/Classes/Foo.h',
@@ -332,7 +353,7 @@ void main() {
332353
output,
333354
containsAllInOrder(<Matcher>[
334355
contains('Unable to run "clang-format". Make sure that it is in your '
335-
'path, or provide a full path with --clang-format.'),
356+
'path, or provide a full path with --clang-format-path.'),
336357
]));
337358
});
338359

@@ -376,7 +397,7 @@ void main() {
376397
]));
377398
});
378399

379-
test('honors --clang-format flag', () async {
400+
test('honors --clang-format-path flag', () async {
380401
const List<String> files = <String>[
381402
'windows/foo_plugin.cpp',
382403
];
@@ -386,8 +407,8 @@ void main() {
386407
extraFiles: files,
387408
);
388409

389-
await runCapturingPrint(
390-
runner, <String>['format', '--clang-format=/path/to/clang-format']);
410+
await runCapturingPrint(runner,
411+
<String>['format', '--clang-format-path=/path/to/clang-format']);
391412

392413
expect(
393414
processRunner.recordedCalls,
@@ -433,6 +454,16 @@ void main() {
433454
]));
434455
});
435456

457+
test('skips clang-format if --no-clang-format flag is provided', () async {
458+
const List<String> files = <String>[
459+
'linux/foo_plugin.cc',
460+
];
461+
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
462+
463+
await runCapturingPrint(runner, <String>['format', '--no-clang-format']);
464+
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
465+
});
466+
436467
group('kotlin-format', () {
437468
test('formats .kt files', () async {
438469
const List<String> files = <String>[
@@ -487,6 +518,16 @@ void main() {
487518
contains('Failed to format Kotlin files: exit code 1.'),
488519
]));
489520
});
521+
522+
test('skips Kotlin if --no-kotlin flag is provided', () async {
523+
const List<String> files = <String>[
524+
'android/src/main/kotlin/io/flutter/plugins/a_plugin/a.kt',
525+
];
526+
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
527+
528+
await runCapturingPrint(runner, <String>['format', '--no-kotlin']);
529+
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
530+
});
490531
});
491532

492533
group('swift-format', () {
@@ -500,20 +541,25 @@ void main() {
500541
extraFiles: files,
501542
);
502543

503-
await runCapturingPrint(
504-
runner, <String>['format', '--swift-format=/path/to/swift-format']);
544+
await runCapturingPrint(runner, <String>[
545+
'format',
546+
'--swift',
547+
'--swift-format-path=/path/to/swift-format'
548+
]);
505549

506550
expect(
507551
processRunner.recordedCalls,
508552
orderedEquals(<ProcessCall>[
553+
const ProcessCall(
554+
'/path/to/swift-format', <String>['--version'], null),
509555
ProcessCall(
510556
'/path/to/swift-format',
511557
<String>['-i', ...getPackagesDirRelativePaths(plugin, files)],
512558
packagesDir.path),
513559
]));
514560
});
515561

516-
test('skips Swift if --swift-format flag is not provided', () async {
562+
test('skips Swift if --swift flag is not provided', () async {
517563
const List<String> files = <String>[
518564
'macos/foo.swift',
519565
];
@@ -528,6 +574,33 @@ void main() {
528574
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
529575
});
530576

577+
test('fails with a clear message if swift-format is not in the path',
578+
() async {
579+
const List<String> files = <String>[
580+
'macos/foo.swift',
581+
];
582+
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
583+
584+
processRunner.mockProcessesForExecutable['swift-format'] =
585+
<FakeProcessInfo>[
586+
FakeProcessInfo(MockProcess(exitCode: 1), <String>['--version']),
587+
];
588+
Error? commandError;
589+
final List<String> output = await runCapturingPrint(
590+
runner, <String>['format', '--swift'], errorHandler: (Error e) {
591+
commandError = e;
592+
});
593+
594+
expect(commandError, isA<ToolExit>());
595+
expect(
596+
output,
597+
containsAllInOrder(<Matcher>[
598+
contains(
599+
'Unable to run "swift-format". Make sure that it is in your path, or '
600+
'provide a full path with --swift-format-path.'),
601+
]));
602+
});
603+
531604
test('fails if swift-format fails', () async {
532605
const List<String> files = <String>[
533606
'macos/foo.swift',
@@ -536,12 +609,16 @@ void main() {
536609

537610
processRunner.mockProcessesForExecutable['swift-format'] =
538611
<FakeProcessInfo>[
612+
FakeProcessInfo(MockProcess(),
613+
<String>['--version']), // check for working swift-format
539614
FakeProcessInfo(MockProcess(exitCode: 1), <String>['-i']),
540615
];
541616
Error? commandError;
542-
final List<String> output = await runCapturingPrint(
543-
runner, <String>['format', '--swift-format=swift-format'],
544-
errorHandler: (Error e) {
617+
final List<String> output = await runCapturingPrint(runner, <String>[
618+
'format',
619+
'--swift',
620+
'--swift-format-path=swift-format'
621+
], errorHandler: (Error e) {
545622
commandError = e;
546623
});
547624

0 commit comments

Comments
 (0)