Skip to content

Commit

Permalink
[tool] Add optional swift-format support (flutter#5204)
Browse files Browse the repository at this point in the history
Adds support for swift-format in the `format` command. For now this is optional, so is only triggered if the flag is explictly passed. In the future, once we have CI support for swift-format, we will likely want to make it on-by-default as with the other formatters.

Part of flutter/flutter#41129
  • Loading branch information
stuartmorgan authored Oct 23, 2023
1 parent 4bf5114 commit e73b364
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 6 deletions.
38 changes: 32 additions & 6 deletions script/tool/lib/src/format_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const int _exitFlutterFormatFailed = 4;
const int _exitJavaFormatFailed = 5;
const int _exitGitFailed = 6;
const int _exitDependencyMissing = 7;
const int _exitSwiftFormatFailed = 8;

final Uri _googleFormatterUrl = Uri.https('github.com',
'/google/google-java-format/releases/download/google-java-format-1.3/google-java-format-1.3-all-deps.jar');
Expand All @@ -44,18 +45,25 @@ class FormatCommand extends PackageCommand {
super.platform,
}) {
argParser.addFlag('fail-on-change', hide: true);
argParser.addOption('clang-format',
argParser.addOption(_clangFormatArg,
defaultsTo: 'clang-format', help: 'Path to "clang-format" executable.');
argParser.addOption('java',
argParser.addOption(_javaArg,
defaultsTo: 'java', help: 'Path to "java" executable.');
argParser.addOption(_swiftFormatArg,
help: 'Path to "swift-format" executable.');
}

static const String _clangFormatArg = 'clang-format';
static const String _javaArg = 'java';
static const String _swiftFormatArg = 'swift-format';

@override
final String name = 'format';

@override
final String description =
'Formats the code of all packages (Java, Objective-C, C++, and Dart).\n\n'
'Formats the code of all packages (Java, Objective-C, C++, Dart, and '
'optionally Swift).\n\n'
'This command requires "git", "flutter" and "clang-format" v5 to be in '
'your path.';

Expand All @@ -71,6 +79,10 @@ class FormatCommand extends PackageCommand {
await _formatDart(files);
await _formatJava(files, googleFormatterPath);
await _formatCppAndObjectiveC(files);
final String? swiftFormat = getNullableStringArg(_swiftFormatArg);
if (swiftFormat != null) {
await _formatSwift(swiftFormat, files);
}

if (getBoolArg('fail-on-change')) {
final bool modified = await _didModifyAnything();
Expand Down Expand Up @@ -141,10 +153,24 @@ class FormatCommand extends PackageCommand {
}
}

Future<void> _formatSwift(String swiftFormat, Iterable<String> files) async {
final Iterable<String> swiftFiles =
_getPathsWithExtensions(files, <String>{'.swift'});
if (swiftFiles.isNotEmpty) {
print('Formatting .swift files...');
final int exitCode =
await _runBatched(swiftFormat, <String>['-i'], files: swiftFiles);
if (exitCode != 0) {
printError('Failed to format Swift files: exit code $exitCode.');
throw ToolExit(_exitSwiftFormatFailed);
}
}
}

Future<String> _findValidClangFormat() async {
final String clangFormatArg = getStringArg('clang-format');
if (await _hasDependency(clangFormatArg)) {
return clangFormatArg;
final String clangFormat = getStringArg(_clangFormatArg);
if (await _hasDependency(clangFormat)) {
return clangFormat;
}

// There is a known issue where "chromium/depot_tools/clang-format"
Expand Down
65 changes: 65 additions & 0 deletions script/tool/test/format_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,71 @@ void main() {
]));
});

group('swift-format', () {
test('formats Swift if --swift-format flag is provided', () async {
const List<String> files = <String>[
'macos/foo.swift',
];
final RepositoryPackage plugin = createFakePlugin(
'a_plugin',
packagesDir,
extraFiles: files,
);

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

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

test('skips Swift if --swift-format flag is not provided', () async {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin(
'a_plugin',
packagesDir,
extraFiles: files,
);

await runCapturingPrint(runner, <String>['format']);

expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
});

test('fails if swift-format fails', () 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>['-i']),
];
Error? commandError;
final List<String> output = await runCapturingPrint(
runner, <String>['format', '--swift-format=swift-format'],
errorHandler: (Error e) {
commandError = e;
});

expect(commandError, isA<ToolExit>());
expect(
output,
containsAllInOrder(<Matcher>[
contains('Failed to format Swift files: exit code 1.'),
]));
});
});

test('skips known non-repo files', () async {
const List<String> skipFiles = <String>[
'/example/build/SomeFramework.framework/Headers/SomeFramework.h',
Expand Down

0 comments on commit e73b364

Please sign in to comment.