Skip to content

[ci] Add flags to formatter command to decide which formatters to run #5905

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 18, 2024
Merged

[ci] Add flags to formatter command to decide which formatters to run #5905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64cf5a5
[ci] Add flags to formatter command to decide which formatters to run
jmagman Jan 16, 2024
ca0792d
remove platform check
jmagman Jan 16, 2024
e937de9
flag tests
jmagman Jan 17, 2024
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
69 changes: 50 additions & 19 deletions script/tool/lib/src/format_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,29 @@ class FormatCommand extends PackageCommand {
super.platform,
}) {
argParser.addFlag('fail-on-change', hide: true);
argParser.addOption(_clangFormatArg,
argParser.addFlag(_dartArg, help: 'Format Dart files', defaultsTo: true);
argParser.addFlag(_clangFormatArg,
help: 'Format with "clang-format"', defaultsTo: true);
argParser.addFlag(_kotlinArg,
help: 'Format Kotlin files', defaultsTo: true);
argParser.addFlag(_javaArg, help: 'Format Java files', defaultsTo: true);
argParser.addFlag(_swiftArg, help: 'Format Swift files');
argParser.addOption(_clangFormatPathArg,
defaultsTo: 'clang-format', help: 'Path to "clang-format" executable.');
argParser.addOption(_javaArg,
argParser.addOption(_javaPathArg,
defaultsTo: 'java', help: 'Path to "java" executable.');
argParser.addOption(_swiftFormatArg,
help: 'Path to "swift-format" executable.');
argParser.addOption(_swiftFormatPathArg,
defaultsTo: 'swift-format', help: 'Path to "swift-format" executable.');
}

static const String _dartArg = 'dart';
static const String _clangFormatArg = 'clang-format';
static const String _kotlinArg = 'kotlin';
static const String _javaArg = 'java';
static const String _swiftFormatArg = 'swift-format';
static const String _swiftArg = 'swift';
static const String _clangFormatPathArg = 'clang-format-path';
static const String _javaPathArg = 'java-path';
static const String _swiftFormatPathArg = 'swift-format-path';

@override
final String name = 'format';
Expand All @@ -80,13 +92,20 @@ class FormatCommand extends PackageCommand {
// due to the startup overhead of the formatters.
final Iterable<String> files =
await _getFilteredFilePaths(getFiles(), relativeTo: packagesDir);
await _formatDart(files);
await _formatJava(files, javaFormatterPath);
await _formatKotlin(files, kotlinFormatterPath);
await _formatCppAndObjectiveC(files);
final String? swiftFormat = getNullableStringArg(_swiftFormatArg);
if (swiftFormat != null) {
await _formatSwift(swiftFormat, files);
if (getBoolArg(_dartArg)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add tests for these flags.

await _formatDart(files);
}
if (getBoolArg(_javaArg)) {
await _formatJava(files, javaFormatterPath);
}
if (getBoolArg(_kotlinArg)) {
await _formatKotlin(files, kotlinFormatterPath);
}
if (getBoolArg(_clangFormatArg)) {
await _formatCppAndObjectiveC(files);
}
if (getBoolArg(_swiftArg)) {
await _formatSwift(files);
}

if (getBoolArg('fail-on-change')) {
Expand Down Expand Up @@ -158,7 +177,8 @@ class FormatCommand extends PackageCommand {
}
}

Future<void> _formatSwift(String swiftFormat, Iterable<String> files) async {
Future<void> _formatSwift(Iterable<String> files) async {
final String swiftFormat = await _findValidSwiftFormat();
final Iterable<String> swiftFiles =
_getPathsWithExtensions(files, <String>{'.swift'});
if (swiftFiles.isNotEmpty) {
Expand All @@ -173,7 +193,7 @@ class FormatCommand extends PackageCommand {
}

Future<String> _findValidClangFormat() async {
final String clangFormat = getStringArg(_clangFormatArg);
final String clangFormat = getStringArg(_clangFormatPathArg);
if (await _hasDependency(clangFormat)) {
return clangFormat;
}
Expand All @@ -188,19 +208,30 @@ class FormatCommand extends PackageCommand {
}
}
printError('Unable to run "clang-format". Make sure that it is in your '
'path, or provide a full path with --clang-format.');
'path, or provide a full path with --$_clangFormatPathArg.');
throw ToolExit(_exitDependencyMissing);
}

Future<String> _findValidSwiftFormat() async {
final String swiftFormat = getStringArg(_swiftFormatPathArg);
if (await _hasDependency(swiftFormat)) {
return swiftFormat;
}

printError('Unable to run "swift-format". Make sure that it is in your '
'path, or provide a full path with --$_swiftFormatPathArg.');
throw ToolExit(_exitDependencyMissing);
}

Future<void> _formatJava(Iterable<String> files, String formatterPath) async {
final Iterable<String> javaFiles =
_getPathsWithExtensions(files, <String>{'.java'});
if (javaFiles.isNotEmpty) {
final String java = getStringArg('java');
final String java = getStringArg(_javaPathArg);
if (!await _hasDependency(java)) {
printError(
'Unable to run "java". Make sure that it is in your path, or '
'provide a full path with --java.');
'provide a full path with --$_javaPathArg.');
throw ToolExit(_exitDependencyMissing);
}

Expand All @@ -220,11 +251,11 @@ class FormatCommand extends PackageCommand {
final Iterable<String> kotlinFiles =
_getPathsWithExtensions(files, <String>{'.kt'});
if (kotlinFiles.isNotEmpty) {
final String java = getStringArg('java');
final String java = getStringArg(_javaPathArg);
if (!await _hasDependency(java)) {
printError(
'Unable to run "java". Make sure that it is in your path, or '
'provide a full path with --java.');
'provide a full path with --$_javaPathArg.');
throw ToolExit(_exitDependencyMissing);
}

Expand Down
103 changes: 90 additions & 13 deletions script/tool/test/format_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,16 @@ void main() {
]));
});

test('skips dart if --no-dart flag is provided', () async {
const List<String> files = <String>[
'lib/a.dart',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);

await runCapturingPrint(runner, <String>['format', '--no-dart']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
});

test('formats .java files', () async {
const List<String> files = <String>[
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
Expand Down Expand Up @@ -220,7 +230,7 @@ void main() {
containsAllInOrder(<Matcher>[
contains(
'Unable to run "java". Make sure that it is in your path, or '
'provide a full path with --java.'),
'provide a full path with --java-path.'),
]));
});

Expand Down Expand Up @@ -250,7 +260,7 @@ void main() {
]));
});

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

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

expect(
processRunner.recordedCalls,
Expand All @@ -279,6 +290,16 @@ void main() {
]));
});

test('skips Java if --no-java flag is provided', () async {
const List<String> files = <String>[
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);

await runCapturingPrint(runner, <String>['format', '--no-java']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
});

test('formats c-ish files', () async {
const List<String> files = <String>[
'ios/Classes/Foo.h',
Expand Down Expand Up @@ -332,7 +353,7 @@ void main() {
output,
containsAllInOrder(<Matcher>[
contains('Unable to run "clang-format". Make sure that it is in your '
'path, or provide a full path with --clang-format.'),
'path, or provide a full path with --clang-format-path.'),
]));
});

Expand Down Expand Up @@ -376,7 +397,7 @@ void main() {
]));
});

test('honors --clang-format flag', () async {
test('honors --clang-format-path flag', () async {
const List<String> files = <String>[
'windows/foo_plugin.cpp',
];
Expand All @@ -386,8 +407,8 @@ void main() {
extraFiles: files,
);

await runCapturingPrint(
runner, <String>['format', '--clang-format=/path/to/clang-format']);
await runCapturingPrint(runner,
<String>['format', '--clang-format-path=/path/to/clang-format']);

expect(
processRunner.recordedCalls,
Expand Down Expand Up @@ -433,6 +454,16 @@ void main() {
]));
});

test('skips clang-format if --no-clang-format flag is provided', () async {
const List<String> files = <String>[
'linux/foo_plugin.cc',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);

await runCapturingPrint(runner, <String>['format', '--no-clang-format']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
});

group('kotlin-format', () {
test('formats .kt files', () async {
const List<String> files = <String>[
Expand Down Expand Up @@ -487,6 +518,16 @@ void main() {
contains('Failed to format Kotlin files: exit code 1.'),
]));
});

test('skips Kotlin if --no-kotlin flag is provided', () async {
const List<String> files = <String>[
'android/src/main/kotlin/io/flutter/plugins/a_plugin/a.kt',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);

await runCapturingPrint(runner, <String>['format', '--no-kotlin']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
});
});

group('swift-format', () {
Expand All @@ -500,20 +541,25 @@ void main() {
extraFiles: files,
);

await runCapturingPrint(
runner, <String>['format', '--swift-format=/path/to/swift-format']);
await runCapturingPrint(runner, <String>[
'format',
'--swift',
'--swift-format-path=/path/to/swift-format'
]);

expect(
processRunner.recordedCalls,
orderedEquals(<ProcessCall>[
const ProcessCall(
'/path/to/swift-format', <String>['--version'], null),
ProcessCall(
'/path/to/swift-format',
<String>['-i', ...getPackagesDirRelativePaths(plugin, files)],
packagesDir.path),
]));
});

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

test('fails with a clear message if swift-format is not in the path',
() async {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
FakeProcessInfo(MockProcess(exitCode: 1), <String>['--version']),
];
Error? commandError;
final List<String> output = await runCapturingPrint(
runner, <String>['format', '--swift'], errorHandler: (Error e) {
commandError = e;
});

expect(commandError, isA<ToolExit>());
expect(
output,
containsAllInOrder(<Matcher>[
contains(
'Unable to run "swift-format". Make sure that it is in your path, or '
'provide a full path with --swift-format-path.'),
]));
});

test('fails if swift-format fails', () async {
const List<String> files = <String>[
'macos/foo.swift',
Expand All @@ -536,12 +609,16 @@ void main() {

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
FakeProcessInfo(MockProcess(),
<String>['--version']), // check for working swift-format
FakeProcessInfo(MockProcess(exitCode: 1), <String>['-i']),
];
Error? commandError;
final List<String> output = await runCapturingPrint(
runner, <String>['format', '--swift-format=swift-format'],
errorHandler: (Error e) {
final List<String> output = await runCapturingPrint(runner, <String>[
'format',
'--swift',
'--swift-format-path=swift-format'
], errorHandler: (Error e) {
commandError = e;
});

Expand Down