FEDX-7265: Dependencies used inside hook/ must be regular dependencies, not dev_dependencies

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Unreleased
 <!-- Add unreleased changes here -->
+
+- **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

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

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';
 

lib/src/dependency_validator.dart

@@ -119,7 +119,7 @@ Future<bool> 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<bool> 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<bool> 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<bool> 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<bool> 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<bool> 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,
     );

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',
           ),
         );
       },

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);