Skip to content

[tools] Run pub get before format #8052

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
stuartmorgan-g merged 3 commits into from
Nov 11, 2024
Merged

[tools] Run pub get before format #8052

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5716dac
Ensure that pub get is run in the format command
stuartmorgan-g Nov 11, 2024
080846f
Fetch before formatting
stuartmorgan-g Nov 11, 2024
3e5ccef
Ignore web generated plugin registrant
stuartmorgan-g Nov 11, 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
Next Next commit
Ensure that pub get is run in the format command
  • Loading branch information
@stuartmorgan-g
stuartmorgan-g committed Nov 11, 2024
commit 5716dac49b7e9b8751cf162ae6d7b8ff15efc8c7
10 changes: 10 additions & 0 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 @@ -12,6 +12,7 @@ import 'package:meta/meta.dart';
import 'common/core.dart';
import 'common/output_utils.dart';
import 'common/package_looping_command.dart';
import 'common/pub_utils.dart';
import 'common/repository_package.dart';

/// In theory this should be 8191, but in practice that was still resulting in
Expand Down Expand Up @@ -120,6 +121,15 @@ class FormatCommand extends PackageLoopingCommand {
relativeTo: package.directory,
);
if (getBoolArg(_dartArg)) {
// Ensure that .dart_tool exists, since without it `dart` doesn't know
// the lanugage version, so the formatter may give different output.
if (!package.directory.childDirectory('.dart_tool').existsSync()) {
Comment on lines +124 to +126
Copy link
Member

@ditman ditman Nov 11, 2024

Choose a reason for hiding this comment

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

Should we warn through the console in case this step needs to be performed?

That would let us know if something like flutter clean gets added by mistake?

Copy link
Collaborator Author

@stuartmorgan-g stuartmorgan-g Nov 11, 2024

Choose a reason for hiding this comment

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

We pass stdout/stderr through in runPubGet, so it's super obvious in the console when this is happening.

if (!await runPubGet(package, processRunner, super.platform)) {
printError('Unable to fetch dependencies.');
return PackageResult.fail(<String>['unable to fetch dependencies']);
}
}

await _formatDart(files, workingDir: package.directory);
}
// Success or failure is determined overall in completeRun, since most code
Expand Down
89 changes: 71 additions & 18 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 @@ -50,6 +50,12 @@ void main() {
runner.addCommand(analyzeCommand);
});

/// Creates the .dart_tool directory for [package] to simulate (as much as
/// this command requires) `pub get` having been run.
void fakePubGet(RepositoryPackage package) {
package.directory.childDirectory('.dart_tool').createSync();
}

/// Returns a modified version of a list of [relativePaths] that are relative
/// to [package] to instead be relative to [packagesDir].
List<String> getPackagesDirRelativePaths(
Expand Down Expand Up @@ -92,6 +98,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

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

Expand All @@ -117,6 +124,7 @@ void main() {
unformattedFile,
],
);
fakePubGet(plugin);

final p.Context posixContext = p.posix;
childFileWithSubcomponents(
Expand All @@ -140,7 +148,9 @@ void main() {
'lib/src/b.dart',
'lib/src/c.dart',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['dart'] = <FakeProcessInfo>[
FakeProcessInfo(MockProcess(exitCode: 1), <String>['format'])
Expand All @@ -163,7 +173,9 @@ void main() {
const List<String> files = <String>[
'lib/a.dart',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-dart']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -179,6 +191,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

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

Expand All @@ -203,7 +216,9 @@ void main() {
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
'android/src/main/java/io/flutter/plugins/a_plugin/b.java',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['java'] = <FakeProcessInfo>[
FakeProcessInfo(MockProcess(exitCode: 1), <String>['-version'])
Expand All @@ -229,7 +244,9 @@ void main() {
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
'android/src/main/java/io/flutter/plugins/a_plugin/b.java',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['java'] = <FakeProcessInfo>[
FakeProcessInfo(
Expand Down Expand Up @@ -260,6 +277,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(
runner, <String>['format', '--java-path=/path/to/java']);
Expand All @@ -284,7 +302,9 @@ void main() {
const List<String> files = <String>[
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-java']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -304,6 +324,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

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

Expand All @@ -328,7 +349,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['clang-format'] =
<FakeProcessInfo>[FakeProcessInfo(MockProcess(exitCode: 1))];
Expand Down Expand Up @@ -357,6 +380,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['clang-format'] =
<FakeProcessInfo>[FakeProcessInfo(MockProcess(exitCode: 1))];
Expand Down Expand Up @@ -396,6 +420,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner,
<String>['format', '--clang-format-path=/path/to/clang-format']);
Expand All @@ -421,7 +446,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['clang-format'] =
<FakeProcessInfo>[
Expand All @@ -448,7 +475,9 @@ void main() {
const List<String> files = <String>[
'linux/foo_plugin.cc',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-clang-format']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -465,6 +494,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

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

Expand All @@ -488,7 +518,9 @@ void main() {
'android/src/main/kotlin/io/flutter/plugins/a_plugin/a.kt',
'android/src/main/kotlin/io/flutter/plugins/a_plugin/b.kt',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['java'] = <FakeProcessInfo>[
FakeProcessInfo(
Expand All @@ -513,7 +545,9 @@ void main() {
const List<String> files = <String>[
'android/src/main/kotlin/io/flutter/plugins/a_plugin/a.kt',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-kotlin']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -530,6 +564,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>[
'format',
Expand Down Expand Up @@ -567,11 +602,12 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin(
final RepositoryPackage plugin = createFakePlugin(
'a_plugin',
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

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

Expand All @@ -583,7 +619,9 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
Expand All @@ -609,7 +647,9 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
Expand Down Expand Up @@ -643,7 +683,9 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
Expand Down Expand Up @@ -694,6 +736,7 @@ void main() {
...javaFiles,
],
);
fakePubGet(plugin);

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

Expand Down Expand Up @@ -732,6 +775,7 @@ void main() {
'example/macos/Flutter/GeneratedPluginRegistrant.swift',
],
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>[
'format',
Expand Down Expand Up @@ -773,7 +817,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

const String changedFilePath = 'packages/a_plugin/linux/foo_plugin.cc';
processRunner.mockProcessesForExecutable['git'] = <FakeProcessInfo>[
Expand Down Expand Up @@ -825,7 +871,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['git'] = <FakeProcessInfo>[
FakeProcessInfo(MockProcess(exitCode: 1), <String>['ls-files'])
Expand All @@ -850,7 +898,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

const String changedFilePath = 'packages/a_plugin/linux/foo_plugin.cc';
processRunner.mockProcessesForExecutable['git'] = <FakeProcessInfo>[
Expand Down Expand Up @@ -892,6 +942,7 @@ void main() {
packagesDir,
extraFiles: <String>[...batch1, extraFile],
);
fakePubGet(package);

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

Expand Down Expand Up @@ -922,11 +973,12 @@ void main() {
// Make the file list one file longer than would fit in a Windows batch.
final List<String> batch = get99CharacterPathExtraFiles(batchSize + 1);

createFakePlugin(
final RepositoryPackage plugin = createFakePlugin(
pluginName,
packagesDir,
extraFiles: batch,
);
fakePubGet(plugin);

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

Expand All @@ -947,6 +999,7 @@ void main() {
packagesDir,
extraFiles: <String>[...batch1, extraFile],
);
fakePubGet(package);

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

Expand Down