diff --git a/lib/src/commands/packages/commands/check/commands/licenses.dart b/lib/src/commands/packages/commands/check/commands/licenses.dart index 32989abea..de3c5eb0c 100644 --- a/lib/src/commands/packages/commands/check/commands/licenses.dart +++ b/lib/src/commands/packages/commands/check/commands/licenses.dart @@ -20,13 +20,23 @@ import 'package:package_config/package_config.dart' as package_config; import 'package:pana/src/license_detection/license_detector.dart' as detector; import 'package:path/path.dart' as path; import 'package:very_good_cli/src/pub_license/spdx_license.gen.dart'; +import 'package:very_good_cli/src/pubspec/pubspec.dart'; import 'package:very_good_cli/src/pubspec_lock/pubspec_lock.dart'; +import 'package:very_good_cli/src/pubspec_workspace/pubspec_workspace.dart'; /// Overrides the [package_config.findPackageConfig] function for testing. @visibleForTesting Future Function(Directory directory)? findPackageConfigOverride; +/// Overrides the [resolveWorkspaceDependencies] function for testing. +@visibleForTesting +Map? Function( + Directory directory, { + required Logger logger, +})? +resolveWorkspaceOverride; + /// Overrides the [detector.detectLicense] function for testing. @visibleForTesting Future Function(String, double)? detectLicenseOverride; @@ -181,7 +191,15 @@ class PackagesCheckLicensesCommand extends Command { final pubspecLockFile = File(path.join(targetPath, pubspecLockBasename)); if (!pubspecLockFile.existsSync()) { progress.cancel(); - _logger.err('Could not find a $pubspecLockBasename in $targetPath'); + if (declaresWorkspaceResolution(targetDirectory)) { + _logger.err( + 'Could not find a $pubspecLockBasename in $targetPath.\n' + 'This package resolves as part of a Pub workspace. ' + 'Run the command from the workspace root instead.', + ); + } else { + _logger.err('Could not find a $pubspecLockBasename in $targetPath'); + } return ExitCode.noInput.code; } @@ -192,21 +210,26 @@ class PackagesCheckLicensesCommand extends Command { return ExitCode.noInput.code; } + final resolveWorkspace = + resolveWorkspaceOverride ?? resolveWorkspaceDependencies; + final workspaceDeps = resolveWorkspace(targetDirectory, logger: _logger); + final filteredDependencies = pubspecLock.packages.where((dependency) { if (!dependency.isPubHosted) return false; if (skippedPackages.contains(dependency.name)) return false; - final dependencyType = dependency.type; + final dependencyType = workspaceDeps == null + ? dependency.type + : workspaceDeps[dependency.name] ?? PubspecDependencyType.transitive; return (dependencyTypes.contains('direct-main') && - dependencyType == PubspecLockPackageDependencyType.directMain) || + dependencyType == PubspecDependencyType.directMain) || (dependencyTypes.contains('direct-dev') && - dependencyType == PubspecLockPackageDependencyType.directDev) || + dependencyType == PubspecDependencyType.directDev) || (dependencyTypes.contains('transitive') && - dependencyType == PubspecLockPackageDependencyType.transitive) || + dependencyType == PubspecDependencyType.transitive) || (dependencyTypes.contains('direct-overridden') && - dependencyType == - PubspecLockPackageDependencyType.directOverridden); + dependencyType == PubspecDependencyType.directOverridden); }); if (filteredDependencies.isEmpty) { diff --git a/lib/src/pubspec/pubspec.dart b/lib/src/pubspec/pubspec.dart new file mode 100644 index 000000000..2ab7aafc0 --- /dev/null +++ b/lib/src/pubspec/pubspec.dart @@ -0,0 +1,87 @@ +/// Shared pubspec-domain primitives built on top of `package:pubspec_parse`. +/// +/// The `packages check licenses` command reads dependency information from two +/// different sources: a `pubspec.lock` file (via `pubspec_lock.dart`) and the +/// `pubspec.yaml` files of a Pub workspace (via `pubspec_workspace.dart`). Both +/// classify dependencies with the same model, so that model lives here — a +/// common ancestor both import — instead of being duplicated across the two +/// sibling parsers. +library; + +import 'dart:io'; +import 'package:pubspec_parse/pubspec_parse.dart'; + +export 'package:pubspec_parse/pubspec_parse.dart'; + +/// {@template pubspec_dependency_type} +/// The classification of a package dependency. +/// {@endtemplate} +enum PubspecDependencyType { + /// Another package that your package needs to work. + /// + /// See also: + /// + /// * [Dart's dependency documentation](https://dart.dev/tools/pub/dependencies) + directMain._('direct main'), + + /// Another package that your package needs during development. + /// + /// See also: + /// + /// * [Dart's developer dependency documentation](https://dart.dev/tools/pub/dependencies#dev-dependencies) + directDev._('direct dev'), + + /// A dependency that your package indirectly uses because one of its + /// dependencies requires it. + /// + /// See also: + /// + /// * [Dart's transitive dependency documentation](https://dart.dev/tools/pub/glossary#transitive-) + transitive._('transitive'), + + /// A dependency that your package overrides that is not already a + /// `direct main` or `direct dev` dependency. + /// + /// See also: + /// + /// * [Dart's dependency override documentation](https://dart.dev/tools/pub/dependencies#dependency-overrides) + directOverridden._('direct overridden'); + + const PubspecDependencyType._(this.value); + + /// Parses a [PubspecDependencyType] from its `pubspec.lock` textual form. + /// + /// Throws an [ArgumentError] if the string is not a valid dependency type. + factory PubspecDependencyType.parse(String value) { + if (_valueMap.containsKey(value)) return _valueMap[value]!; + + throw ArgumentError.value( + value, + 'value', + 'Invalid PubspecDependencyType value.', + ); + } + + static final Map _valueMap = { + for (final type in PubspecDependencyType.values) type.value: type, + }; + + /// The textual representation of the [PubspecDependencyType] as it appears in + /// the `dependency` field of a `pubspec.lock` file. + final String value; +} + +/// Tolerantly parses a [Pubspec] from [pubspecFile]. +/// +/// Returns `null` when the file does not exist or cannot be parsed. Parsing is +/// lenient so valid-but-unmodeled keys (e.g. a `flutter:` block) do not throw. +Pubspec? tryParsePubspec(File pubspecFile) { + if (!pubspecFile.existsSync()) return null; + try { + return Pubspec.parse(pubspecFile.readAsStringSync(), lenient: true); + // Tolerate any malformed pubspec by returning null instead of throwing. + // ignore: avoid_catches_without_on_clauses + } catch (_) { + return null; + } +} diff --git a/lib/src/pubspec_lock/pubspec_lock.dart b/lib/src/pubspec_lock/pubspec_lock.dart index 3ea78bd15..4f30d8c9a 100644 --- a/lib/src/pubspec_lock/pubspec_lock.dart +++ b/lib/src/pubspec_lock/pubspec_lock.dart @@ -9,6 +9,7 @@ library; import 'dart:collection'; import 'package:equatable/equatable.dart'; +import 'package:very_good_cli/src/pubspec/pubspec.dart'; import 'package:yaml/yaml.dart'; /// {@template PubspecLockParseException} @@ -88,7 +89,7 @@ class PubspecLockPackage extends Equatable { required YamlMap data, }) { final dependency = data['dependency'] as String; - final dependencyType = PubspecLockPackageDependencyType.parse(dependency); + final dependencyType = PubspecDependencyType.parse(dependency); final source = data['source'] as String; late final bool isPubHosted; @@ -110,8 +111,8 @@ class PubspecLockPackage extends Equatable { /// The name of the dependency. final String name; - /// {@macro PubspecLockDependencyType} - final PubspecLockPackageDependencyType type; + /// {@macro pubspec_dependency_type} + final PubspecDependencyType type; /// Whether the dependency is hosted on pub.dev or not. final bool isPubHosted; @@ -119,64 +120,3 @@ class PubspecLockPackage extends Equatable { @override List get props => [type, isPubHosted]; } - -/// {@template PubspecLockDependencyType} -/// The type of a [PubspecLockPackage]. -/// {@endtemplate} -enum PubspecLockPackageDependencyType { - /// Another package that your package needs to work. - /// - /// See also: - /// - /// * [Dart's dependency documentation](https://dart.dev/tools/pub/dependencies) - directMain._('direct main'), - - /// Another package that your package needs during development. - /// - /// See also: - /// - /// * [Dart's developer dependency documentation](https://dart.dev/tools/pub/dependencies#dev-dependencies) - directDev._('direct dev'), - - /// A dependency that your package indirectly uses because one of its - /// dependencies requires it. - /// - /// See also: - /// - /// * [Dart's transitive dependency documentation](https://dart.dev/tools/pub/glossary#transitive-) - transitive._('transitive'), - - /// A dependency that your package overrides that is not already a - /// `direct main` or `direct dev` dependency. - /// - /// See also: - /// - /// * [Dart's dependency override documentation](https://dart.dev/tools/pub/dependencies#dependency-overrides) - directOverridden._('direct overridden'); - - const PubspecLockPackageDependencyType._(this.value); - - /// Parses a [PubspecLockPackageDependencyType] from a string. - /// - /// Throws an [ArgumentError] if the string is not a valid dependency type. - factory PubspecLockPackageDependencyType.parse(String value) { - if (_valueMap.containsKey(value)) { - return _valueMap[value]!; - } - - throw ArgumentError.value( - value, - 'value', - 'Invalid PubspecLockPackageDependencyType value.', - ); - } - - static Map _valueMap = { - for (final type in PubspecLockPackageDependencyType.values) - type.value: type, - }; - - /// The string representation of the [PubspecLockPackageDependencyType] - /// as it appears in a pubspec.lock file. - final String value; -} diff --git a/lib/src/pubspec_workspace/pubspec_workspace.dart b/lib/src/pubspec_workspace/pubspec_workspace.dart new file mode 100644 index 000000000..4627fa748 --- /dev/null +++ b/lib/src/pubspec_workspace/pubspec_workspace.dart @@ -0,0 +1,159 @@ +/// A tolerant resolver for Pub workspace dependency classification. +/// +/// This is used by the `packages check licenses` command. In a Pub workspace, +/// member packages share a single `pubspec.lock` at the workspace root, and +/// that lock classifies every member's direct dependency as `transitive` (only +/// the root package's own dependencies are classified relative to it). As a +/// result, running the command at the workspace root reports no direct +/// dependencies. +/// +/// This resolver rebuilds the correct classification by unioning the +/// directly-declared dependencies across the root and every member +/// `pubspec.yaml`. It mirrors the shape and philosophy of `pubspec_lock.dart`: +/// a small, tolerant, single-purpose parser. It is not a general workspace +/// model. +library; + +import 'dart:io'; + +import 'package:glob/glob.dart'; +import 'package:glob/list_local_fs.dart'; +import 'package:mason_logger/mason_logger.dart'; +import 'package:path/path.dart' as path; +import 'package:very_good_cli/src/pubspec/pubspec.dart'; + +/// The basename of a pubspec file. +const _pubspecBasename = 'pubspec.yaml'; + +/// Resolves the directly-declared dependencies across the Pub workspace rooted +/// at [rootDirectory], mapping each dependency name to its workspace-wide +/// [PubspecDependencyType]. +/// +/// Walks the root and every member `pubspec.yaml` (recursively following nested +/// `workspace:` lists and glob entries) and unions their declared dependencies. +/// Precedence when a name appears under multiple types across members: +/// `directMain` > `directDev` > `directOverridden` (approximates pub's +/// precedence). Names not directly declared by any member are absent from the +/// map; the caller treats an absent name as +/// [PubspecDependencyType.transitive]. +/// +/// Returns `null` when [rootDirectory] has no readable workspace-root pubspec +/// (missing pubspec, or no non-empty `workspace:` list) — the caller then falls +/// back to the lock's own classification (non-workspace behavior). A present +/// but unparseable root pubspec logs a warning via [logger] before returning +/// `null`. [logger] also receives a warning for every skipped member. +Map? resolveWorkspaceDependencies( + Directory rootDirectory, { + required Logger logger, +}) { + final rootPubspecFile = File( + path.join(rootDirectory.path, _pubspecBasename), + ); + + // A missing pubspec is the normal non-workspace case: fall back silently to + // the lock's own classification. + if (!rootPubspecFile.existsSync()) return null; + + final rootPubspec = tryParsePubspec(rootPubspecFile); + if (rootPubspec == null) { + logger.warn( + '''Could not parse the workspace-root $_pubspecBasename in ${rootDirectory.path}. Falling back to the lock file classification.''', + ); + return null; + } + + final workspace = rootPubspec.workspace; + if (workspace == null || workspace.isEmpty) return null; + + final visited = {}; + final directDev = {}; + final directMain = {}; + final directOverridden = {}; + + void visit(Directory directory, Pubspec pubspec) { + if (!visited.add(directory.resolveSymbolicLinksSync())) return; + + directMain.addAll(pubspec.dependencies.keys); + directDev.addAll(pubspec.devDependencies.keys); + directOverridden.addAll(pubspec.dependencyOverrides.keys); + + for (final entry in pubspec.workspace ?? const []) { + for (final memberDirectory in _expandMembers(directory, entry, logger)) { + final memberPubspec = tryParsePubspec( + File(path.join(memberDirectory.path, _pubspecBasename)), + ); + if (memberPubspec == null) { + logger.warn( + '''Skipping workspace member at ${memberDirectory.path}: missing or unparseable $_pubspecBasename.''', + ); + continue; + } + visit(memberDirectory, memberPubspec); + } + } + } + + visit(rootDirectory, rootPubspec); + + // Build highest precedence first so lower-precedence writes of the same name + // are no-ops: directMain > directDev > directOverridden. + final dependencies = {}; + + for (final name in directMain) { + dependencies[name] = PubspecDependencyType.directMain; + } + + for (final name in directDev) { + dependencies.putIfAbsent(name, () => PubspecDependencyType.directDev); + } + + for (final name in directOverridden) { + dependencies.putIfAbsent( + name, + () => PubspecDependencyType.directOverridden, + ); + } + + return dependencies; +} + +/// Whether the package rooted at [directory] declares `resolution: workspace`, +/// indicating it is a member of a Pub workspace and must have its licenses +/// checked from the workspace root instead. +bool declaresWorkspaceResolution(Directory directory) { + final pubspec = tryParsePubspec( + File(path.join(directory.path, _pubspecBasename)), + ); + return pubspec?.resolution == 'workspace'; +} + +/// Expands a single `workspace:` [entry] relative to [base] into the member +/// directories it matches. +/// +/// A literal path is the no-wildcard case of a glob, so one code path covers +/// both. Only existing directories are returned. When nothing matches, [logger] +/// receives a warning and an empty iterable is returned. +Iterable _expandMembers( + Directory base, + String entry, + Logger logger, +) { + List matches; + try { + matches = Glob(entry).listSync(root: base.path); + // A missing intermediate directory (e.g. `packages/*` when `packages/` + // does not exist) surfaces as a FileSystemException; treat it as no match. + } on FileSystemException { + matches = const []; + } + + final directories = matches.whereType().toList(); + if (directories.isEmpty) { + logger.warn( + '''No workspace member directory matched "$entry" (resolved from ${base.path}).''', + ); + return const []; + } + + return directories; +} diff --git a/pubspec.yaml b/pubspec.yaml index 5883f57b0..c2e236bbc 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -26,7 +26,7 @@ dependencies: pana: ^0.23.0 path: ^1.9.0 pub_updater: ^0.5.0 - pubspec_parse: ^1.3.0 + pubspec_parse: ^1.5.0 stack_trace: ^1.11.1 stream_channel: ^2.1.4 universal_io: ^2.2.2 diff --git a/test/src/commands/packages/commands/check/commands/licenses_test.dart b/test/src/commands/packages/commands/check/commands/licenses_test.dart index 38d6e727f..c36f25147 100644 --- a/test/src/commands/packages/commands/check/commands/licenses_test.dart +++ b/test/src/commands/packages/commands/check/commands/licenses_test.dart @@ -14,6 +14,7 @@ import 'package:pana/src/license_detection/license_detector.dart' as detector; import 'package:path/path.dart' as path; import 'package:test/test.dart'; import 'package:very_good_cli/src/commands/packages/commands/check/commands/commands.dart'; +import 'package:very_good_cli/src/pubspec/pubspec.dart'; import '../../../../../../helpers/helpers.dart'; @@ -92,6 +93,8 @@ void main() { findPackageConfigOverride = (_) async => packageConfig; addTearDown(() => findPackageConfigOverride = null); + addTearDown(() => resolveWorkspaceOverride = null); + tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -1754,6 +1757,195 @@ and limitations under the License.'''); }), ); }); + + group('workspace', () { + test( + '''reclassifies member dependencies and reports them at the root''', + withRunner((commandRunner, logger, pubUpdater, printLogs) async { + File( + path.join(tempDirectory.path, 'pubspec.yaml'), + ).writeAsStringSync(_workspaceRootPubspecContent); + File( + path.join(tempDirectory.path, 'app', 'pubspec.yaml'), + ) + ..createSync(recursive: true) + ..writeAsStringSync(_appMemberPubspecContent); + File( + path.join( + tempDirectory.path, + 'packages', + 'pkg_a', + 'pubspec.yaml', + ), + ) + ..createSync(recursive: true) + ..writeAsStringSync(_pkgAMemberPubspecContent); + File( + path.join(tempDirectory.path, pubspecLockBasename), + ).writeAsStringSync(_workspacePubspecLockContent); + + when(() => packageConfig.packages).thenReturn({ + veryGoodTestRunnerConfigPackage, + cliCompletionConfigPackage, + }); + when(() => detectorResult.matches).thenReturn([mitLicenseMatch]); + when(() => logger.progress(any())).thenReturn(progress); + + final result = await commandRunner.run([ + ...commandArguments, + tempDirectory.path, + ]); + + verify( + () => progress.complete( + '''Retrieved 2 licenses from 2 packages of type: MIT (2).''', + ), + ).called(1); + + expect(result, equals(ExitCode.success.code)); + }), + ); + + test( + '''lists lock transitives not claimed by any member under --dependency-type transitive''', + withRunner((commandRunner, logger, pubUpdater, printLogs) async { + File( + path.join(tempDirectory.path, 'pubspec.yaml'), + ).writeAsStringSync(_workspaceRootPubspecContent); + File( + path.join(tempDirectory.path, 'app', 'pubspec.yaml'), + ) + ..createSync(recursive: true) + ..writeAsStringSync(_appMemberPubspecContent); + File( + path.join( + tempDirectory.path, + 'packages', + 'pkg_a', + 'pubspec.yaml', + ), + ) + ..createSync(recursive: true) + ..writeAsStringSync(_pkgAMemberPubspecContent); + File( + path.join(tempDirectory.path, pubspecLockBasename), + ).writeAsStringSync(_workspacePubspecLockContent); + + when(() => packageConfig.packages).thenReturn({yamlConfigPackage}); + when(() => detectorResult.matches).thenReturn([mitLicenseMatch]); + when(() => logger.progress(any())).thenReturn(progress); + + final result = await commandRunner.run([ + ...commandArguments, + '--dependency-type', + 'transitive', + tempDirectory.path, + ]); + + verify( + () => progress.complete( + '''Retrieved 1 license from 1 package of type: MIT (1).''', + ), + ).called(1); + + expect(result, equals(ExitCode.success.code)); + }), + ); + + test( + '''warns and continues when a workspace entry points to a missing directory''', + withRunner((commandRunner, logger, pubUpdater, printLogs) async { + File( + path.join(tempDirectory.path, 'pubspec.yaml'), + ).writeAsStringSync(_missingMemberWorkspaceRootPubspecContent); + File( + path.join(tempDirectory.path, 'app', 'pubspec.yaml'), + ) + ..createSync(recursive: true) + ..writeAsStringSync(_appMemberPubspecContent); + File( + path.join(tempDirectory.path, pubspecLockBasename), + ).writeAsStringSync(_workspacePubspecLockContent); + + when( + () => packageConfig.packages, + ).thenReturn({veryGoodTestRunnerConfigPackage}); + when(() => detectorResult.matches).thenReturn([mitLicenseMatch]); + when(() => logger.progress(any())).thenReturn(progress); + + final result = await commandRunner.run([ + ...commandArguments, + tempDirectory.path, + ]); + + verify(() => logger.warn(any())).called(1); + verify( + () => progress.complete( + '''Retrieved 1 license from 1 package of type: MIT (1).''', + ), + ).called(1); + + expect(result, equals(ExitCode.success.code)); + }), + ); + + test( + '''uses the injected resolver override to reclassify dependencies''', + withRunner((commandRunner, logger, pubUpdater, printLogs) async { + File( + path.join(tempDirectory.path, pubspecLockBasename), + ).writeAsStringSync(_workspacePubspecLockContent); + + resolveWorkspaceOverride = (_, {required logger}) => { + 'very_good_test_runner': PubspecDependencyType.directMain, + }; + + when( + () => packageConfig.packages, + ).thenReturn({veryGoodTestRunnerConfigPackage}); + when(() => detectorResult.matches).thenReturn([mitLicenseMatch]); + when(() => logger.progress(any())).thenReturn(progress); + + final result = await commandRunner.run([ + ...commandArguments, + tempDirectory.path, + ]); + + verify( + () => progress.complete( + '''Retrieved 1 license from 1 package of type: MIT (1).''', + ), + ).called(1); + + expect(result, equals(ExitCode.success.code)); + }), + ); + + test( + '''shows workspace-root guidance when run inside a member with no lock''', + withRunner((commandRunner, logger, pubUpdater, printLogs) async { + File( + path.join(tempDirectory.path, 'pubspec.yaml'), + ).writeAsStringSync(_appMemberPubspecContent); + + when(() => logger.progress(any())).thenReturn(progress); + + final result = await commandRunner.run([ + ...commandArguments, + tempDirectory.path, + ]); + + final errorMessage = + 'Could not find a $pubspecLockBasename in ${tempDirectory.path}.\n' + 'This package resolves as part of a Pub workspace. ' + 'Run the command from the workspace root instead.'; + verify(() => logger.err(errorMessage)).called(1); + verify(() => progress.cancel()).called(1); + + expect(result, equals(ExitCode.noInput.code)); + }), + ); + }); }); } @@ -1870,3 +2062,79 @@ sdks: dart: ">=3.10.0 <4.0.0" '''; + +/// A workspace-root `pubspec.yaml` declaring two members and its own dev dep. +const _workspaceRootPubspecContent = ''' +name: workspace_root +environment: + sdk: ^3.11.0 +dev_dependencies: + very_good_analysis: ^5.1.0 +workspace: + - app + - packages/pkg_a +'''; + +/// A workspace-root `pubspec.yaml` with one valid member and one pointing to a +/// missing directory. +const _missingMemberWorkspaceRootPubspecContent = ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - does_not_exist +'''; + +/// A workspace member `pubspec.yaml` with a hosted direct dependency. +const _appMemberPubspecContent = ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + very_good_test_runner: ^0.1.2 +'''; + +/// A workspace member `pubspec.yaml` with a hosted direct dependency. +const _pkgAMemberPubspecContent = ''' +name: pkg_a +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + cli_completion: ^0.4.0 +'''; + +/// A shared workspace `pubspec.lock` where the members' direct dependencies are +/// marked `transitive`, as pub does at the workspace root. +const _workspacePubspecLockContent = ''' +packages: + very_good_test_runner: + dependency: transitive + description: + name: very_good_test_runner + sha256: "4d41e5d7677d259b9a1599c78645ac2d36bc2bd6ff7773507bcb0bab41417fe2" + url: "https://pub.dev" + source: hosted + version: "0.1.2" + cli_completion: + dependency: transitive + description: + name: cli_completion + sha256: "1e87700c029c77041d836e57f9016b5c90d353151c43c2ca0c36deaadc05aa3a" + url: "https://pub.dev" + source: hosted + version: "0.4.0" + yaml: + dependency: transitive + description: + name: yaml + sha256: "75769501ea3489fca56601ff33454fe45507ea3bfb014161abc3b43ae25989d5" + url: "https://pub.dev" + source: hosted + version: "3.1.2" +sdks: + dart: ">=3.11.0 <4.0.0" + +'''; diff --git a/test/src/pubspec/pubspec_test.dart b/test/src/pubspec/pubspec_test.dart new file mode 100644 index 000000000..505c198a7 --- /dev/null +++ b/test/src/pubspec/pubspec_test.dart @@ -0,0 +1,104 @@ +import 'dart:io'; + +import 'package:path/path.dart' as path; +import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:test/test.dart'; +import 'package:very_good_cli/src/pubspec/pubspec.dart'; + +void main() { + group('$PubspecDependencyType', () { + group('parse', () { + test('parses successfully `direct main`', () { + expect( + PubspecDependencyType.parse('direct main'), + equals(PubspecDependencyType.directMain), + ); + }); + + test('parses successfully `direct dev`', () { + expect( + PubspecDependencyType.parse('direct dev'), + equals(PubspecDependencyType.directDev), + ); + }); + + test('parses successfully `direct overridden`', () { + expect( + PubspecDependencyType.parse('direct overridden'), + equals(PubspecDependencyType.directOverridden), + ); + }); + + test('parses successfully `transitive`', () { + expect( + PubspecDependencyType.parse('transitive'), + equals(PubspecDependencyType.transitive), + ); + }); + + test('throws a $ArgumentError when type is invalid', () { + expect( + () => PubspecDependencyType.parse('invalid'), + throwsA(isA()), + ); + }); + }); + }); + + group('tryParsePubspec', () { + late Directory tempDirectory; + + setUp(() { + tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + }); + + File pubspecFile(String content) { + return File(path.join(tempDirectory.path, 'pubspec.yaml')) + ..writeAsStringSync(content); + } + + test('returns null when the file does not exist', () { + final file = File(path.join(tempDirectory.path, 'pubspec.yaml')); + + expect(tryParsePubspec(file), isNull); + }); + + test('returns null when the content cannot be parsed', () { + final file = pubspecFile('{{{ not valid yaml'); + + expect(tryParsePubspec(file), isNull); + }); + + test('parses a valid pubspec', () { + final file = pubspecFile(''' +name: example +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + + final pubspec = tryParsePubspec(file); + + expect(pubspec, isA()); + expect(pubspec!.name, equals('example')); + expect(pubspec.dependencies.keys, contains('path')); + }); + + test('tolerates valid-but-unmodeled keys', () { + final file = pubspecFile(''' +name: example +environment: + sdk: ^3.11.0 +flutter: + uses-material-design: true +'''); + + final pubspec = tryParsePubspec(file); + + expect(pubspec, isA()); + expect(pubspec!.name, equals('example')); + }); + }); +} diff --git a/test/src/pubspec_lock/pubspec_lock_test.dart b/test/src/pubspec_lock/pubspec_lock_test.dart index 52109a1ad..7026f5c20 100644 --- a/test/src/pubspec_lock/pubspec_lock_test.dart +++ b/test/src/pubspec_lock/pubspec_lock_test.dart @@ -3,6 +3,7 @@ // ignore_for_file: prefer_const_constructors import 'package:test/test.dart'; +import 'package:very_good_cli/src/pubspec/pubspec.dart'; import 'package:very_good_cli/src/pubspec_lock/pubspec_lock.dart'; void main() { @@ -16,32 +17,32 @@ void main() { equals([ PubspecLockPackage( name: 'very_good_test_runner', - type: PubspecLockPackageDependencyType.directMain, + type: PubspecDependencyType.directMain, isPubHosted: true, ), PubspecLockPackage( name: 'very_good_analysis', - type: PubspecLockPackageDependencyType.directDev, + type: PubspecDependencyType.directDev, isPubHosted: true, ), PubspecLockPackage( name: 'yaml', - type: PubspecLockPackageDependencyType.transitive, + type: PubspecDependencyType.transitive, isPubHosted: true, ), PubspecLockPackage( name: 'path', - type: PubspecLockPackageDependencyType.directOverridden, + type: PubspecDependencyType.directOverridden, isPubHosted: true, ), PubspecLockPackage( name: 'foo', - type: PubspecLockPackageDependencyType.directMain, + type: PubspecDependencyType.directMain, isPubHosted: false, ), PubspecLockPackage( name: 'yaml2', - type: PubspecLockPackageDependencyType.transitive, + type: PubspecDependencyType.transitive, isPubHosted: false, ), ]), @@ -67,7 +68,7 @@ void main() { expect( PubspecLockPackage( name: 'foo', - type: PubspecLockPackageDependencyType.directMain, + type: PubspecDependencyType.directMain, isPubHosted: true, ), isA(), @@ -77,17 +78,17 @@ void main() { test('supports value equality', () { final package1 = PubspecLockPackage( name: 'foo', - type: PubspecLockPackageDependencyType.directMain, + type: PubspecDependencyType.directMain, isPubHosted: true, ); final package2 = PubspecLockPackage( name: 'foo', - type: PubspecLockPackageDependencyType.directMain, + type: PubspecDependencyType.directMain, isPubHosted: true, ); final package3 = PubspecLockPackage( name: 'bar', - type: PubspecLockPackageDependencyType.transitive, + type: PubspecDependencyType.transitive, isPubHosted: false, ); @@ -96,45 +97,6 @@ void main() { expect(package2, isNot(equals(package3))); }); }); - - group('$PubspecLockPackageDependencyType', () { - group('parse', () { - test('parses successfully `direct main`', () { - expect( - PubspecLockPackageDependencyType.parse('direct main'), - equals(PubspecLockPackageDependencyType.directMain), - ); - }); - - test('parses successfully `direct dev`', () { - expect( - PubspecLockPackageDependencyType.parse('direct dev'), - equals(PubspecLockPackageDependencyType.directDev), - ); - }); - - test('parses successfully `direct overridden`', () { - expect( - PubspecLockPackageDependencyType.parse('direct overridden'), - equals(PubspecLockPackageDependencyType.directOverridden), - ); - }); - - test('parses successfully `transitive`', () { - expect( - PubspecLockPackageDependencyType.parse('transitive'), - equals(PubspecLockPackageDependencyType.transitive), - ); - }); - - test('throws a $ArgumentError when type is invalid', () { - expect( - () => PubspecLockPackageDependencyType.parse('invalid'), - throwsA(isA()), - ); - }); - }); - }); } /// An example pubspec.lock content used to test the [PubspecLock] class. diff --git a/test/src/pubspec_workspace/pubspec_workspace_test.dart b/test/src/pubspec_workspace/pubspec_workspace_test.dart new file mode 100644 index 000000000..55c2e1f77 --- /dev/null +++ b/test/src/pubspec_workspace/pubspec_workspace_test.dart @@ -0,0 +1,610 @@ +import 'dart:io'; + +import 'package:mason_logger/mason_logger.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; +import 'package:very_good_cli/src/pubspec/pubspec.dart'; +import 'package:very_good_cli/src/pubspec_workspace/pubspec_workspace.dart'; + +class _MockLogger extends Mock implements Logger {} + +void main() { + /// Writes a `pubspec.yaml` with [content] into a subdirectory [name] + /// (which maybe a nested path) of [root], creating directories as needed. + Directory writePubspec(Directory root, String name, String content) { + final directory = Directory( + path.join(root.path, name), + )..createSync(recursive: true); + File(path.join(directory.path, 'pubspec.yaml')).writeAsStringSync(content); + return directory; + } + + group('resolveWorkspaceDependencies', () { + late Directory tempDirectory; + late Logger logger; + + setUp(() { + tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + logger = _MockLogger(); + }); + + test('returns null when the root pubspec.yaml is missing', () { + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, isNull); + verifyNever(() => logger.warn(any())); + }); + + test('returns null and warns when the root pubspec is unparseable', () { + File( + path.join(tempDirectory.path, 'pubspec.yaml'), + ).writeAsStringSync('{{{ not valid yaml'); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, isNull); + verify(() => logger.warn(any())).called(1); + }); + + test('returns null for a non-workspace package (no workspace key)', () { + writePubspec(tempDirectory, '.', ''' +name: single_package +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, isNull); + verifyNever(() => logger.warn(any())); + }); + + test( + 'returns null for a non-list workspace key (treated as non-workspace)', + () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: not-a-list +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, isNull); + }, + ); + + test('returns null for an empty workspace list', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: [] +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, isNull); + }); + + test("unions members' direct deps under direct-main and direct-dev", () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - packages/pkg_a +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +dev_dependencies: + test: ^1.24.0 +'''); + writePubspec(tempDirectory, 'packages/pkg_a', ''' +name: pkg_a +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + collection: ^1.18.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, { + 'path': PubspecDependencyType.directMain, + 'collection': PubspecDependencyType.directMain, + 'test': PubspecDependencyType.directDev, + }); + verifyNever(() => logger.warn(any())); + }); + + test("includes the root's own dependencies in the union", () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +dependencies: + args: ^2.4.0 +dev_dependencies: + build_runner: ^2.4.0 +dependency_overrides: + meta: ^1.9.0 +workspace: + - app +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, { + 'args': PubspecDependencyType.directMain, + 'path': PubspecDependencyType.directMain, + 'build_runner': PubspecDependencyType.directDev, + 'meta': PubspecDependencyType.directOverridden, + }); + }); + + test( + 'resolves a cross-member type conflict via precedence main > dev', + () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - packages/pkg_a +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + shared: ^1.0.0 +'''); + writePubspec(tempDirectory, 'packages/pkg_a', ''' +name: pkg_a +resolution: workspace +environment: + sdk: ^3.11.0 +dev_dependencies: + shared: ^1.0.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect( + result!['shared'], + PubspecDependencyType.directMain, + ); + }, + ); + + test( + 'resolves a cross-member type conflict via precedence dev > overridden', + () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - packages/pkg_a +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dev_dependencies: + shared: ^1.0.0 +'''); + writePubspec(tempDirectory, 'packages/pkg_a', ''' +name: pkg_a +resolution: workspace +environment: + sdk: ^3.11.0 +dependency_overrides: + shared: ^1.0.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect( + result!['shared'], + PubspecDependencyType.directDev, + ); + }, + ); + + test( + 'resolves a cross-member type conflict via precedence main > overridden', + () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - packages/pkg_a +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + shared: ^1.0.0 +'''); + writePubspec(tempDirectory, 'packages/pkg_a', ''' +name: pkg_a +resolution: workspace +environment: + sdk: ^3.11.0 +dependency_overrides: + shared: ^1.0.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect( + result!['shared'], + PubspecDependencyType.directMain, + ); + }, + ); + + test('resolves a two-level nested workspace', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +workspace: + - nested +'''); + writePubspec(tempDirectory, 'app/nested', ''' +name: nested +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + collection: ^1.18.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, { + 'path': PubspecDependencyType.directMain, + 'collection': PubspecDependencyType.directMain, + }); + }); + + test('terminates on a cyclic / self-referential workspace graph', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app +'''); + // The member points back at itself, forming a cycle. + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +workspace: + - . +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, {'path': PubspecDependencyType.directMain}); + }); + + test('counts a member reached through overlapping entries once', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - packages/pkg_a + - packages/* +'''); + writePubspec(tempDirectory, 'packages/pkg_a', ''' +name: pkg_a +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, {'path': PubspecDependencyType.directMain}); + }); + + test('expands a glob workspace entry to matching member directories', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - packages/* +'''); + writePubspec(tempDirectory, 'packages/pkg_a', ''' +name: pkg_a +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + writePubspec(tempDirectory, 'packages/pkg_b', ''' +name: pkg_b +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + collection: ^1.18.0 +'''); + // A matched directory without a pubspec.yaml is skipped, not fatal. + Directory( + path.join(tempDirectory.path, 'packages', 'not_a_package'), + ).createSync(recursive: true); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, { + 'path': PubspecDependencyType.directMain, + 'collection': PubspecDependencyType.directMain, + }); + verify(() => logger.warn(any())).called(1); + }); + + test('warns and continues when a glob entry cannot be listed', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - blocked/* +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + // A file where the glob expects a directory makes listing throw a + // FileSystemException, which is treated as no match. + File(path.join(tempDirectory.path, 'blocked')).writeAsStringSync(''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, {'path': PubspecDependencyType.directMain}); + verify(() => logger.warn(any())).called(1); + }); + + test('warns and continues when a workspace entry matches no directory', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - does_not_exist +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, {'path': PubspecDependencyType.directMain}); + verify(() => logger.warn(any())).called(1); + }); + + test('warns and skips a member with an unparseable pubspec', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app + - broken +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +'''); + writePubspec(tempDirectory, 'broken', '{{{ not valid yaml'); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, {'path': PubspecDependencyType.directMain}); + verify(() => logger.warn(any())).called(1); + }); + + test('leniently parses a member with valid-but-unmodeled keys', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + path: ^1.9.0 +flutter: + uses-material-design: true +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, {'path': PubspecDependencyType.directMain}); + verifyNever(() => logger.warn(any())); + }); + + test('returns an empty map when no member declares direct deps', () { + writePubspec(tempDirectory, '.', ''' +name: workspace_root +environment: + sdk: ^3.11.0 +workspace: + - app +'''); + writePubspec(tempDirectory, 'app', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +'''); + + final result = resolveWorkspaceDependencies( + tempDirectory, + logger: logger, + ); + + expect(result, isEmpty); + }); + }); + + group('declaresWorkspaceResolution', () { + late Directory tempDirectory; + + setUp(() { + tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + }); + + test('returns true when pubspec declares resolution: workspace', () { + writePubspec(tempDirectory, '.', ''' +name: app +resolution: workspace +environment: + sdk: ^3.11.0 +'''); + + expect(declaresWorkspaceResolution(tempDirectory), isTrue); + }); + + test('returns false when pubspec does not declare a resolution', () { + writePubspec(tempDirectory, '.', ''' +name: app +environment: + sdk: ^3.11.0 +'''); + + expect(declaresWorkspaceResolution(tempDirectory), isFalse); + }); + + test('returns false when there is no pubspec', () { + expect(declaresWorkspaceResolution(tempDirectory), isFalse); + }); + }); +}