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..d029d16 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -58,7 +58,7 @@ Future checkPackage({required String root}) async { .map((s) { try { return makeGlob("$root/$s"); - } catch (_, __) { + } catch (_) { logger.shout(yellow.wrap('invalid glob syntax: "$s"')); return null; } @@ -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 @@ -309,8 +317,7 @@ Future checkPackage({required String root}) async { .difference(packagesUsedInPublicFiles) .difference(packagesUsedOutsidePublicDirs) // Remove this package, since we know they're using our executable - ..remove(dependencyValidatorPackageName) - ..removeAll(ignoredPackages); + ..remove(dependencyValidatorPackageName); final packageConfig = await findPackageConfig(Directory.current); if (packageConfig == null) { @@ -400,6 +407,8 @@ Future checkPackage({required String root}) async { ); } + unusedDependencies.removeAll(ignoredPackages); + if (unusedDependencies.isNotEmpty) { log( Level.WARNING, 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/executable_test.dart b/test/executable_test.dart index 71f9b47..c3e9d86 100644 --- a/test/executable_test.dart +++ b/test/executable_test.dart @@ -313,7 +313,7 @@ void main() { devDependencies: { "build_runner": hostedCompatibleWith('2.3.3'), 'coverage': hostedAny, - 'dart_style': hostedCompatibleWith('2.3.2'), + 'dart_style': hostedAny, }, project: [ d.dir('lib', [d.file('main.dart', 'book fake = true;')]), @@ -331,7 +331,7 @@ void main() { dependencies: { "build_runner": hostedCompatibleWith('2.3.3'), "coverage": hostedAny, - "dart_style": hostedCompatibleWith('2.3.2'), + "dart_style": hostedAny, }, project: [ d.dir('lib', [d.file('main.dart', 'bool fake = true;')]), @@ -353,9 +353,8 @@ void main() { () async { result = await checkProject( devDependencies: { - 'build_test': hostedCompatibleWith('2.0.1'), - 'build_vm_compilers': hostedCompatibleWith('1.0.3'), - 'build_web_compilers': hostedCompatibleWith('3.2.7'), + 'build_test': hostedAny, + 'build_web_compilers': hostedAny, }, project: [ d.dir('lib', [d.file('main.dart', 'book fake = true;')]), 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'), + ]); + }); + }); }