From 136e772f7753ab77c60e462f6e945778d9fff64f Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Mon, 27 Jan 2025 15:43:51 -0800 Subject: [PATCH] [tool] Refactor args of strings or YAML file lists --- script/configs/README.md | 2 - .../dart_unit_tests_wasm_exceptions.yaml | 2 - .../exclude_all_packages_app_wasm.yaml | 2 - .../configs/exclude_integration_web_wasm.yaml | 2 - script/tool/lib/src/analyze_command.dart | 18 ++------ .../tool/lib/src/common/package_command.dart | 42 +++++++++---------- .../tool/lib/src/pubspec_check_command.dart | 20 +-------- .../test/common/package_command_test.dart | 18 ++++++++ 8 files changed, 44 insertions(+), 62 deletions(-) diff --git a/script/configs/README.md b/script/configs/README.md index 73691a0bbda..42c241966ca 100644 --- a/script/configs/README.md +++ b/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 []. \ No newline at end of file diff --git a/script/configs/dart_unit_tests_wasm_exceptions.yaml b/script/configs/dart_unit_tests_wasm_exceptions.yaml index 9978ed364e9..2f0782f0986 100644 --- a/script/configs/dart_unit_tests_wasm_exceptions.yaml +++ b/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`! diff --git a/script/configs/exclude_all_packages_app_wasm.yaml b/script/configs/exclude_all_packages_app_wasm.yaml index 7c503b36fbd..cb7fa4a9cdd 100644 --- a/script/configs/exclude_all_packages_app_wasm.yaml +++ b/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`! diff --git a/script/configs/exclude_integration_web_wasm.yaml b/script/configs/exclude_integration_web_wasm.yaml index 8172d521699..af0b87b6e65 100644 --- a/script/configs/exclude_integration_web_wasm.yaml +++ b/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`! diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 2f7d28217ab..ec36d830b14 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/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 files = package.directory.listSync(recursive: true, followLinks: false); for (final FileSystemEntity file in files) { @@ -94,18 +93,7 @@ class AnalyzeCommand extends PackageLoopingCommand { @override Future initializeRun() async { - _allowedCustomAnalysisDirectories = - getStringListArg(_customAnalysisFlag).expand((String item) { - if (item.endsWith('.yaml')) { - final File file = packagesDir.fileSystem.file(item); - final Object? yaml = loadYaml(file.readAsStringSync()); - if (yaml == null) { - return []; - } - return (yaml as YamlList).toList().cast(); - } - return [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(['Unexpected local analysis options']); } final int exitCode = await processRunner.runAndStream(_dartBinaryPath, diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index 8b3b02626fe..e002d22b225 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -77,7 +77,7 @@ abstract class PackageCommand extends Command { 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: [], @@ -252,6 +252,22 @@ abstract class PackageCommand extends Command { return List.from(argResults![key] as List? ?? []); } + /// Convenience accessor for arguments containing a list of strings and/or + /// YAML files containing lists of strings. + Set getYamlListArg(String key) { + return getStringListArg(key).expand((String item) { + if (item.endsWith('.yaml')) { + final File file = packagesDir.fileSystem.file(item); + final Object? yaml = loadYaml(file.readAsStringSync()); + if (yaml == null) { + return const []; + } + return (yaml as YamlList).toList().cast(); + } + return [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 { _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 _expandYamlInPackageList(List items) { - return items.expand((String item) { - if (item.endsWith('.yaml')) { - final File file = packagesDir.fileSystem.file(item); - return (loadYaml(file.readAsStringSync()) as YamlList) - .toList() - .cast(); - } - return [item]; - }).toSet(); - } - /// Returns the set of packages to exclude based on the `--exclude` argument. Set getExcludedPackageNames() { - final Set excludedPackages = _excludedPackages ?? - _expandYamlInPackageList(getStringListArg(_excludeArg)); + final Set excludedPackages = + _excludedPackages ?? getYamlListArg(_excludeArg); // Cache for future calls. _excludedPackages = excludedPackages; return excludedPackages; @@ -460,9 +461,8 @@ abstract class PackageCommand extends Command { final Set excludedPackageNames = getExcludedPackageNames(); final bool hasFilter = argResults?.wasParsed(_filterPackagesArg) ?? false; - final Set? excludeAllButPackageNames = hasFilter - ? _expandYamlInPackageList(getStringListArg(_filterPackagesArg)) - : null; + final Set? excludeAllButPackageNames = + hasFilter ? getYamlListArg(_filterPackagesArg) : null; if (excludeAllButPackageNames != null && excludeAllButPackageNames.isNotEmpty) { final List sortedList = excludeAllButPackageNames.toList() diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index db6e7d0dc79..65143f770a1 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/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 _getAllowedPackages(String flag) { - return getStringListArg(flag).expand((String item) { - if (item.endsWith('.yaml')) { - final File file = packagesDir.fileSystem.file(item); - final Object? yaml = loadYaml(file.readAsStringSync()); - if (yaml == null) { - return []; - } - return (yaml as YamlList).toList().cast(); - } - return [item]; - }); + _allowedUnpinnedPackages.addAll(getYamlListArg(_allowDependenciesFlag)); + _allowedPinnedPackages.addAll(getYamlListArg(_allowPinnedDependenciesFlag)); } @override diff --git a/script/tool/test/common/package_command_test.dart b/script/tool/test/common/package_command_test.dart index 112388642f9..099efee6b2e 100644 --- a/script/tool/test/common/package_command_test.dart +++ b/script/tool/test/common/package_command_test.dart @@ -204,6 +204,24 @@ void main() { expect(command.plugins, unorderedEquals([])); }); + 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, [ + 'sample', + '--packages=plugin1', + '--exclude=${configFile1.path},${configFile2.path},${configFile3.path}', + ]); + expect(command.plugins, unorderedEquals([plugin1.path])); + }); + test('filter-packages-to accepts config files', () async { final RepositoryPackage plugin1 = createFakePlugin('plugin1', packagesDir);