From 1600be72aa7cd6835f23fd438dbe541f7bcb6554 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 9 Sep 2026 08:40:18 +0000 Subject: [PATCH] Ignore nested packages in surrounding package (#173) --- CHANGELOG.md | 1 + lib/src/dependency_validator.dart | 34 +++-- lib/src/utils.dart | 21 +++ test/nested_packages_test.dart | 206 ++++++++++++++++++++++++++++++ test/utils_test.dart | 50 ++++++++ 5 files changed, 299 insertions(+), 13 deletions(-) create mode 100644 test/nested_packages_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 737aacf..63878bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +- Ignore nested packages when validating surrounding packages (#173). # 5.0.5 diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index 7eb7d9e..cbf8054 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -105,16 +105,31 @@ Future checkPackage({required String root}) async { '${bulletItems(devDeps)}\n', ); + final nestedPackages = listNestedPackages(root); + final nestedPackageGlobs = [ + for (final nested in nestedPackages) + makeGlob('${p.normalize(nested.path)}/**'), + for (final subpackage in pubspec.workspace ?? []) + makeGlob('${p.normalize('$root/$subpackage')}/**'), + ]; + logger.fine( + 'nested package globs:\n' + '${bulletItems(nestedPackageGlobs.map((g) => g.pattern))}\n', + ); + final publicDirs = ['$root/bin/', '$root/lib/']; logger.fine("Excluding: $excludes"); final publicDartFiles = [ - for (final dir in publicDirs) ...listDartFilesIn(dir, excludes), + for (final dir in publicDirs) + ...listDartFilesIn(dir, [...excludes, ...nestedPackageGlobs]), ]; final publicScssFiles = [ - for (final dir in publicDirs) ...listScssFilesIn(dir, excludes), + for (final dir in publicDirs) + ...listScssFilesIn(dir, [...excludes, ...nestedPackageGlobs]), ]; final publicLessFiles = [ - for (final dir in publicDirs) ...listLessFilesIn(dir, excludes), + for (final dir in publicDirs) + ...listLessFilesIn(dir, [...excludes, ...nestedPackageGlobs]), ]; logger @@ -156,27 +171,20 @@ Future checkPackage({required String root}) async { final publicDirGlobs = [for (final dir in publicDirs) makeGlob('$dir**')]; - final subpackageGlobs = [ - for (final subpackage in pubspec.workspace ?? []) - makeGlob('$root/$subpackage**'), - ]; - - logger.fine('subpackage globs: $subpackageGlobs'); - final nonPublicDartFiles = listDartFilesIn('$root/', [ ...excludes, ...publicDirGlobs, - ...subpackageGlobs, + ...nestedPackageGlobs, ]); final nonPublicScssFiles = listScssFilesIn('$root/', [ ...excludes, ...publicDirGlobs, - ...subpackageGlobs, + ...nestedPackageGlobs, ]); final nonPublicLessFiles = listLessFilesIn('$root/', [ ...excludes, ...publicDirGlobs, - ...subpackageGlobs, + ...nestedPackageGlobs, ]); logger diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 427a5f5..7622d4c 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -94,6 +94,27 @@ Iterable listFilesWithExtensionIn( .where((file) => excludes.every((glob) => !glob.matches(file.path))); } +/// Returns an iterable of all directories containing a `pubspec.yaml` file +/// within [dirPath], excluding [dirPath] itself. +/// +/// This also excludes directories inside hidden directories, like `.dart_tool/`. +Iterable listNestedPackages(String dirPath) { + final rootDir = Directory(dirPath); + if (!rootDir.existsSync()) return []; + + final rootCanonicalPath = p.canonicalize(rootDir.path); + + return rootDir + .listSync(recursive: true) + .whereType() + .where( + (file) => !p.split(file.path).any((d) => d != '.' && d.startsWith('.')), + ) + .where((file) => p.basename(file.path) == 'pubspec.yaml') + .map((file) => file.parent) + .where((dir) => p.canonicalize(dir.path) != rootCanonicalPath); +} + /// Logs the given [message] at [level] and lists all of the given [dependencies]. void log(Level level, String message, Iterable dependencies) { final sortedDependencies = dependencies.toList()..sort(); diff --git a/test/nested_packages_test.dart b/test/nested_packages_test.dart new file mode 100644 index 0000000..df1c643 --- /dev/null +++ b/test/nested_packages_test.dart @@ -0,0 +1,206 @@ +import 'dart:convert'; +import 'package:dependency_validator/src/dependency_validator.dart'; +import 'package:pub_semver/pub_semver.dart'; +import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:test/test.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; + +import 'pubspec_to_json.dart'; +import 'utils.dart'; + +void main() => group('Nested packages', () { + initLogs(); + + test('ignores dependencies used only in nested packages', () async { + final rootPubspec = Pubspec( + 'code_assets', + environment: requireDart36, + dependencies: { + 'http': HostedDependency(version: VersionConstraint.any), + }, + ); + + final nestedPubspec = Pubspec( + 'host_name', + environment: requireDart36, + devDependencies: { + 'ffigen': HostedDependency(version: VersionConstraint.any), + }, + ); + + final dir = d.dir('code_assets', [ + d.file('pubspec.yaml', jsonEncode(rootPubspec.toJson())), + d.dir('lib', [ + d.file('code_assets.dart', 'import "package:http/http.dart";'), + ]), + d.dir('example', [ + d.dir('host_name', [ + d.file('pubspec.yaml', jsonEncode(nestedPubspec.toJson())), + d.dir('tool', [ + d.file('ffigen.dart', 'import "package:ffigen/ffigen.dart";'), + ]), + d.dir('lib', [ + d.file( + 'host_name.dart', 'import "package:archive/archive.dart";'), + ]), + ]), + ]), + ]); + + await dir.create(); + final result = await checkPackage(root: '${d.sandbox}/code_assets'); + expect(result, isTrue); + }); + + test( + 'fails when root package itself has undeclared dependencies outside nested packages', + () async { + final rootPubspec = Pubspec( + 'code_assets', + environment: requireDart36, + dependencies: {}, + ); + + final nestedPubspec = Pubspec( + 'host_name', + environment: requireDart36, + devDependencies: { + 'ffigen': HostedDependency(version: VersionConstraint.any), + }, + ); + + final dir = d.dir('code_assets_with_issue', [ + d.file('pubspec.yaml', jsonEncode(rootPubspec.toJson())), + d.dir('tool', [ + // Undeclared dependency in root package's own tool dir + d.file('root_tool.dart', 'import "package:meta/meta.dart";'), + ]), + d.dir('example', [ + d.dir('host_name', [ + d.file('pubspec.yaml', jsonEncode(nestedPubspec.toJson())), + d.dir('tool', [ + d.file('ffigen.dart', 'import "package:ffigen/ffigen.dart";'), + ]), + ]), + ]), + ]); + + await dir.create(); + final result = + await checkPackage(root: '${d.sandbox}/code_assets_with_issue'); + expect(result, isFalse); + }); + + test('ignores deeply nested packages', () async { + final rootPubspec = Pubspec( + 'root_pkg', + environment: requireDart36, + ); + + final deeplyNestedPubspec = Pubspec( + 'deep_pkg', + environment: requireDart36, + ); + + final dir = d.dir('root_pkg', [ + d.file('pubspec.yaml', jsonEncode(rootPubspec.toJson())), + d.dir('example', [ + d.dir('nested', [ + d.dir('deep', [ + d.file( + 'pubspec.yaml', jsonEncode(deeplyNestedPubspec.toJson())), + d.dir('lib', [ + d.file('deep.dart', 'import "package:meta/meta.dart";'), + ]), + ]), + ]), + ]), + ]); + + await dir.create(); + final result = await checkPackage(root: '${d.sandbox}/root_pkg'); + expect(result, isTrue); + }); + + test('ignores SCSS and Less files in nested packages', () async { + final rootPubspec = Pubspec( + 'web_pkg', + environment: requireDart36, + ); + + final nestedPubspec = Pubspec( + 'nested_web_pkg', + environment: requireDart36, + ); + + final dir = d.dir('web_pkg', [ + d.file('pubspec.yaml', jsonEncode(rootPubspec.toJson())), + d.dir('example', [ + d.dir('nested_web', [ + d.file('pubspec.yaml', jsonEncode(nestedPubspec.toJson())), + d.dir('web', [ + d.file( + 'style.scss', '@import "package:foo_styles/style.scss";'), + d.file( + 'style.less', '@import "packages/bar_styles/style.less";'), + ]), + ]), + ]), + ]); + + await dir.create(); + final result = await checkPackage(root: '${d.sandbox}/web_pkg'); + expect(result, isTrue); + }); + + test('works with workspace subpackages that contain nested packages', + () async { + final workspacePubspec = Pubspec( + 'workspace_root', + environment: requireDart36, + workspace: ['pkgs/code_assets'], + ); + + final subpackagePubspec = Pubspec( + 'code_assets', + environment: requireDart36, + resolution: 'workspace', + dependencies: { + 'http': HostedDependency(version: VersionConstraint.any), + }, + ); + + final nestedPubspec = Pubspec( + 'host_name', + environment: requireDart36, + dependencies: { + 'ffigen': HostedDependency(version: VersionConstraint.any), + }, + ); + + final dir = d.dir('workspace', [ + d.file('pubspec.yaml', jsonEncode(workspacePubspec.toJson())), + d.dir('pkgs', [ + d.dir('code_assets', [ + d.file('pubspec.yaml', jsonEncode(subpackagePubspec.toJson())), + d.dir('lib', [ + d.file('code_assets.dart', 'import "package:http/http.dart";'), + ]), + d.dir('example', [ + d.dir('host_name', [ + d.file('pubspec.yaml', jsonEncode(nestedPubspec.toJson())), + d.dir('tool', [ + d.file( + 'ffigen.dart', 'import "package:ffigen/ffigen.dart";'), + ]), + ]), + ]), + ]), + ]), + ]); + + await dir.create(); + final result = await checkPackage(root: '${d.sandbox}/workspace'); + expect(result, isTrue); + }); + }); diff --git a/test/utils_test.dart b/test/utils_test.dart index 4c7407c..bcd82e5 100644 --- a/test/utils_test.dart +++ b/test/utils_test.dart @@ -13,6 +13,7 @@ // limitations under the License. @TestOn('vm') +import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; @@ -449,4 +450,53 @@ include: package:pedantic/analysis_options.1.8.0.yaml }); }); }); + + group('listNestedPackages', () { + test('returns empty when no directory exists', () { + expect(listNestedPackages('${d.sandbox}/non_existent'), isEmpty); + }); + + test('returns empty when only root pubspec exists', () async { + await d.dir('pkg', [ + d.file('pubspec.yaml', 'name: pkg'), + d.dir('lib', [d.file('pkg.dart', 'void main() {}')]), + ]).create(); + + expect(listNestedPackages('${d.sandbox}/pkg'), isEmpty); + }); + + test('discovers nested packages in subdirectories', () async { + await d.dir('complex_pkg', [ + d.file('pubspec.yaml', 'name: complex_pkg'), + d.dir('lib', [d.file('main.dart', 'void main() {}')]), + d.dir('example', [ + d.dir('host_name', [ + d.file('pubspec.yaml', 'name: host_name'), + d.dir('tool', [d.file('ffigen.dart', 'void main() {}')]), + ]), + ]), + d.dir('pkgs', [ + d.dir('nested_sub', [ + d.file('pubspec.yaml', 'name: nested_sub'), + d.dir('lib', [d.file('nested.dart', 'void main() {}')]), + ]), + ]), + d.dir('.dart_tool', [ + d.dir('hidden_sub', [ + d.file('pubspec.yaml', 'name: hidden_sub'), + ]), + ]), + ]).create(); + + final nested = listNestedPackages('${d.sandbox}/complex_pkg') + .map((dir) => p.relative(dir.path, from: '${d.sandbox}/complex_pkg')) + .toList() + ..sort(); + + expect(nested, [ + p.join('example', 'host_name'), + p.join('pkgs', 'nested_sub'), + ]); + }); + }); }