[flutter_tools] refactor FlutterManifest to be context-free (#54555)

packages/flutter_tools/lib/src/asset.dart

@@ -128,7 +128,11 @@ class _ManifestAssetBundle implements AssetBundle {
     packagesPath ??= globals.fs.path.absolute(PackageMap.globalPackagesPath);
     FlutterManifest flutterManifest;
     try {
-      flutterManifest = FlutterManifest.createFromPath(manifestPath);
+      flutterManifest = FlutterManifest.createFromPath(
+        manifestPath,
+        logger: globals.logger,
+        fileSystem: globals.fs,
+      );
     } on Exception catch (e) {
       globals.printStatus('Error detected in pubspec.yaml:', emphasis: true);
       globals.printError('$e');
@@ -187,7 +191,11 @@ class _ManifestAssetBundle implements AssetBundle {
       final Uri packageUri = package.packageUriRoot;
       if (packageUri != null && packageUri.scheme == 'file') {
         final String packageManifestPath = globals.fs.path.fromUri(packageUri.resolve('../pubspec.yaml'));
-        final FlutterManifest packageFlutterManifest = FlutterManifest.createFromPath(packageManifestPath);
+        final FlutterManifest packageFlutterManifest = FlutterManifest.createFromPath(
+          packageManifestPath,
+          logger: globals.logger,
+          fileSystem: globals.fs,
+        );
         if (packageFlutterManifest == null) {
           continue;
         }

packages/flutter_tools/lib/src/flutter_manifest.dart

@@ -7,42 +7,45 @@ import 'package:pub_semver/pub_semver.dart';
 import 'package:yaml/yaml.dart';
 
 import 'base/file_system.dart';
+import 'base/logger.dart';
 import 'base/user_messages.dart';
 import 'base/utils.dart';
 import 'cache.dart';
-import 'globals.dart' as globals;
 import 'plugins.dart';
 
 /// A wrapper around the `flutter` section in the `pubspec.yaml` file.
 class FlutterManifest {
-  FlutterManifest._();
+  FlutterManifest._(this._logger);
 
   /// Returns an empty manifest.
-  static FlutterManifest empty() {
-    final FlutterManifest manifest = FlutterManifest._();
+  factory FlutterManifest.empty({ @required Logger logger }) {
+    final FlutterManifest manifest = FlutterManifest._(logger);
     manifest._descriptor = const <String, dynamic>{};
     manifest._flutterDescriptor = const <String, dynamic>{};
     return manifest;
   }
 
   /// Returns null on invalid manifest. Returns empty manifest on missing file.
-  static FlutterManifest createFromPath(String path) {
-    if (path == null || !globals.fs.isFileSync(path)) {
-      return _createFromYaml(null);
+  static FlutterManifest createFromPath(String path, {
+    @required FileSystem fileSystem,
+    @required Logger logger,
+  }) {
+    if (path == null || !fileSystem.isFileSync(path)) {
+      return _createFromYaml(null, logger);
     }
-    final String manifest = globals.fs.file(path).readAsStringSync();
-    return createFromString(manifest);
+    final String manifest = fileSystem.file(path).readAsStringSync();
+    return FlutterManifest.createFromString(manifest, logger: logger);
   }
 
   /// Returns null on missing or invalid manifest
   @visibleForTesting
-  static FlutterManifest createFromString(String manifest) {
-    return _createFromYaml(loadYaml(manifest) as YamlMap);
+  static FlutterManifest createFromString(String manifest, { @required Logger logger }) {
+    return _createFromYaml(loadYaml(manifest) as YamlMap, logger);
   }
 
-  static FlutterManifest _createFromYaml(YamlMap yamlDocument) {
-    final FlutterManifest pubspec = FlutterManifest._();
-    if (yamlDocument != null && !_validate(yamlDocument)) {
+  static FlutterManifest _createFromYaml(YamlMap yamlDocument, Logger logger) {
+    final FlutterManifest pubspec = FlutterManifest._(logger);
+    if (yamlDocument != null && !_validate(yamlDocument, logger)) {
       return null;
     }
 
@@ -63,6 +66,8 @@ class FlutterManifest {
     return pubspec;
   }
 
+  final Logger _logger;
+
   /// A map representation of the entire `pubspec.yaml` file.
   Map<String, dynamic> _descriptor;
 
@@ -91,7 +96,7 @@ class FlutterManifest {
       version = Version.parse(verStr);
     } on Exception {
       if (!_hasShowInvalidVersionMsg) {
-        globals.printStatus(userMessages.invalidVersionSettingHintMessage(verStr), emphasis: true);
+        _logger.printStatus(userMessages.invalidVersionSettingHintMessage(verStr), emphasis: true);
         _hasShowInvalidVersionMsg = true;
       }
     }
@@ -203,14 +208,14 @@ class FlutterManifest {
     final List<Uri> results = <Uri>[];
     for (final Object asset in assets) {
       if (asset is! String || asset == null || asset == '') {
-        globals.printError('Asset manifest contains a null or empty uri.');
+        _logger.printError('Asset manifest contains a null or empty uri.');
         continue;
       }
       final String stringAsset = asset as String;
       try {
         results.add(Uri(pathSegments: stringAsset.split('/')));
       } on FormatException {
-        globals.printError('Asset manifest contains invalid uri: $asset.');
+        _logger.printError('Asset manifest contains invalid uri: $asset.');
       }
     }
     return results;
@@ -233,11 +238,11 @@ class FlutterManifest {
       final YamlList fontFiles = fontFamily['fonts'] as YamlList;
       final String familyName = fontFamily['family'] as String;
       if (familyName == null) {
-        globals.printError('Warning: Missing family name for font.', emphasis: true);
+        _logger.printError('Warning: Missing family name for font.', emphasis: true);
         continue;
       }
       if (fontFiles == null) {
-        globals.printError('Warning: No fonts specified for font $familyName', emphasis: true);
+        _logger.printError('Warning: No fonts specified for font $familyName', emphasis: true);
         continue;
       }
 
@@ -245,7 +250,7 @@ class FlutterManifest {
       for (final Map<dynamic, dynamic> fontFile in fontFiles.cast<Map<dynamic, dynamic>>()) {
         final String asset = fontFile['asset'] as String;
         if (asset == null) {
-          globals.printError('Warning: Missing asset in fonts for $familyName', emphasis: true);
+          _logger.printError('Warning: Missing asset in fonts for $familyName', emphasis: true);
           continue;
         }
 
@@ -310,16 +315,16 @@ class FontAsset {
 }
 
 @visibleForTesting
-String buildSchemaDir(FileSystem fs) {
-  return globals.fs.path.join(
-    globals.fs.path.absolute(Cache.flutterRoot), 'packages', 'flutter_tools', 'schema',
+String buildSchemaDir(FileSystem fileSystem) {
+  return fileSystem.path.join(
+    fileSystem.path.absolute(Cache.flutterRoot), 'packages', 'flutter_tools', 'schema',
   );
 }
 
 @visibleForTesting
-String buildSchemaPath(FileSystem fs) {
-  return globals.fs.path.join(
-    buildSchemaDir(fs),
+String buildSchemaPath(FileSystem fileSystem) {
+  return fileSystem.path.join(
+    buildSchemaDir(fileSystem),
     'pubspec_yaml.json',
   );
 }
@@ -327,7 +332,7 @@ String buildSchemaPath(FileSystem fs) {
 /// This method should be kept in sync with the schema in
 /// `$FLUTTER_ROOT/packages/flutter_tools/schema/pubspec_yaml.json`,
 /// but avoid introducing dependencies on packages for simple validation.
-bool _validate(YamlMap manifest) {
+bool _validate(YamlMap manifest, Logger logger) {
   final List<String> errors = <String>[];
   for (final MapEntry<dynamic, dynamic> kvp in manifest.entries) {
     if (kvp.key is! String) {
@@ -357,8 +362,8 @@ bool _validate(YamlMap manifest) {
   }
 
   if (errors.isNotEmpty) {
-    globals.printStatus('Error detected in pubspec.yaml:', emphasis: true);
-    globals.printError(errors.join('\n'));
+    logger.printStatus('Error detected in pubspec.yaml:', emphasis: true);
+    logger.printError(errors.join('\n'));
     return false;
   }
 

packages/flutter_tools/lib/src/globals.dart

@@ -43,7 +43,10 @@ Logger get logger => context.get<Logger>();
 OperatingSystemUtils get os => context.get<OperatingSystemUtils>();
 PersistentToolState get persistentToolState => PersistentToolState.instance;
 Usage get flutterUsage => context.get<Usage>();
-FlutterProjectFactory get projectFactory => context.get<FlutterProjectFactory>() ?? FlutterProjectFactory();
+FlutterProjectFactory get projectFactory => context.get<FlutterProjectFactory>() ?? FlutterProjectFactory(
+  logger: logger,
+  fileSystem: fs,
+);
 
 const FileSystem _kLocalFs = LocalFileSystem();
 

packages/flutter_tools/lib/src/project.dart

@@ -12,6 +12,7 @@ import 'android/gradle_utils.dart' as gradle;
 import 'artifacts.dart';
 import 'base/common.dart';
 import 'base/file_system.dart';
+import 'base/logger.dart';
 import 'build_info.dart';
 import 'bundle.dart' as bundle;
 import 'features.dart';
@@ -24,7 +25,14 @@ import 'plugins.dart';
 import 'template.dart';
 
 class FlutterProjectFactory {
-  FlutterProjectFactory();
+  FlutterProjectFactory({
+    @required Logger logger,
+    @required FileSystem fileSystem,
+  }) : _logger = logger,
+       _fileSystem = fileSystem;
+
+  final Logger _logger;
+  final FileSystem _fileSystem;
 
   @visibleForTesting
   final Map<String, FlutterProject> projects =
@@ -34,14 +42,18 @@ class FlutterProjectFactory {
   /// if `pubspec.yaml` or `example/pubspec.yaml` is invalid.
   FlutterProject fromDirectory(Directory directory) {
     assert(directory != null);
-    return projects.putIfAbsent(directory.path, /* ifAbsent */ () {
+    return projects.putIfAbsent(directory.path, () {
       final FlutterManifest manifest = FlutterProject._readManifest(
         directory.childFile(bundle.defaultManifestPath).path,
+        logger: _logger,
+        fileSystem: _fileSystem,
       );
       final FlutterManifest exampleManifest = FlutterProject._readManifest(
         FlutterProject._exampleDirectory(directory)
             .childFile(bundle.defaultManifestPath)
             .path,
+        logger: _logger,
+        fileSystem: _fileSystem,
       );
       return FlutterProject(directory, manifest, exampleManifest);
     });
@@ -167,7 +179,7 @@ class FlutterProject {
   FlutterProject get example => FlutterProject(
     _exampleDirectory(directory),
     _exampleManifest,
-    FlutterManifest.empty(),
+    FlutterManifest.empty(logger: globals.logger),
   );
 
   /// True if this project is a Flutter module project.
@@ -187,13 +199,20 @@ class FlutterProject {
   ///
   /// Completes with an empty [FlutterManifest], if the file does not exist.
   /// Completes with a ToolExit on validation error.
-  static FlutterManifest _readManifest(String path) {
+  static FlutterManifest _readManifest(String path, {
+    @required Logger logger,
+    @required FileSystem fileSystem,
+  }) {
     FlutterManifest manifest;
     try {
-      manifest = FlutterManifest.createFromPath(path);
+      manifest = FlutterManifest.createFromPath(
+        path,
+        logger: logger,
+        fileSystem: fileSystem,
+      );
     } on YamlException catch (e) {
-      globals.printStatus('Error detected in pubspec.yaml:', emphasis: true);
-      globals.printError('$e');
+      logger.printStatus('Error detected in pubspec.yaml:', emphasis: true);
+      logger.printError('$e');
     }
     if (manifest == null) {
       throwToolExit('Please correct the pubspec.yaml file at $path');

packages/flutter_tools/test/general.shard/github_template_test.dart

@@ -160,7 +160,10 @@ void main() {
           fileSystem: fs,
           logger: logger,
           client: SuccessShortenURLFakeHttpClient(),
-          flutterProjectFactory: FlutterProjectFactory(),
+          flutterProjectFactory: FlutterProjectFactory(
+            fileSystem: fs,
+            logger: logger,
+          ),
         );
         expect(
             await creator.toolCrashIssueTemplateGitHubURL(command, error, stackTrace, doctorText),
@@ -176,7 +179,10 @@ void main() {
           fileSystem: fs,
           logger: logger,
           client: FakeHttpClient(),
-          flutterProjectFactory: FlutterProjectFactory(),
+          flutterProjectFactory: FlutterProjectFactory(
+            fileSystem: fs,
+            logger: logger,
+          ),
         );
         expect(
             await creator.toolCrashIssueTemplateGitHubURL(command, error, stackTrace, doctorText),
@@ -199,7 +205,10 @@ void main() {
           fileSystem: fs,
           logger: logger,
           client: FakeHttpClient(),
-          flutterProjectFactory: FlutterProjectFactory(),
+          flutterProjectFactory: FlutterProjectFactory(
+            fileSystem: fs,
+            logger: logger,
+          ),
         );
         final Directory projectDirectory = fs.currentDirectory;
 

packages/flutter_tools/test/general.shard/project_test.dart

@@ -8,6 +8,7 @@ import 'package:file/file.dart';
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/context.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/features.dart';
 import 'package:flutter_tools/src/flutter_manifest.dart';
@@ -23,6 +24,10 @@ import '../src/context.dart';
 import '../src/testbed.dart';
 
 void main() {
+  // TODO(jonahwilliams): remove once FlutterProject is fully refactored.
+  // this is safe since no tests have expectations on the test logger.
+  final BufferLogger logger = BufferLogger.test();
+
   group('Project', () {
     group('construction', () {
       testInMemory('fails on null directory', () async {
@@ -154,8 +159,8 @@ void main() {
       testInMemory('does nothing, if project is not created', () async {
         final FlutterProject project = FlutterProject(
           globals.fs.directory('not_created'),
-          FlutterManifest.empty(),
-          FlutterManifest.empty(),
+          FlutterManifest.empty(logger: logger),
+          FlutterManifest.empty(logger: logger),
         );
         await project.ensureReadyForPlatformSpecificTooling();
         expectNotExists(project.directory);
@@ -197,7 +202,10 @@ void main() {
         FileSystem: () => MemoryFileSystem(),
         ProcessManager: () => FakeProcessManager.any(),
         FeatureFlags: () => TestFeatureFlags(isMacOSEnabled: true),
-        FlutterProjectFactory: () => FlutterProjectFactory(),
+        FlutterProjectFactory: () => FlutterProjectFactory(
+          logger: logger,
+          fileSystem: globals.fs,
+        ),
       });
       testUsingContext('generates Xcode configuration for macOS', () async {
         final FlutterProject project = await someProject();
@@ -208,7 +216,10 @@ void main() {
         FileSystem: () => MemoryFileSystem(),
         ProcessManager: () => FakeProcessManager.any(),
         FeatureFlags: () => TestFeatureFlags(isMacOSEnabled: true),
-        FlutterProjectFactory: () => FlutterProjectFactory(),
+        FlutterProjectFactory: () => FlutterProjectFactory(
+          logger: logger,
+          fileSystem: globals.fs,
+        ),
       });
       testUsingContext('injects plugins for Linux', () async {
         final FlutterProject project = await someProject();
@@ -220,7 +231,10 @@ void main() {
         FileSystem: () => MemoryFileSystem(),
         ProcessManager: () => FakeProcessManager.any(),
         FeatureFlags: () => TestFeatureFlags(isLinuxEnabled: true),
-        FlutterProjectFactory: () => FlutterProjectFactory(),
+        FlutterProjectFactory: () => FlutterProjectFactory(
+          logger: logger,
+          fileSystem: globals.fs,
+        ),
       });
       testUsingContext('injects plugins for Windows', () async {
         final FlutterProject project = await someProject();
@@ -246,7 +260,10 @@ EndGlobal''');
         FileSystem: () => MemoryFileSystem(),
         ProcessManager: () => FakeProcessManager.any(),
         FeatureFlags: () => TestFeatureFlags(isWindowsEnabled: true),
-        FlutterProjectFactory: () => FlutterProjectFactory(),
+        FlutterProjectFactory: () => FlutterProjectFactory(
+          logger: logger,
+          fileSystem: globals.fs,
+        ),
       });
       testInMemory('creates Android library in module', () async {
         final FlutterProject project = await aModuleProject();
@@ -312,7 +329,10 @@ EndGlobal''');
       setUp(() {
         fs = MemoryFileSystem();
         mockXcodeProjectInterpreter = MockXcodeProjectInterpreter();
-        flutterProjectFactory = FlutterProjectFactory();
+        flutterProjectFactory = FlutterProjectFactory(
+          logger: logger,
+          fileSystem: fs,
+        );
       });
 
       testInMemory('default host app language', () async {
@@ -348,7 +368,10 @@ apply plugin: 'kotlin-android'
         fs = MemoryFileSystem();
         mockPlistUtils = MockPlistUtils();
         mockXcodeProjectInterpreter = MockXcodeProjectInterpreter();
-        flutterProjectFactory = FlutterProjectFactory();
+        flutterProjectFactory = FlutterProjectFactory(
+          fileSystem: fs,
+          logger: logger,
+        );
       });
 
       void testWithMocks(String description, Future<void> testMethod()) {
@@ -521,8 +544,12 @@ apply plugin: 'kotlin-android'
     FlutterProjectFactory flutterProjectFactory;
 
     setUp(() {
-      testbed = Testbed();
-      flutterProjectFactory = FlutterProjectFactory();
+      testbed = Testbed(setup: () {
+        flutterProjectFactory = FlutterProjectFactory(
+          fileSystem: globals.fs,
+          logger: globals.logger,
+        );
+      });
     });
 
     test('Handles asking for builders from an invalid pubspec', () => testbed.run(() {
@@ -646,7 +673,10 @@ void testInMemory(String description, Future<void> testMethod()) {
   packagesFile.createSync(recursive: true);
   packagesFile.writeAsStringSync('flutter_template_images:${dummyTemplateImagesDirectory.uri}');
 
-  final FlutterProjectFactory flutterProjectFactory = FlutterProjectFactory();
+  final FlutterProjectFactory flutterProjectFactory = FlutterProjectFactory(
+    fileSystem: testFileSystem,
+    logger: globals.logger ?? BufferLogger.test(),
+  );
 
   testUsingContext(
     description,

packages/flutter_tools/test/src/android_common.dart

@@ -4,6 +4,7 @@
 
 import 'package:meta/meta.dart';
 
+import 'package:flutter_tools/src/globals.dart' as globals;
 import 'package:flutter_tools/src/android/android_builder.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/build_info.dart';
@@ -39,7 +40,11 @@ class FakeAndroidBuilder implements AndroidBuilder {
 /// within [directoryOverride].
 class FakeFlutterProjectFactory extends FlutterProjectFactory {
   FakeFlutterProjectFactory(this.directoryOverride) :
-    assert(directoryOverride != null);
+    assert(directoryOverride != null),
+    super(
+      fileSystem: globals.fs,
+      logger: globals.logger,
+    );
 
   final Directory directoryOverride;