Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[flutter_plugin_tools] Improve and test 'format' #4145

Merged
merged 1 commit into from
Jul 8, 2021
Merged

[flutter_plugin_tools] Improve and test 'format' #4145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
141 changes: 96 additions & 45 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 @@ -14,6 +14,11 @@ import 'common/core.dart';
import 'common/plugin_command.dart';
import 'common/process_runner.dart';

const int _exitClangFormatFailed = 3;
const int _exitFlutterFormatFailed = 4;
const int _exitJavaFormatFailed = 5;
const int _exitGitFailed = 6;

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 Down Expand Up @@ -43,14 +48,18 @@ class FormatCommand extends PluginCommand {
Future<void> run() async {
final String googleFormatterPath = await _getGoogleFormatterPath();

await _formatDart();
await _formatJava(googleFormatterPath);
await _formatCppAndObjectiveC();
// This class is not based on PackageLoopingCommand because running the
// formatters separately for each package is an order of magnitude slower,
// due to the startup overhead of the formatters.
final Iterable<String> files = await _getFilteredFilePaths(getFiles());
await _formatDart(files);
await _formatJava(files, googleFormatterPath);
await _formatCppAndObjectiveC(files);

if (getBoolArg('fail-on-change')) {
final bool modified = await _didModifyAnything();
if (modified) {
throw ToolExit(1);
throw ToolExit(exitCommandFoundErrors);
}
}
}
Expand All @@ -60,9 +69,12 @@ class FormatCommand extends PluginCommand {
'git',
<String>['ls-files', '--modified'],
workingDir: packagesDir,
exitOnError: true,
logOnError: true,
);
if (modifiedFiles.exitCode != 0) {
printError('Unable to determine changed files.');
throw ToolExit(_exitGitFailed);
}

print('\n\n');

Expand All @@ -79,66 +91,105 @@ class FormatCommand extends PluginCommand {
'pub global run flutter_plugin_tools format" or copy-paste '
'this command into your terminal:');

print('patch -p1 <<DONE');
final io.ProcessResult diff = await processRunner.run(
'git',
<String>['diff'],
workingDir: packagesDir,
exitOnError: true,
logOnError: true,
);
if (diff.exitCode != 0) {
printError('Unable to determine diff.');
throw ToolExit(_exitGitFailed);
}
print('patch -p1 <<DONE');
print(diff.stdout);
print('DONE');
return true;
}

Future<void> _formatCppAndObjectiveC() async {
print('Formatting all .cc, .cpp, .mm, .m, and .h files...');
final Iterable<String> allFiles = <String>[
...await _getFilesWithExtension('.h'),
...await _getFilesWithExtension('.m'),
...await _getFilesWithExtension('.mm'),
...await _getFilesWithExtension('.cc'),
...await _getFilesWithExtension('.cpp'),
];
// Split this into multiple invocations to avoid a
// 'ProcessException: Argument list too long'.
final Iterable<List<String>> batches = partition(allFiles, 100);
for (final List<String> batch in batches) {
await processRunner.runAndStream(getStringArg('clang-format'),
<String>['-i', '--style=Google', ...batch],
workingDir: packagesDir, exitOnError: true);
Future<void> _formatCppAndObjectiveC(Iterable<String> files) async {
final Iterable<String> clangFiles = _getPathsWithExtensions(
files, <String>{'.h', '.m', '.mm', '.cc', '.cpp'});
if (clangFiles.isNotEmpty) {
print('Formatting .cc, .cpp, .h, .m, and .mm files...');
final Iterable<List<String>> batches = partition(clangFiles, 100);
int exitCode = 0;
for (final List<String> batch in batches) {
batch.sort(); // For ease of testing; partition changes the order.
exitCode = await processRunner.runAndStream(
getStringArg('clang-format'),
<String>['-i', '--style=Google', ...batch],
workingDir: packagesDir);
if (exitCode != 0) {
break;
}
}
if (exitCode != 0) {
printError(
'Failed to format C, C++, and Objective-C files: exit code $exitCode.');
throw ToolExit(_exitClangFormatFailed);
}
}
}

Future<void> _formatJava(String googleFormatterPath) async {
print('Formatting all .java files...');
final Iterable<String> javaFiles = await _getFilesWithExtension('.java');
await processRunner.runAndStream('java',
<String>['-jar', googleFormatterPath, '--replace', ...javaFiles],
workingDir: packagesDir, exitOnError: true);
Future<void> _formatJava(
Iterable<String> files, String googleFormatterPath) async {
final Iterable<String> javaFiles =
_getPathsWithExtensions(files, <String>{'.java'});
if (javaFiles.isNotEmpty) {
print('Formatting .java files...');
final int exitCode = await processRunner.runAndStream('java',
<String>['-jar', googleFormatterPath, '--replace', ...javaFiles],
workingDir: packagesDir);
if (exitCode != 0) {
printError('Failed to format Java files: exit code $exitCode.');
throw ToolExit(_exitJavaFormatFailed);
}
}
}

Future<void> _formatDart() async {
// This actually should be fine for non-Flutter Dart projects, no need to
// specifically shell out to dartfmt -w in that case.
print('Formatting all .dart files...');
final Iterable<String> dartFiles = await _getFilesWithExtension('.dart');
if (dartFiles.isEmpty) {
print(
'No .dart files to format. If you set the `--exclude` flag, most likey they were skipped');
} else {
await processRunner.runAndStream(
Future<void> _formatDart(Iterable<String> files) async {
final Iterable<String> dartFiles =
_getPathsWithExtensions(files, <String>{'.dart'});
if (dartFiles.isNotEmpty) {
print('Formatting .dart files...');
// `flutter format` doesn't require the project to actually be a Flutter
// project.
final int exitCode = await processRunner.runAndStream(
'flutter', <String>['format', ...dartFiles],
workingDir: packagesDir, exitOnError: true);
workingDir: packagesDir);
if (exitCode != 0) {
printError('Failed to format Dart files: exit code $exitCode.');
throw ToolExit(_exitFlutterFormatFailed);
}
}
}

Future<List<String>> _getFilesWithExtension(String extension) async =>
getFiles()
.where((File file) => p.extension(file.path) == extension)
.map((File file) => file.path)
.toList();
Future<Iterable<String>> _getFilteredFilePaths(Stream<File> files) async {
// Returns a pattern to check for [directories] as a subset of a file path.
RegExp pathFragmentForDirectories(List<String> directories) {
final String s = p.separator;
return RegExp('(?:^|$s)${p.joinAll(directories)}$s');
}

return files
.map((File file) => file.path)
.where((String path) =>
// Ignore files in build/ directories (e.g., headers of frameworks)
// to avoid useless extra work in local repositories.
!path.contains(
pathFragmentForDirectories(<String>['example', 'build'])) &&
Comment on lines +180 to +181
Copy link
Member

Choose a reason for hiding this comment

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

Can these be implemented with some util from the path library? maybe isWithin? (probably not, it looks like it expects absolute paths)

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g Jul 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I looked a bit before writing this, and couldn't find anything that did arbitrary relate path containment.

// Ignore files in Pods, which are not part of the repository.
!path.contains(pathFragmentForDirectories(<String>['Pods'])) &&
// Ignore .dart_tool/, which can have various intermediate files.
!path.contains(pathFragmentForDirectories(<String>['.dart_tool'])))
.toList();
}

Iterable<String> _getPathsWithExtensions(
Iterable<String> files, Set<String> extensions) {
return files.where((String path) => extensions.contains(p.extension(path)));
}

Future<String> _getGoogleFormatterPath() async {
final String javaFormatterPath = p.join(
Expand Down
Loading