[flutter_tools] remove globals from plist parser and update tests (#51444)

packages/flutter_tools/lib/src/android/android_studio.dart

@@ -41,14 +41,14 @@ class AndroidStudio implements Comparable<AndroidStudio> {
   factory AndroidStudio.fromMacOSBundle(String bundlePath) {
     String studioPath = globals.fs.path.join(bundlePath, 'Contents');
     String plistFile = globals.fs.path.join(studioPath, 'Info.plist');
-    Map<String, dynamic> plistValues = PlistParser.instance.parseFile(plistFile);
+    Map<String, dynamic> plistValues = globals.plistParser.parseFile(plistFile);
     // As AndroidStudio managed by JetBrainsToolbox could have a wrapper pointing to the real Android Studio.
     // Check if we've found a JetBrainsToolbox wrapper and deal with it properly.
     final String jetBrainsToolboxAppBundlePath = plistValues['JetBrainsToolboxApp'] as String;
     if (jetBrainsToolboxAppBundlePath != null) {
       studioPath = globals.fs.path.join(jetBrainsToolboxAppBundlePath, 'Contents');
       plistFile = globals.fs.path.join(studioPath, 'Info.plist');
-      plistValues = PlistParser.instance.parseFile(plistFile);
+      plistValues = globals.plistParser.parseFile(plistFile);
     }
 
     final String versionString = plistValues[PlistParser.kCFBundleShortVersionStringKey] as String;

packages/flutter_tools/lib/src/application_package.dart

@@ -312,7 +312,7 @@ abstract class IOSApp extends ApplicationPackage {
       globals.printError('Invalid prebuilt iOS app. Does not contain Info.plist.');
       return null;
     }
-    final String id = PlistParser.instance.getValueFromFile(
+    final String id = globals.plistParser.getValueFromFile(
       plistPath,
       PlistParser.kCFBundleIdentifierKey,
     );

packages/flutter_tools/lib/src/doctor.dart

@@ -862,7 +862,7 @@ class IntelliJValidatorOnMac extends IntelliJValidator {
 
   @override
   String get version {
-    _version ??= PlistParser.instance.getValueFromFile(
+    _version ??= globals.plistParser.getValueFromFile(
         plistFile,
         PlistParser.kCFBundleShortVersionStringKey,
       ) ?? 'unknown';
@@ -876,7 +876,8 @@ class IntelliJValidatorOnMac extends IntelliJValidator {
       return _pluginsPath;
     }
 
-    final String altLocation = PlistParser.instance.getValueFromFile(plistFile, 'JetBrainsToolboxApp');
+    final String altLocation = globals.plistParser
+      .getValueFromFile(plistFile, 'JetBrainsToolboxApp');
 
     if (altLocation != null) {
       _pluginsPath = altLocation + '.plugins';

packages/flutter_tools/lib/src/globals.dart

@@ -22,6 +22,7 @@ import 'base/user_messages.dart';
 import 'cache.dart';
 import 'ios/ios_deploy.dart';
 import 'ios/mac.dart';
+import 'ios/plist_parser.dart';
 import 'macos/xcode.dart';
 import 'persistent_tool_state.dart';
 import 'version.dart';
@@ -150,5 +151,12 @@ final AnsiTerminal _defaultAnsiTerminal = AnsiTerminal(
 /// The global Stdio wrapper.
 Stdio get stdio => context.get<Stdio>() ?? const Stdio();
 
+PlistParser get plistParser => context.get<PlistParser>() ?? (_defaultInstance ??= PlistParser(
+  fileSystem: fs,
+  processManager: processManager,
+  logger: logger,
+));
+PlistParser _defaultInstance;
+
 /// The [ChromeLauncher] instance.
 ChromeLauncher get chromeLauncher => context.get<ChromeLauncher>();

packages/flutter_tools/lib/src/ios/bitcode.dart

@@ -9,7 +9,6 @@ import '../base/process.dart';
 import '../base/version.dart';
 import '../build_info.dart';
 import '../globals.dart' as globals;
-import '../ios/plist_parser.dart';
 import '../macos/xcode.dart';
 
 const bool kBitcodeEnabledDefault = false;
@@ -28,7 +27,7 @@ Future<void> validateBitcode(BuildMode buildMode, TargetPlatform targetPlatform)
 
   final RunResult clangResult = await xcode.clang(<String>['--version']);
   final String clangVersion = clangResult.stdout.split('\n').first;
-  final String engineClangVersion = PlistParser.instance.getValueFromFile(
+  final String engineClangVersion = globals.plistParser.getValueFromFile(
     globals.fs.path.join(flutterFrameworkPath, 'Info.plist'),
     'ClangVersion',
   );

packages/flutter_tools/lib/src/ios/plist_parser.dart

@@ -2,23 +2,33 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import '../base/context.dart';
+import 'package:meta/meta.dart';
+import 'package:process/process.dart';
+
 import '../base/file_system.dart';
 import '../base/io.dart';
+import '../base/logger.dart';
 import '../base/process.dart';
 import '../base/utils.dart';
 import '../convert.dart';
-import '../globals.dart' as globals;
 
 class PlistParser {
-  const PlistParser();
+  PlistParser({
+    @required FileSystem fileSystem,
+    @required Logger logger,
+    @required ProcessManager processManager,
+  }) : _fileSystem = fileSystem,
+       _logger = logger,
+       _processUtils = ProcessUtils(logger: logger, processManager: processManager);
+
+  final FileSystem _fileSystem;
+  final Logger _logger;
+  final ProcessUtils _processUtils;
 
   static const String kCFBundleIdentifierKey = 'CFBundleIdentifier';
   static const String kCFBundleShortVersionStringKey = 'CFBundleShortVersionString';
   static const String kCFBundleExecutable = 'CFBundleExecutable';
 
-  static PlistParser get instance => context.get<PlistParser>() ?? const PlistParser();
-
   /// Parses the plist file located at [plistFilePath] and returns the
   /// associated map of key/value property list pairs.
   ///
@@ -29,26 +39,26 @@ class PlistParser {
   Map<String, dynamic> parseFile(String plistFilePath) {
     assert(plistFilePath != null);
     const String executable = '/usr/bin/plutil';
-    if (!globals.fs.isFileSync(executable)) {
+    if (!_fileSystem.isFileSync(executable)) {
       throw const FileNotFoundException(executable);
     }
-    if (!globals.fs.isFileSync(plistFilePath)) {
+    if (!_fileSystem.isFileSync(plistFilePath)) {
       return const <String, dynamic>{};
     }
 
-    final String normalizedPlistPath = globals.fs.path.absolute(plistFilePath);
+    final String normalizedPlistPath = _fileSystem.path.absolute(plistFilePath);
 
     try {
       final List<String> args = <String>[
         executable, '-convert', 'json', '-o', '-', normalizedPlistPath,
       ];
-      final String jsonContent = processUtils.runSync(
+      final String jsonContent = _processUtils.runSync(
         args,
         throwOnError: true,
       ).stdout.trim();
       return castStringKeyedMap(json.decode(jsonContent));
     } on ProcessException catch (error) {
-      globals.printTrace('$error');
+      _logger.printTrace('$error');
       return const <String, dynamic>{};
     }
   }

packages/flutter_tools/lib/src/ios/simulators.dart

@@ -392,7 +392,7 @@ class IOSSimulator extends Device {
       // parsing the xcodeproj or configuration files.
       // See https://github.com/flutter/flutter/issues/31037 for more information.
       final String plistPath = globals.fs.path.join(package.simulatorBundlePath, 'Info.plist');
-      final String bundleIdentifier = PlistParser.instance.getValueFromFile(plistPath, PlistParser.kCFBundleIdentifierKey);
+      final String bundleIdentifier = globals.plistParser.getValueFromFile(plistPath, PlistParser.kCFBundleIdentifierKey);
 
       await SimControl.instance.launch(id, bundleIdentifier, args);
     } catch (error) {

packages/flutter_tools/lib/src/macos/application_package.dart

@@ -66,7 +66,7 @@ abstract class MacOSApp extends ApplicationPackage {
       globals.printError('Invalid prebuilt macOS app. Does not contain Info.plist.');
       return null;
     }
-    final Map<String, dynamic> propertyValues = PlistParser.instance.parseFile(plistPath);
+    final Map<String, dynamic> propertyValues = globals.plistParser.parseFile(plistPath);
     final String id = propertyValues[PlistParser.kCFBundleIdentifierKey] as String;
     final String executableName = propertyValues[PlistParser.kCFBundleExecutable] as String;
     if (id == null) {

packages/flutter_tools/lib/src/project.dart

@@ -402,7 +402,7 @@ class IosProject extends FlutterProjectPlatform implements XcodeBasedProject {
     // Try parsing the default, first.
     if (defaultInfoPlist.existsSync()) {
       try {
-        fromPlist = PlistParser.instance.getValueFromFile(
+        fromPlist = globals.plistParser.getValueFromFile(
           defaultHostInfoPlist.path,
           PlistParser.kCFBundleIdentifierKey,
         );

packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart

@@ -13,7 +13,6 @@ import 'package:flutter_tools/src/base/user_messages.dart';
 import 'package:flutter_tools/src/doctor.dart';
 import 'package:flutter_tools/src/features.dart';
 import 'package:flutter_tools/src/globals.dart' as globals;
-import 'package:flutter_tools/src/ios/plist_parser.dart';
 import 'package:flutter_tools/src/proxy_validator.dart';
 import 'package:flutter_tools/src/reporting/reporting.dart';
 import 'package:flutter_tools/src/vscode/vscode.dart';
@@ -77,8 +76,6 @@ void main() {
 
       final IntelliJValidatorOnMac validatorNotViaToolbox = IntelliJValidatorOnMac('Test', 'Test', pathNotViaToolbox);
       expect(validatorNotViaToolbox.plistFile, 'test/data/intellij/mac_not_via_toolbox/Contents/Info.plist');
-    }, overrides: <Type, Generator>{
-      PlistParser: () => const PlistParser(),
     });
 
     testUsingContext('vs code validator when both installed', () async {

packages/flutter_tools/test/general.shard/ios/plist_parser_test.dart

@@ -3,13 +3,14 @@
 // found in the LICENSE file.
 
 import 'dart:convert';
-import 'dart:io';
+import 'dart:io' as io;
 
 import 'package:file/file.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/base/terminal.dart';
 import 'package:flutter_tools/src/ios/plist_parser.dart';
-import 'package:flutter_tools/src/globals.dart' as globals;
-
+import 'package:platform/platform.dart';
 import 'package:process/process.dart';
 
 import '../../src/common.dart';
@@ -33,84 +34,94 @@ const String base64PlistJson =
     'HV0dGVyLmZsdXR0ZXIuYXBwIn0=';
 
 void main() {
-  group('PlistUtils', () {
-    // The tests herein explicitly don't use `MemoryFileSystem` or a mocked
-    // `ProcessManager` because doing so wouldn't actually test what we want to
-    // test, which is that the underlying tool we're using to parse Plist files
-    // works with the way we're calling it.
-    final Map<Type, Generator> overrides = <Type, Generator>{
-      FileSystem: () => const LocalFileSystemBlockingSetCurrentDirectory(),
-      ProcessManager: () => const LocalProcessManager(),
-    };
-
-    const PlistParser parser = PlistParser();
-
-    if (Platform.isMacOS) {
-      group('getValueFromFile', () {
-        File file;
-
-        setUp(() {
-          file = globals.fs.file('foo.plist')..createSync();
-        });
-
-        tearDown(() {
-          file.deleteSync();
-        });
-
-        testUsingContext('works with xml file', () async {
-          file.writeAsBytesSync(base64.decode(base64PlistXml));
-          expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
-          expect(parser.getValueFromFile(file.absolute.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
-          expect(testLogger.statusText, isEmpty);
-          expect(testLogger.errorText, isEmpty);
-        }, overrides: overrides);
-
-        testUsingContext('works with binary file', () async {
-          file.writeAsBytesSync(base64.decode(base64PlistBinary));
-          expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
-          expect(parser.getValueFromFile(file.absolute.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
-          expect(testLogger.statusText, isEmpty);
-          expect(testLogger.errorText, isEmpty);
-        }, overrides: overrides);
-
-        testUsingContext('works with json file', () async {
-          file.writeAsBytesSync(base64.decode(base64PlistJson));
-          expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
-          expect(parser.getValueFromFile(file.absolute.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
-          expect(testLogger.statusText, isEmpty);
-          expect(testLogger.errorText, isEmpty);
-        }, overrides: overrides);
-
-        testUsingContext('returns null for non-existent plist file', () async {
-          expect(parser.getValueFromFile('missing.plist', 'CFBundleIdentifier'), null);
-          expect(testLogger.statusText, isEmpty);
-          expect(testLogger.errorText, isEmpty);
-        }, overrides: overrides);
-
-        testUsingContext('returns null for non-existent key within plist', () async {
-          file.writeAsBytesSync(base64.decode(base64PlistXml));
-          expect(parser.getValueFromFile(file.path, 'BadKey'), null);
-          expect(parser.getValueFromFile(file.absolute.path, 'BadKey'), null);
-          expect(testLogger.statusText, isEmpty);
-          expect(testLogger.errorText, isEmpty);
-        }, overrides: overrides);
-
-        testUsingContext('returns null for malformed plist file', () async {
-          file.writeAsBytesSync(const <int>[1, 2, 3, 4, 5, 6]);
-          expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), null);
-          expect(testLogger.statusText, isNotEmpty);
-          expect(testLogger.errorText, isEmpty);
-        }, overrides: overrides);
-      });
-    } else {
-      testUsingContext('throws when /usr/bin/plutil is not found', () async {
-        expect(
-          () => parser.getValueFromFile('irrelevant.plist', 'ununsed'),
-          throwsA(isA<FileNotFoundException>()),
-        );
-        expect(testLogger.statusText, isEmpty);
-        expect(testLogger.errorText, isEmpty);
-      }, overrides: overrides);
-    }
+  // The tests herein explicitly don't use `MemoryFileSystem` or a mocked
+  // `ProcessManager` because doing so wouldn't actually test what we want to
+  // test, which is that the underlying tool we're using to parse Plist files
+  // works with the way we're calling it.
+  FileSystem fileSystem;
+  ProcessManager processManager;
+  File file;
+  PlistParser parser;
+  BufferLogger logger;
+
+  setUp(() {
+    logger = BufferLogger(
+      outputPreferences: OutputPreferences.test(),
+      terminal: AnsiTerminal(
+        platform: const LocalPlatform(),
+        stdio: null,
+      ),
+    );
+    fileSystem = const LocalFileSystemBlockingSetCurrentDirectory();
+    processManager = const LocalProcessManager();
+    parser = PlistParser(
+      fileSystem: fileSystem,
+      processManager: processManager,
+      logger: logger,
+    );
+    file = fileSystem.file('foo.plist')..createSync();
+  });
+
+  tearDown(() {
+    file.deleteSync();
   });
+
+  testWithoutContext('PlistParser.getValueFromFile works with xml file', () {
+    file.writeAsBytesSync(base64.decode(base64PlistXml));
+
+    expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
+    expect(parser.getValueFromFile(file.absolute.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
+    expect(logger.statusText, isEmpty);
+    expect(logger.errorText, isEmpty);
+  }, skip: !io.Platform.isMacOS);
+
+  testWithoutContext('PlistParser.getValueFromFile works with binary file', () {
+    file.writeAsBytesSync(base64.decode(base64PlistBinary));
+
+    expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
+    expect(parser.getValueFromFile(file.absolute.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
+    expect(logger.statusText, isEmpty);
+    expect(logger.errorText, isEmpty);
+  }, skip: !io.Platform.isMacOS);
+
+  testWithoutContext('PlistParser.getValueFromFile works with json file', () {
+    file.writeAsBytesSync(base64.decode(base64PlistJson));
+
+    expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
+    expect(parser.getValueFromFile(file.absolute.path, 'CFBundleIdentifier'), 'io.flutter.flutter.app');
+    expect(logger.statusText, isEmpty);
+    expect(logger.errorText, isEmpty);
+  }, skip: !io.Platform.isMacOS);
+
+  testWithoutContext('PlistParser.getValueFromFile returns null for non-existent plist file', () {
+    expect(parser.getValueFromFile('missing.plist', 'CFBundleIdentifier'), null);
+    expect(logger.statusText, isEmpty);
+    expect(logger.errorText, isEmpty);
+  }, skip: !io.Platform.isMacOS);
+
+  testWithoutContext('PlistParser.getValueFromFile returns null for non-existent key within plist', () {
+    file.writeAsBytesSync(base64.decode(base64PlistXml));
+
+    expect(parser.getValueFromFile(file.path, 'BadKey'), null);
+    expect(parser.getValueFromFile(file.absolute.path, 'BadKey'), null);
+    expect(logger.statusText, isEmpty);
+    expect(logger.errorText, isEmpty);
+  }, skip: !io.Platform.isMacOS);
+
+  testWithoutContext('PlistParser.getValueFromFile returns null for malformed plist file', () {
+    file.writeAsBytesSync(const <int>[1, 2, 3, 4, 5, 6]);
+
+    expect(parser.getValueFromFile(file.path, 'CFBundleIdentifier'), null);
+    expect(logger.statusText, isNotEmpty);
+    expect(logger.errorText, isEmpty);
+  }, skip: !io.Platform.isMacOS);
+
+  testWithoutContext('PlistParser.getValueFromFile throws when /usr/bin/plutil is not found', () async {
+    expect(
+      () => parser.getValueFromFile('irrelevant.plist', 'ununsed'),
+      throwsA(isA<FileNotFoundException>()),
+    );
+    expect(logger.statusText, isEmpty);
+    expect(logger.errorText, isEmpty);
+  }, skip: io.Platform.isMacOS);
 }

packages/flutter_tools/test/general.shard/ios/simulators_test.dart

@@ -498,7 +498,7 @@ void main() {
 
     testUsingContext("startApp uses compiled app's Info.plist to find CFBundleIdentifier", () async {
       final IOSSimulator device = IOSSimulator('x', name: 'iPhone SE', simulatorCategory: 'iOS 11.2');
-      when(PlistParser.instance.getValueFromFile(any, any)).thenReturn('correct');
+      when(globals.plistParser.getValueFromFile(any, any)).thenReturn('correct');
 
       final Directory mockDir = globals.fs.currentDirectory;
       final IOSApp package = PrebuiltIOSApp(projectBundleId: 'incorrect', bundleName: 'name', bundleDir: mockDir);