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

Commit 996a4a9

Browse files
stuartmorgan-gstuartmorgan-g
authored
[flutter_plugin_tools] Simplify filesystem usage (#4014)
- Replaces most explicit use of `fileSystem` with path construction using the `child*` utility methods - Removes explicit passing of a filesystem to the commands; we're already passing a `Directory` for the root where the tool operates, and we should never be using a different filesystem than that directory's filesystem, so passing it was both redundant, and a potential source of test bugs.
1 parent 8ac6286 commit 996a4a9

34 files changed

+177
-231
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,9 @@ import 'common.dart';
1313
class AnalyzeCommand extends PluginCommand {
1414
/// Creates a analysis command instance.
1515
AnalyzeCommand(
16-
Directory packagesDir,
17-
FileSystem fileSystem, {
16+
Directory packagesDir, {
1817
ProcessRunner processRunner = const ProcessRunner(),
19-
}) : super(packagesDir, fileSystem, processRunner: processRunner) {
18+
}) : super(packagesDir, processRunner: processRunner) {
2019
argParser.addMultiOption(_customAnalysisFlag,
2120
help:
2221
'Directories (comma separated) that are allowed to have their own analysis options.',

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,9 @@ import 'common.dart';
1515
class BuildExamplesCommand extends PluginCommand {
1616
/// Creates an instance of the build command.
1717
BuildExamplesCommand(
18-
Directory packagesDir,
19-
FileSystem fileSystem, {
18+
Directory packagesDir, {
2019
ProcessRunner processRunner = const ProcessRunner(),
21-
}) : super(packagesDir, fileSystem, processRunner: processRunner) {
20+
}) : super(packagesDir, processRunner: processRunner) {
2221
argParser.addFlag(kLinux, defaultsTo: false);
2322
argParser.addFlag(kMacos, defaultsTo: false);
2423
argParser.addFlag(kWeb, defaultsTo: false);
@@ -69,7 +68,7 @@ class BuildExamplesCommand extends PluginCommand {
6968

7069
if (getBoolArg(kLinux)) {
7170
print('\nBUILDING Linux for $packageName');
72-
if (isLinuxPlugin(plugin, fileSystem)) {
71+
if (isLinuxPlugin(plugin)) {
7372
final int buildExitCode = await processRunner.runAndStream(
7473
flutterCommand,
7574
<String>[
@@ -89,7 +88,7 @@ class BuildExamplesCommand extends PluginCommand {
8988

9089
if (getBoolArg(kMacos)) {
9190
print('\nBUILDING macOS for $packageName');
92-
if (isMacOsPlugin(plugin, fileSystem)) {
91+
if (isMacOsPlugin(plugin)) {
9392
final int exitCode = await processRunner.runAndStream(
9493
flutterCommand,
9594
<String>[
@@ -109,7 +108,7 @@ class BuildExamplesCommand extends PluginCommand {
109108

110109
if (getBoolArg(kWeb)) {
111110
print('\nBUILDING web for $packageName');
112-
if (isWebPlugin(plugin, fileSystem)) {
111+
if (isWebPlugin(plugin)) {
113112
final int buildExitCode = await processRunner.runAndStream(
114113
flutterCommand,
115114
<String>[
@@ -129,7 +128,7 @@ class BuildExamplesCommand extends PluginCommand {
129128

130129
if (getBoolArg(kWindows)) {
131130
print('\nBUILDING Windows for $packageName');
132-
if (isWindowsPlugin(plugin, fileSystem)) {
131+
if (isWindowsPlugin(plugin)) {
133132
final int buildExitCode = await processRunner.runAndStream(
134133
flutterCommand,
135134
<String>[
@@ -149,7 +148,7 @@ class BuildExamplesCommand extends PluginCommand {
149148

150149
if (getBoolArg(kIpa)) {
151150
print('\nBUILDING IPA for $packageName');
152-
if (isIosPlugin(plugin, fileSystem)) {
151+
if (isIosPlugin(plugin)) {
153152
final int exitCode = await processRunner.runAndStream(
154153
flutterCommand,
155154
<String>[
@@ -170,7 +169,7 @@ class BuildExamplesCommand extends PluginCommand {
170169

171170
if (getBoolArg(kApk)) {
172171
print('\nBUILDING APK for $packageName');
173-
if (isAndroidPlugin(plugin, fileSystem)) {
172+
if (isAndroidPlugin(plugin)) {
174173
final int exitCode = await processRunner.runAndStream(
175174
flutterCommand,
176175
<String>[

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

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,13 @@ const String kApk = 'apk';
4848
const String kEnableExperiment = 'enable-experiment';
4949

5050
/// Returns whether the given directory contains a Flutter package.
51-
bool isFlutterPackage(FileSystemEntity entity, FileSystem fileSystem) {
51+
bool isFlutterPackage(FileSystemEntity entity) {
5252
if (entity is! Directory) {
5353
return false;
5454
}
5555

5656
try {
57-
final File pubspecFile =
58-
fileSystem.file(p.join(entity.path, 'pubspec.yaml'));
57+
final File pubspecFile = entity.childFile('pubspec.yaml');
5958
final YamlMap pubspecYaml =
6059
loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
6160
final YamlMap? dependencies = pubspecYaml['dependencies'] as YamlMap?;
@@ -78,8 +77,7 @@ bool isFlutterPackage(FileSystemEntity entity, FileSystem fileSystem) {
7877
/// plugin:
7978
/// platforms:
8079
/// [platform]:
81-
bool pluginSupportsPlatform(
82-
String platform, FileSystemEntity entity, FileSystem fileSystem) {
80+
bool pluginSupportsPlatform(String platform, FileSystemEntity entity) {
8381
assert(platform == kIos ||
8482
platform == kAndroid ||
8583
platform == kWeb ||
@@ -91,8 +89,7 @@ bool pluginSupportsPlatform(
9189
}
9290

9391
try {
94-
final File pubspecFile =
95-
fileSystem.file(p.join(entity.path, 'pubspec.yaml'));
92+
final File pubspecFile = entity.childFile('pubspec.yaml');
9693
final YamlMap pubspecYaml =
9794
loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
9895
final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
@@ -120,33 +117,33 @@ bool pluginSupportsPlatform(
120117
}
121118

122119
/// Returns whether the given directory contains a Flutter Android plugin.
123-
bool isAndroidPlugin(FileSystemEntity entity, FileSystem fileSystem) {
124-
return pluginSupportsPlatform(kAndroid, entity, fileSystem);
120+
bool isAndroidPlugin(FileSystemEntity entity) {
121+
return pluginSupportsPlatform(kAndroid, entity);
125122
}
126123

127124
/// Returns whether the given directory contains a Flutter iOS plugin.
128-
bool isIosPlugin(FileSystemEntity entity, FileSystem fileSystem) {
129-
return pluginSupportsPlatform(kIos, entity, fileSystem);
125+
bool isIosPlugin(FileSystemEntity entity) {
126+
return pluginSupportsPlatform(kIos, entity);
130127
}
131128

132129
/// Returns whether the given directory contains a Flutter web plugin.
133-
bool isWebPlugin(FileSystemEntity entity, FileSystem fileSystem) {
134-
return pluginSupportsPlatform(kWeb, entity, fileSystem);
130+
bool isWebPlugin(FileSystemEntity entity) {
131+
return pluginSupportsPlatform(kWeb, entity);
135132
}
136133

137134
/// Returns whether the given directory contains a Flutter Windows plugin.
138-
bool isWindowsPlugin(FileSystemEntity entity, FileSystem fileSystem) {
139-
return pluginSupportsPlatform(kWindows, entity, fileSystem);
135+
bool isWindowsPlugin(FileSystemEntity entity) {
136+
return pluginSupportsPlatform(kWindows, entity);
140137
}
141138

142139
/// Returns whether the given directory contains a Flutter macOS plugin.
143-
bool isMacOsPlugin(FileSystemEntity entity, FileSystem fileSystem) {
144-
return pluginSupportsPlatform(kMacos, entity, fileSystem);
140+
bool isMacOsPlugin(FileSystemEntity entity) {
141+
return pluginSupportsPlatform(kMacos, entity);
145142
}
146143

147144
/// Returns whether the given directory contains a Flutter linux plugin.
148-
bool isLinuxPlugin(FileSystemEntity entity, FileSystem fileSystem) {
149-
return pluginSupportsPlatform(kLinux, entity, fileSystem);
145+
bool isLinuxPlugin(FileSystemEntity entity) {
146+
return pluginSupportsPlatform(kLinux, entity);
150147
}
151148

152149
/// Throws a [ToolExit] with `exitCode` and log the `errorMessage` in red.
@@ -169,8 +166,7 @@ class ToolExit extends Error {
169166
abstract class PluginCommand extends Command<void> {
170167
/// Creates a command to operate on [packagesDir] with the given environment.
171168
PluginCommand(
172-
this.packagesDir,
173-
this.fileSystem, {
169+
this.packagesDir, {
174170
this.processRunner = const ProcessRunner(),
175171
this.gitDir,
176172
}) {
@@ -223,11 +219,6 @@ abstract class PluginCommand extends Command<void> {
223219
/// The directory containing the plugin packages.
224220
final Directory packagesDir;
225221

226-
/// The file system.
227-
///
228-
/// This can be overridden for testing.
229-
final FileSystem fileSystem;
230-
231222
/// The process runner.
232223
///
233224
/// This can be overridden for testing.
@@ -414,28 +405,25 @@ abstract class PluginCommand extends Command<void> {
414405
/// Returns whether the specified entity is a directory containing a
415406
/// `pubspec.yaml` file.
416407
bool _isDartPackage(FileSystemEntity entity) {
417-
return entity is Directory &&
418-
fileSystem.file(p.join(entity.path, 'pubspec.yaml')).existsSync();
408+
return entity is Directory && entity.childFile('pubspec.yaml').existsSync();
419409
}
420410

421411
/// Returns the example Dart packages contained in the specified plugin, or
422412
/// an empty List, if the plugin has no examples.
423413
Iterable<Directory> getExamplesForPlugin(Directory plugin) {
424-
final Directory exampleFolder =
425-
fileSystem.directory(p.join(plugin.path, 'example'));
414+
final Directory exampleFolder = plugin.childDirectory('example');
426415
if (!exampleFolder.existsSync()) {
427416
return <Directory>[];
428417
}
429-
if (isFlutterPackage(exampleFolder, fileSystem)) {
418+
if (isFlutterPackage(exampleFolder)) {
430419
return <Directory>[exampleFolder];
431420
}
432421
// Only look at the subdirectories of the example directory if the example
433422
// directory itself is not a Dart package, and only look one level below the
434423
// example directory for other dart packages.
435424
return exampleFolder
436425
.listSync()
437-
.where(
438-
(FileSystemEntity entity) => isFlutterPackage(entity, fileSystem))
426+
.where((FileSystemEntity entity) => isFlutterPackage(entity))
439427
.cast<Directory>();
440428
}
441429

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import 'dart:async';
88
import 'dart:io' as io;
99

1010
import 'package:file/file.dart';
11-
import 'package:path/path.dart' as p;
1211
import 'package:pub_semver/pub_semver.dart';
1312
import 'package:pubspec_parse/pubspec_parse.dart';
1413

@@ -18,11 +17,10 @@ import 'common.dart';
1817
class CreateAllPluginsAppCommand extends PluginCommand {
1918
/// Creates an instance of the builder command.
2019
CreateAllPluginsAppCommand(
21-
Directory packagesDir,
22-
FileSystem fileSystem, {
20+
Directory packagesDir, {
2321
this.pluginsRoot,
24-
}) : super(packagesDir, fileSystem) {
25-
pluginsRoot ??= fileSystem.currentDirectory;
22+
}) : super(packagesDir) {
23+
pluginsRoot ??= packagesDir.fileSystem.currentDirectory;
2624
appDirectory = pluginsRoot.childDirectory('all_plugins');
2725
}
2826

@@ -161,8 +159,7 @@ class CreateAllPluginsAppCommand extends PluginCommand {
161159

162160
await for (final Directory package in getPlugins()) {
163161
final String pluginName = package.path.split('/').last;
164-
final File pubspecFile =
165-
fileSystem.file(p.join(package.path, 'pubspec.yaml'));
162+
final File pubspecFile = package.childFile('pubspec.yaml');
166163
final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
167164

168165
if (pubspec.publishTo != 'none') {

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

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@ import 'common.dart';
1212
class DriveExamplesCommand extends PluginCommand {
1313
/// Creates an instance of the drive command.
1414
DriveExamplesCommand(
15-
Directory packagesDir,
16-
FileSystem fileSystem, {
15+
Directory packagesDir, {
1716
ProcessRunner processRunner = const ProcessRunner(),
18-
}) : super(packagesDir, fileSystem, processRunner: processRunner) {
17+
}) : super(packagesDir, processRunner: processRunner) {
1918
argParser.addFlag(kAndroid,
2019
help: 'Runs the Android implementation of the examples');
2120
argParser.addFlag(kIos,
@@ -67,7 +66,7 @@ class DriveExamplesCommand extends PluginCommand {
6766
continue;
6867
}
6968
print('\n==========\nChecking $pluginName...');
70-
if (!(await _pluginSupportedOnCurrentPlatform(plugin, fileSystem))) {
69+
if (!(await _pluginSupportedOnCurrentPlatform(plugin))) {
7170
print('Not supported for the target platform; skipping.');
7271
continue;
7372
}
@@ -79,8 +78,7 @@ class DriveExamplesCommand extends PluginCommand {
7978
++examplesFound;
8079
final String packageName =
8180
p.relative(example.path, from: packagesDir.path);
82-
final Directory driverTests =
83-
fileSystem.directory(p.join(example.path, 'test_driver'));
81+
final Directory driverTests = example.childDirectory('test_driver');
8482
if (!driverTests.existsSync()) {
8583
print('No driver tests found for $packageName');
8684
continue;
@@ -98,21 +96,21 @@ class DriveExamplesCommand extends PluginCommand {
9896
'.dart',
9997
);
10098
String deviceTestPath = p.join('test', deviceTestName);
101-
if (!fileSystem
99+
if (!example.fileSystem
102100
.file(p.join(example.path, deviceTestPath))
103101
.existsSync()) {
104102
// If the app isn't in test/ folder, look in test_driver/ instead.
105103
deviceTestPath = p.join('test_driver', deviceTestName);
106104
}
107105

108106
final List<String> targetPaths = <String>[];
109-
if (fileSystem
107+
if (example.fileSystem
110108
.file(p.join(example.path, deviceTestPath))
111109
.existsSync()) {
112110
targetPaths.add(deviceTestPath);
113111
} else {
114112
final Directory integrationTests =
115-
fileSystem.directory(p.join(example.path, 'integration_test'));
113+
example.childDirectory('integration_test');
116114

117115
if (await integrationTests.exists()) {
118116
await for (final FileSystemEntity integrationTest
@@ -145,27 +143,27 @@ Tried searching for the following:
145143
driveArgs.add('--enable-experiment=$enableExperiment');
146144
}
147145

148-
if (isLinux && isLinuxPlugin(plugin, fileSystem)) {
146+
if (isLinux && isLinuxPlugin(plugin)) {
149147
driveArgs.addAll(<String>[
150148
'-d',
151149
'linux',
152150
]);
153151
}
154-
if (isMacos && isMacOsPlugin(plugin, fileSystem)) {
152+
if (isMacos && isMacOsPlugin(plugin)) {
155153
driveArgs.addAll(<String>[
156154
'-d',
157155
'macos',
158156
]);
159157
}
160-
if (isWeb && isWebPlugin(plugin, fileSystem)) {
158+
if (isWeb && isWebPlugin(plugin)) {
161159
driveArgs.addAll(<String>[
162160
'-d',
163161
'web-server',
164162
'--web-port=7357',
165163
'--browser-name=chrome',
166164
]);
167165
}
168-
if (isWindows && isWindowsPlugin(plugin, fileSystem)) {
166+
if (isWindows && isWindowsPlugin(plugin)) {
169167
driveArgs.addAll(<String>[
170168
'-d',
171169
'windows',
@@ -220,35 +218,35 @@ Tried searching for the following:
220218
}
221219

222220
Future<bool> _pluginSupportedOnCurrentPlatform(
223-
FileSystemEntity plugin, FileSystem fileSystem) async {
221+
FileSystemEntity plugin) async {
224222
final bool isAndroid = getBoolArg(kAndroid);
225223
final bool isIOS = getBoolArg(kIos);
226224
final bool isLinux = getBoolArg(kLinux);
227225
final bool isMacos = getBoolArg(kMacos);
228226
final bool isWeb = getBoolArg(kWeb);
229227
final bool isWindows = getBoolArg(kWindows);
230228
if (isAndroid) {
231-
return isAndroidPlugin(plugin, fileSystem);
229+
return isAndroidPlugin(plugin);
232230
}
233231
if (isIOS) {
234-
return isIosPlugin(plugin, fileSystem);
232+
return isIosPlugin(plugin);
235233
}
236234
if (isLinux) {
237-
return isLinuxPlugin(plugin, fileSystem);
235+
return isLinuxPlugin(plugin);
238236
}
239237
if (isMacos) {
240-
return isMacOsPlugin(plugin, fileSystem);
238+
return isMacOsPlugin(plugin);
241239
}
242240
if (isWeb) {
243-
return isWebPlugin(plugin, fileSystem);
241+
return isWebPlugin(plugin);
244242
}
245243
if (isWindows) {
246-
return isWindowsPlugin(plugin, fileSystem);
244+
return isWindowsPlugin(plugin);
247245
}
248246
// When we are here, no flags are specified. Only return true if the plugin
249247
// supports Android for legacy command support.
250248
// TODO(cyanglaz): Make Android flag also required like other platforms
251249
// (breaking change). https://github.com/flutter/flutter/issues/58285
252-
return isAndroidPlugin(plugin, fileSystem);
250+
return isAndroidPlugin(plugin);
253251
}
254252
}

0 commit comments

Comments
 (0)