[tool] Refactor args of strings or YAML file lists (#8513)

Multiple commands in the Flutter plugin tool have arguments that accepts a list of strings or a list of YAML files that contain a list of strings. This introduces a `getYamlListArg` helper so that this logic doesn't need to be duplicated multiple times. 

In a subsequent pull requests, this will be used to add a list of packages that are allowed to have Xcode warnings. This will be used by the google_sign_in_ios package which will have warnings when using SwiftPM.

Part of: flutter/flutter#146904

Author: loic-sharma

script/configs/README.md

@@ -12,5 +12,3 @@ Expected format:
 # Reason for exclusion
 - name_of_package
 ```
-
-If there are no exclusions there should be an file with [].

script/configs/dart_unit_tests_wasm_exceptions.yaml

@@ -6,5 +6,3 @@
 #
 # This is only used for the rare case where a package fails in Wasm, but works
 # in JS mode.
-
-[] # Needed so the contents of this file are an empty array, not `null`!

script/configs/exclude_all_packages_app_wasm.yaml

@@ -4,5 +4,3 @@
 #
 # This is only used for the rare case where a package fails in Wasm, but works
 # in JS mode.
-
-[] # Needed so the contents of this file are an empty array, not `null`!

script/configs/exclude_integration_web_wasm.yaml

@@ -4,5 +4,3 @@
 #
 # This is only used for the rare case where a package fails in Wasm, but works
 # in JS mode.
-
-[] # Needed so the contents of this file are an empty array, not `null`!

script/tool/lib/src/analyze_command.dart

@@ -5,7 +5,6 @@
 import 'dart:io' as io;
 
 import 'package:file/file.dart';
-import 'package:yaml/yaml.dart';
 
 import 'common/output_utils.dart';
 import 'common/package_looping_command.dart';
@@ -64,7 +63,7 @@ class AnalyzeCommand extends PackageLoopingCommand {
   final bool hasLongOutput = false;
 
   /// Checks that there are no unexpected analysis_options.yaml files.
-  bool _hasUnexpecetdAnalysisOptions(RepositoryPackage package) {
+  bool _hasUnexpectedAnalysisOptions(RepositoryPackage package) {
     final List<FileSystemEntity> files =
         package.directory.listSync(recursive: true, followLinks: false);
     for (final FileSystemEntity file in files) {
@@ -94,18 +93,7 @@ class AnalyzeCommand extends PackageLoopingCommand {
 
   @override
   Future<void> initializeRun() async {
-    _allowedCustomAnalysisDirectories =
-        getStringListArg(_customAnalysisFlag).expand<String>((String item) {
-      if (item.endsWith('.yaml')) {
-        final File file = packagesDir.fileSystem.file(item);
-        final Object? yaml = loadYaml(file.readAsStringSync());
-        if (yaml == null) {
-          return <String>[];
-        }
-        return (yaml as YamlList).toList().cast<String>();
-      }
-      return <String>[item];
-    }).toSet();
+    _allowedCustomAnalysisDirectories = getYamlListArg(_customAnalysisFlag);
 
     // Use the Dart SDK override if one was passed in.
     final String? dartSdk = argResults![_analysisSdk] as String?;
@@ -161,7 +149,7 @@ class AnalyzeCommand extends PackageLoopingCommand {
       }
     }
 
-    if (_hasUnexpecetdAnalysisOptions(package)) {
+    if (_hasUnexpectedAnalysisOptions(package)) {
       return PackageResult.fail(<String>['Unexpected local analysis options']);
     }
     final int exitCode = await processRunner.runAndStream(_dartBinaryPath,

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

@@ -77,7 +77,7 @@ abstract class PackageCommand extends Command<void> {
     argParser.addMultiOption(
       _excludeArg,
       abbr: 'e',
-      help: 'A list of packages to exclude from from this command.\n\n'
+      help: 'A list of packages to exclude from this command.\n\n'
           'Alternately, a list of one or more YAML files that contain a list '
           'of packages to exclude.',
       defaultsTo: <String>[],
@@ -252,6 +252,22 @@ abstract class PackageCommand extends Command<void> {
     return List<String>.from(argResults![key] as List<String>? ?? <String>[]);
   }
 
+  /// Convenience accessor for arguments containing a list of strings and/or
+  /// YAML files containing lists of strings.
+  Set<String> getYamlListArg(String key) {
+    return getStringListArg(key).expand<String>((String item) {
+      if (item.endsWith('.yaml')) {
+        final File file = packagesDir.fileSystem.file(item);
+        final Object? yaml = loadYaml(file.readAsStringSync());
+        if (yaml == null) {
+          return const <String>[];
+        }
+        return (yaml as YamlList).toList().cast<String>();
+      }
+      return <String>[item];
+    }).toSet();
+  }
+
   /// If true, commands should log timing information that might be useful in
   /// analyzing their runtime (e.g., the per-package time for multi-package
   /// commands).
@@ -277,25 +293,10 @@ abstract class PackageCommand extends Command<void> {
     _shardCount = shardCount;
   }
 
-  /// Converts a list of items which are either package names or yaml files
-  /// containing a list of package names to a flat list of package names by
-  /// reading all the file contents.
-  Set<String> _expandYamlInPackageList(List<String> items) {
-    return items.expand<String>((String item) {
-      if (item.endsWith('.yaml')) {
-        final File file = packagesDir.fileSystem.file(item);
-        return (loadYaml(file.readAsStringSync()) as YamlList)
-            .toList()
-            .cast<String>();
-      }
-      return <String>[item];
-    }).toSet();
-  }
-
   /// Returns the set of packages to exclude based on the `--exclude` argument.
   Set<String> getExcludedPackageNames() {
-    final Set<String> excludedPackages = _excludedPackages ??
-        _expandYamlInPackageList(getStringListArg(_excludeArg));
+    final Set<String> excludedPackages =
+        _excludedPackages ?? getYamlListArg(_excludeArg);
     // Cache for future calls.
     _excludedPackages = excludedPackages;
     return excludedPackages;
@@ -460,9 +461,8 @@ abstract class PackageCommand extends Command<void> {
 
     final Set<String> excludedPackageNames = getExcludedPackageNames();
     final bool hasFilter = argResults?.wasParsed(_filterPackagesArg) ?? false;
-    final Set<String>? excludeAllButPackageNames = hasFilter
-        ? _expandYamlInPackageList(getStringListArg(_filterPackagesArg))
-        : null;
+    final Set<String>? excludeAllButPackageNames =
+        hasFilter ? getYamlListArg(_filterPackagesArg) : null;
     if (excludeAllButPackageNames != null &&
         excludeAllButPackageNames.isNotEmpty) {
       final List<String> sortedList = excludeAllButPackageNames.toList()

script/tool/lib/src/pubspec_check_command.dart

@@ -115,24 +115,8 @@ class PubspecCheckCommand extends PackageLoopingCommand {
       }
     }
     // Load explicitly allowed packages.
-    _allowedUnpinnedPackages
-        .addAll(_getAllowedPackages(_allowDependenciesFlag));
-    _allowedPinnedPackages
-        .addAll(_getAllowedPackages(_allowPinnedDependenciesFlag));
-  }
-
-  Iterable<String> _getAllowedPackages(String flag) {
-    return getStringListArg(flag).expand<String>((String item) {
-      if (item.endsWith('.yaml')) {
-        final File file = packagesDir.fileSystem.file(item);
-        final Object? yaml = loadYaml(file.readAsStringSync());
-        if (yaml == null) {
-          return <String>[];
-        }
-        return (yaml as YamlList).toList().cast<String>();
-      }
-      return <String>[item];
-    });
+    _allowedUnpinnedPackages.addAll(getYamlListArg(_allowDependenciesFlag));
+    _allowedPinnedPackages.addAll(getYamlListArg(_allowPinnedDependenciesFlag));
   }
 
   @override

script/tool/test/common/package_command_test.dart

@@ -204,6 +204,24 @@ void main() {
       expect(command.plugins, unorderedEquals(<String>[]));
     });
 
+    test('exclude accepts empty config files', () async {
+      final RepositoryPackage plugin1 =
+          createFakePlugin('plugin1', packagesDir);
+      final File configFile1 = packagesDir.childFile('exclude1.yaml');
+      configFile1.createSync();
+      final File configFile2 = packagesDir.childFile('exclude2.yaml');
+      configFile2.writeAsStringSync('\n');
+      final File configFile3 = packagesDir.childFile('exclude3.yaml');
+      configFile3.writeAsStringSync('# - plugin1');
+
+      await runCapturingPrint(runner, <String>[
+        'sample',
+        '--packages=plugin1',
+        '--exclude=${configFile1.path},${configFile2.path},${configFile3.path}',
+      ]);
+      expect(command.plugins, unorderedEquals(<String>[plugin1.path]));
+    });
+
     test('filter-packages-to accepts config files', () async {
       final RepositoryPackage plugin1 =
           createFakePlugin('plugin1', packagesDir);