diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d3057c..b008598 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Unreleased + +- **Breaking Change:** Treat `hook/` as a public-facing directory when validating dependencies. Dependencies imported in hook scripts run at build/link time (dart build, flutter build) and must be regular dependencies, not dev_dependencies. +This is enabled by default, and will break the execution of dependency_validator if it occurs within the codebase. +Resolution is to either move the dependency to `dependencies`, or `ignore`/`exclude` it. + - Ignore nested packages when validating surrounding packages (#173). - Parse files with the language version the package declares instead of the diff --git a/README.md b/README.md index dc4a9e1..7d39572 100644 --- a/README.md +++ b/README.md @@ -22,10 +22,12 @@ used even if it isn't imported. [dart-build]: https://github.com/dart-lang/build - Missing: When a dependency is used in the package but not declared in the `pubspec.yaml` -- Under-promoted: When a dependency is used within `lib/` but only declared as a dev_dependency. -- Over-promoted: When a dependency is only used outside `lib/` but declared as a dependency. +- Under-promoted: When a dependency is used within `lib/`, `bin/`, or `hook/` but only declared as a dev_dependency. +- Over-promoted: When a dependency is only used outside `lib/`, `bin/`, and `hook/` but declared as a dependency. - Unused: When a dependency is not used in the package but declared in the `pubspec.yaml`. +Hook scripts in `hook/` (for example, `build.dart` and `link.dart`) run at build/link time (`dart build`, `flutter build`), so their imports must be regular `dependencies`. To opt out, use `ignore` to suppress warnings for a specific package name (for example, a dev_dependency used only in hooks), or `exclude: ["hook/**"]` to skip scanning the hook directory entirely (which also skips missing-dependency checks in hook files). + ## Configuration There may be packages that are intentionally depended on but not used, or there diff --git a/lib/src/constants.dart b/lib/src/constants.dart index a7e7d29..a31bbaa 100644 --- a/lib/src/constants.dart +++ b/lib/src/constants.dart @@ -14,6 +14,16 @@ final RegExp importLessPackageRegex = RegExp( r'@import\s+(?:\(.*\)\s+)?"(?:packages\/|package:\/\/)([a-zA-Z1-9_-]+)\/', ); +/// Directory names treated as public-facing for dependency validation. +const publicDirNames = ['lib', 'bin', 'hook']; + +/// Human-readable list of [publicDirNames], e.g. `lib/, bin/, or hook/`. +String publicDirsDescription({String conjunction = 'or'}) { + final dirs = [for (final name in publicDirNames) '$name/']; + if (dirs.length == 1) return dirs.first; + return '${dirs.sublist(0, dirs.length - 1).join(', ')}, $conjunction ${dirs.last}'; +} + /// String key in pubspec.yaml for the dependencies map. const String dependenciesKey = 'dependencies'; diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index 835a360..2cad62a 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -119,7 +119,7 @@ Future checkPackage({required String root}) async { '${bulletItems(nestedPackageGlobs.map((g) => g.pattern))}\n', ); - final publicDirs = ['$root/bin/', '$root/lib/']; + final publicDirs = [for (final dir in publicDirNames) '$root/$dir/']; logger.fine("Excluding: $excludes"); final publicDartFiles = [ for (final dir in publicDirs) @@ -250,7 +250,7 @@ Future checkPackage({required String root}) async { if (missingDependencies.isNotEmpty) { log( Level.WARNING, - 'These packages are used in lib/ but are not dependencies:', + 'These packages are used in ${publicDirsDescription()} but are not dependencies:', missingDependencies, ); result = false; @@ -272,7 +272,7 @@ Future checkPackage({required String root}) async { if (missingDevDependencies.isNotEmpty) { log( Level.WARNING, - 'These packages are used outside lib/ but are not dev_dependencies:', + 'These packages are used outside ${publicDirsDescription(conjunction: 'and')} but are not dev_dependencies:', missingDevDependencies, ); result = false; @@ -292,7 +292,7 @@ Future checkPackage({required String root}) async { if (overPromotedDependencies.isNotEmpty) { log( Level.WARNING, - 'These packages are only used outside lib/ and should be downgraded to dev_dependencies:', + 'These packages are only used outside ${publicDirsDescription(conjunction: 'and')} and should be downgraded to dev_dependencies:', overPromotedDependencies, ); result = false; @@ -308,7 +308,7 @@ Future checkPackage({required String root}) async { if (underPromotedDependencies.isNotEmpty) { log( Level.WARNING, - 'These packages are used in lib/ and should be promoted to actual dependencies:', + 'These packages are used in ${publicDirsDescription()} and should be promoted to actual dependencies:', underPromotedDependencies, ); result = false; @@ -391,7 +391,7 @@ Future checkPackage({required String root}) async { if (nonDevPackagesWithExecutables.isNotEmpty) { logIntersection( Level.WARNING, - 'The following packages contain executables, and are only used outside of lib/. These should be downgraded to dev_dependencies:', + 'The following packages contain executables, and are only used outside of ${publicDirsDescription(conjunction: 'and')}. These should be downgraded to dev_dependencies:', unusedDependencies, nonDevPackagesWithExecutables, ); diff --git a/test/executable_test.dart b/test/executable_test.dart index a452d16..96a1f0c 100644 --- a/test/executable_test.dart +++ b/test/executable_test.dart @@ -49,7 +49,9 @@ void main() { expect(result.exitCode, 1); expect( result.stderr, - contains('These packages are used in lib/ but are not dependencies:'), + contains( + 'These packages are used in lib/, bin/, or hook/ but are not dependencies:', + ), ); expect(result.stderr, contains('yaml')); expect(result.stderr, contains('some_scss_package')); @@ -120,7 +122,7 @@ void main() { expect( result.stderr, contains( - 'These packages are only used outside lib/ and should be downgraded to dev_dependencies:', + 'These packages are only used outside lib/, bin/, and hook/ and should be downgraded to dev_dependencies:', ), ); expect(result.stderr, contains('path')); @@ -171,7 +173,7 @@ void main() { expect( result.stderr, contains( - 'These packages are used in lib/ and should be promoted to actual dependencies:', + 'These packages are used in lib/, bin/, or hook/ and should be promoted to actual dependencies:', ), ); expect(result.stderr, contains('logging')); @@ -201,6 +203,171 @@ void main() { ); }); + group('fails when hook scripts use dev_dependencies', () { + final devDependencies = {"yaml": hostedAny}; + final config = DepValidatorConfig(ignore: ['yaml']); + + final project = [ + d.dir('hook', [ + d.file('build.dart', 'import "package:yaml/yaml.dart";'), + ]), + ]; + + test('', () async { + result = await checkProject( + project: project, + devDependencies: devDependencies, + ); + expect(result.exitCode, 1); + expect( + result.stderr, + contains( + 'These packages are used in lib/, bin/, or hook/ and should be promoted to actual dependencies:', + ), + ); + expect(result.stderr, contains('yaml')); + }); + + test('except when they are ignored', () async { + result = await checkProject( + project: project, + devDependencies: devDependencies, + config: config, + ); + expect(result.exitCode, 0); + }); + + test( + 'except when they are ignored (deprecated pubspec method)', + () async { + result = await checkProject( + project: project, + devDependencies: devDependencies, + config: config, + embedConfigInPubspec: true, + ); + expect(result.exitCode, 0); + }, + ); + }); + + group('fails when hook link scripts use dev_dependencies', () { + final devDependencies = {"yaml": hostedAny}; + + test('', () async { + result = await checkProject( + devDependencies: devDependencies, + project: [ + d.dir('hook', [ + d.file('link.dart', 'import "package:yaml/yaml.dart";'), + ]), + ], + ); + expect(result.exitCode, 1); + expect( + result.stderr, + contains( + 'These packages are used in lib/, bin/, or hook/ and should be promoted to actual dependencies:', + ), + ); + expect(result.stderr, contains('yaml')); + }); + }); + + test('passes when hook scripts use regular dependencies', () async { + result = await checkProject( + dependencies: {"yaml": hostedAny}, + project: [ + d.dir('hook', [ + d.file('build.dart', 'import "package:yaml/yaml.dart";'), + ]), + ], + ); + expect(result.exitCode, 0); + expect(result.stdout, contains('No dependency issues found!')); + }); + + test('passes when hook link scripts use regular dependencies', () async { + result = await checkProject( + dependencies: {"yaml": hostedAny}, + project: [ + d.dir('hook', [ + d.file('link.dart', 'import "package:yaml/yaml.dart";'), + ]), + ], + ); + expect(result.exitCode, 0); + expect(result.stdout, contains('No dependency issues found!')); + }); + + test( + 'passes when hook-only dependency is not flagged as over-promoted', + () async { + result = await checkProject( + dependencies: {"yaml": hostedAny}, + project: [ + d.dir('hook', [ + d.file('build.dart', 'import "package:yaml/yaml.dart";'), + ]), + ], + ); + expect(result.exitCode, 0); + expect( + result.stderr, + isNot( + contains( + 'These packages are only used outside lib/, bin/, and hook/ and should be downgraded to dev_dependencies:', + ), + ), + ); + }, + ); + + group('fails when hook scripts use undeclared dependencies', () { + final project = [ + d.dir('hook', [ + d.file('build.dart', 'import "package:yaml/yaml.dart";'), + ]), + ]; + final excludeHook = DepValidatorConfig(exclude: ['hook/**']); + + test('', () async { + result = await checkProject(project: project); + expect(result.exitCode, 1); + expect( + result.stderr, + contains( + 'These packages are used in lib/, bin/, or hook/ but are not dependencies:', + ), + ); + expect(result.stderr, contains('yaml')); + }); + + test('except when hook is excluded', () async { + result = await checkProject(project: project, config: excludeHook); + expect(result.exitCode, 0); + expect(result.stderr, isEmpty); + }); + + test( + 'except when hook is excluded (deprecated pubspec method)', + () async { + result = await checkProject( + project: project, + config: excludeHook, + embedConfigInPubspec: true, + ); + expect(result.exitCode, 0); + expect( + result.stderr, + contains( + 'Configuring dependency_validator in pubspec.yaml is deprecated', + ), + ); + }, + ); + }); + group('fails when there are unused packages', () { final devDependencies = {'yaml': hostedAny}; @@ -308,30 +475,6 @@ void main() { expect(result.stdout, contains('No dependency issues found!')); }); - test('passes when a parameter carries the final modifier', () async { - result = await checkProject( - dependencies: {"logging": hostedAny}, - project: [ - d.dir('lib', [ - d.file( - 'main.dart', - unindent(''' - import 'package:logging/logging.dart'; - - void log(final Logger logger, {final String message = ''}) { - logger.info(message); - } - '''), - ), - ]), - ], - ); - - expect(result.stdout, isNot(contains('Error parsing'))); - expect(result.exitCode, 0); - expect(result.stdout, contains('No dependency issues found!')); - }); - test('passes when dependencies not used provide executables', () async { result = await checkProject( devDependencies: { @@ -366,7 +509,7 @@ void main() { expect( result.stderr, contains( - 'The following packages contain executables, and are only used outside of lib/. These should be downgraded to dev_dependencies', + 'The following packages contain executables, and are only used outside of lib/, bin/, and hook/. These should be downgraded to dev_dependencies', ), ); }, diff --git a/test/utils_test.dart b/test/utils_test.dart index bcd82e5..edf0f4e 100644 --- a/test/utils_test.dart +++ b/test/utils_test.dart @@ -22,6 +22,19 @@ import 'package:dependency_validator/src/constants.dart'; import 'package:dependency_validator/src/utils.dart'; void main() { + group('publicDirsDescription', () { + test('default conjunction', () { + expect(publicDirsDescription(), 'lib/, bin/, or hook/'); + }); + + test('and conjunction', () { + expect( + publicDirsDescription(conjunction: 'and'), + 'lib/, bin/, and hook/', + ); + }); + }); + group('getAnalysisOptionsIncludePackage', () { test('no analysis_options.yaml', () { expect(getAnalysisOptionsIncludePackage(path: d.sandbox), isNull);