FEDX-7267: Support Dart doc imports in dependency_validator

.github/workflows/ci.yaml

@@ -10,14 +10,14 @@ jobs:
   build:
     uses: Workiva/gha-dart-oss/.github/workflows/build.yaml@v0.1.15
     with:
-      sdk: 3.7.2 # mirrors .tool-versions
+      sdk: 3.8.1 # mirrors .tool-versions
 
   checks:
     uses: Workiva/gha-dart-oss/.github/workflows/checks.yaml@v0.1.15
     with:
-      sdk: 3.7.2 # mirrors .tool-versions
+      sdk: 3.8.1 # mirrors .tool-versions
 
   unit-tests:
     uses: Workiva/gha-dart-oss/.github/workflows/test-unit.yaml@v0.1.15
     with:
-      sdk: 3.7.2 # mirrors .tool-versions
+      sdk: 3.8.1 # mirrors .tool-versions

.tool-versions

@@ -1 +1 @@
-dart 3.7.2
+dart 3.8.1

CHANGELOG.md

@@ -6,6 +6,17 @@
   newest one the analyzer knows about, which may be unreleased and reject
   valid code
 
+- Added support for Dart `@docImport` documentation imports when scanning
+  package usage.
+- Packages referenced only via `@docImport` in `lib/` must still be declared
+  in `pubspec.yaml` as either a `dependency` or `dev_dependency`; when
+  declared, they are accepted in either section and are not flagged as
+  over-promoted or unused. If such a package also has real imports outside
+  `lib/`, the normal over-promotion check still applies.
+- Removed the internal `getDartDirectivePackageNames` API in favor of
+  `getDartPackageUsage`.
+- **Breaking:** requires Dart 3.8 or above.
+
 - Allow up to analyzer 14
 
 - Fix warning when `analyzer` is depended on but not used so that it is still

lib/src/constants.dart

@@ -53,8 +53,8 @@ class DependencyPinEvaluation {
   /// possible prerelease.
   static const DependencyPinEvaluation buildOrPrerelease =
       DependencyPinEvaluation._(
-    'Builds or preleases as max bounds block minor bumps and patches.',
-  );
+        'Builds or preleases as max bounds block minor bumps and patches.',
+      );
 
   /// 1.2.3
   static const DependencyPinEvaluation directPin = DependencyPinEvaluation._(

lib/src/dependency_validator.dart

@@ -58,7 +58,7 @@ Future<bool> checkPackage({required String root}) async {
       .map((s) {
         try {
           return makeGlob("$root/$s");
-        } catch (_, __) {
+        } catch (_) {
           logger.shout(yellow.wrap('invalid glob syntax: "$s"'));
           return null;
         }
@@ -148,13 +148,14 @@ Future<bool> checkPackage({required String root}) async {
       '${bulletItems(publicLessFiles.map((f) => f.path))}\n',
     );
 
-  // Read each file in lib/ and parse the package names from every import and
-  // export directive.
+  // Read each file in lib/ and parse the package names from every import,
+  // export directive, and doc import.
   final packagesUsedInPublicFiles = <String>{};
+  final packagesUsedViaDocImportInPublicFiles = <String>{};
   for (final file in publicDartFiles) {
-    packagesUsedInPublicFiles.addAll(
-      getDartDirectivePackageNames(file, featureSet: featureSet),
-    );
+    final usage = getDartPackageUsage(file, featureSet: featureSet);
+    packagesUsedInPublicFiles.addAll(usage.directivePackageNames);
+    packagesUsedViaDocImportInPublicFiles.addAll(usage.docImportPackageNames);
   }
   for (final file in publicScssFiles) {
     final matches = importScssPackageRegex.allMatches(file.readAsStringSync());
@@ -206,15 +207,18 @@ Future<bool> checkPackage({required String root}) async {
     );
 
   // Read each file outside lib/ and parse the package names from every
-  // import and export directive.
+  // import, export directive, and doc import.
   final packagesUsedOutsidePublicDirs = <String>{
     // For more info on analysis options:
     // https://dart.dev/guides/language/analysis-options#the-analysis-options-file
     if (optionsIncludePackage != null) optionsIncludePackage,
   };
+  final packagesUsedViaDocImportOutsidePublicDirs = <String>{};
   for (final file in nonPublicDartFiles) {
-    packagesUsedOutsidePublicDirs.addAll(
-      getDartDirectivePackageNames(file, featureSet: featureSet),
+    final usage = getDartPackageUsage(file, featureSet: featureSet);
+    packagesUsedOutsidePublicDirs.addAll(usage.directivePackageNames);
+    packagesUsedViaDocImportOutsidePublicDirs.addAll(
+      usage.docImportPackageNames,
     );
   }
   for (final file in nonPublicScssFiles) {
@@ -230,6 +234,19 @@ Future<bool> checkPackage({required String root}) async {
     }
   }
 
+  // Packages that are doc-imported in lib/ and have no real (non-doc) usage
+  // anywhere outside lib/. Doc imports are not runtime dependencies, so these
+  // are valid in either `dependencies` or `dev_dependencies`. A package with
+  // real usage outside lib/ is still subject to the normal over-promotion check.
+  final packagesUsedOnlyViaDocImport = packagesUsedViaDocImportInPublicFiles
+      .difference(packagesUsedOutsidePublicDirs);
+
+  // Doc imports are not runtime dependencies, so treat them like usage outside
+  // lib/ for the missing/unused dependency checks.
+  packagesUsedOutsidePublicDirs
+    ..addAll(packagesUsedViaDocImportOutsidePublicDirs)
+    ..addAll(packagesUsedViaDocImportInPublicFiles);
+
   logger.fine(
     'packages used outside public dirs:\n'
     '${bulletItems(packagesUsedOutsidePublicDirs)}\n',
@@ -283,9 +300,12 @@ Future<bool> checkPackage({required String root}) async {
   final overPromotedDependencies =
       // Start with dependencies that are not used in lib/
       (deps
-          .difference(packagesUsedInPublicFiles)
-          // Intersect with deps that are used outside lib/ (excludes unused deps)
-          .intersection(packagesUsedOutsidePublicDirs))
+            .difference(packagesUsedInPublicFiles)
+            // Intersect with deps that are used outside lib/ (excludes unused deps)
+            .intersection(packagesUsedOutsidePublicDirs))
+        // Doc-import-only packages are accepted in either dependencies or
+        // dev_dependencies.
+        ..removeAll(packagesUsedOnlyViaDocImport)
         // Ignore known over-promoted packages.
         ..removeAll(ignoredPackages);
 
@@ -343,11 +363,12 @@ Future<bool> checkPackage({required String root}) async {
     pubspec.dependencies.keys,
     '.',
   );
-  bool rootPackageReferencesDependencyInBuildYaml(String dependencyName) => [
-        ...rootBuildConfig.globalOptions.keys,
-        for (final target in rootBuildConfig.buildTargets.values)
-          ...target.builders.keys,
-      ]
+  bool rootPackageReferencesDependencyInBuildYaml(String dependencyName) =>
+      [
+            ...rootBuildConfig.globalOptions.keys,
+            for (final target in rootBuildConfig.buildTargets.values)
+              ...target.builders.keys,
+          ]
           .map((key) => normalizeBuilderKeyUsage(key, pubspec.name))
           .any((key) => key.startsWith('$dependencyName:'));
 
@@ -386,8 +407,9 @@ Future<bool> checkPackage({required String root}) async {
       if (providesExecutable(package)) package,
   };
 
-  final nonDevPackagesWithExecutables =
-      packagesWithExecutables.where(pubspec.dependencies.containsKey).toSet();
+  final nonDevPackagesWithExecutables = packagesWithExecutables
+      .where(pubspec.dependencies.containsKey)
+      .toSet();
   if (nonDevPackagesWithExecutables.isNotEmpty) {
     logIntersection(
       Level.WARNING,
@@ -434,10 +456,7 @@ Future<bool> checkPackage({required String root}) async {
 Future<bool> dependencyDefinesAutoAppliedBuilder(String path) async =>
     (await BuildConfig.fromPackageDir(
       path,
-    ))
-        .builderDefinitions
-        .values
-        .any((def) => def.autoApply != AutoApply.none);
+    )).builderDefinitions.values.any((def) => def.autoApply != AutoApply.none);
 
 /// Checks for dependency pins.
 ///

lib/src/import_export_ast_visitor.dart

@@ -4,6 +4,7 @@ import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/analysis/utilities.dart';
 import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:pub_semver/pub_semver.dart';
 
@@ -22,9 +23,23 @@ FeatureSet featureSetForSdkConstraint(VersionConstraint? sdkConstraint) {
   );
 }
 
-/// Returns the list of package names that are exported and imported into the
-/// provided dart file
-Set<String> getDartDirectivePackageNames(File file, {FeatureSet? featureSet}) {
+/// Package names referenced in a Dart file via import/export directives and
+/// doc imports.
+class DartPackageUsage {
+  /// Package names from `import` and `export` directives.
+  final Set<String> directivePackageNames;
+
+  /// Package names from `@docImport` documentation imports.
+  final Set<String> docImportPackageNames;
+
+  const DartPackageUsage({
+    required this.directivePackageNames,
+    required this.docImportPackageNames,
+  });
+}
+
+/// Returns the package names referenced in the provided Dart file.
+DartPackageUsage getDartPackageUsage(File file, {FeatureSet? featureSet}) {
   ParseStringResult parsed;
   try {
     parsed = parseString(
@@ -39,27 +54,102 @@ Set<String> getDartDirectivePackageNames(File file, {FeatureSet? featureSet}) {
   }
 
   final visitor = ImportExportVisitor();
-  parsed.unit.visitChildren(visitor);
-  return visitor.packageNames;
+  parsed.unit.accept(visitor);
+  _collectDocImportsFromPrecedingComments(
+    parsed.unit.beginToken.precedingComments,
+    visitor.docImportPackageNames,
+  );
+  return DartPackageUsage(
+    directivePackageNames: visitor.directivePackageNames,
+    docImportPackageNames: visitor.docImportPackageNames,
+  );
 }
 
-class ImportExportVisitor extends GeneralizingAstVisitor {
-  Set<String> packageNames = {};
+/// Collects `@docImport` package names from comment tokens that are not
+/// attached to any AST node (e.g. a file containing only a doc comment).
+///
+/// Mirrors the analyzer's own doc comment parsing as closely as is practical:
+/// only `///` and `/** */` doc comments are considered, `@docImport` must start
+/// a line, and fenced code blocks are skipped.
+void _collectDocImportsFromPrecedingComments(
+  Token? commentToken,
+  Set<String> docImportPackageNames,
+) {
+  var inFencedCodeBlock = false;
+  for (var token = commentToken; token != null; token = token.next) {
+    if (token is! CommentToken) continue;
 
-  @override
-  void visitDirective(Directive node) {
-    if (node is! UriBasedDirective) return;
+    final lexeme = token.lexeme;
+    final isBlockDocComment = lexeme.startsWith('/**');
+    if (!isBlockDocComment && !lexeme.startsWith('///')) continue;
+
+    // A block doc comment is self-contained; don't carry fence state into it.
+    if (isBlockDocComment) inFencedCodeBlock = false;
+
+    for (final line in lexeme.split('\n')) {
+      final content = _stripDocCommentDecoration(line);
+      if (content.startsWith('```')) {
+        inFencedCodeBlock = !inFencedCodeBlock;
+        continue;
+      }
+      if (inFencedCodeBlock) continue;
+      _collectDocImportFromLine(content, docImportPackageNames);
+    }
+  }
+}
+
+/// Strips the leading `///`, `/**`, or ` * ` and trailing `*/` from a single
+/// line of a doc comment lexeme.
+String _stripDocCommentDecoration(String line) {
+  var content = line.trim();
+  if (content.startsWith('///') || content.startsWith('/**')) {
+    content = content.substring(3);
+  } else if (content.startsWith('*')) {
+    content = content.substring(1);
+  }
+  if (content.endsWith('*/')) {
+    content = content.substring(0, content.length - 2);
+  }
+  return content.trim();
+}
+
+final _docImportUriPattern = RegExp(r'''^@docImport\s+(['"])(.+?)\1''');
 
-    final uri = node.uri.stringValue;
-    if (uri == null) return;
+void _collectDocImportFromLine(String line, Set<String> docImportPackageNames) {
+  final match = _docImportUriPattern.firstMatch(line);
+  if (match == null) return;
+  _addPackageName(match.group(2), docImportPackageNames);
+}
+
+void _addPackageName(String? uri, Set<String> packageNames) {
+  if (uri == null) return;
 
-    // ignore relative path imports
-    if (!uri.startsWith('package:')) return;
+  // ignore relative path imports
+  if (!uri.startsWith('package:')) return;
 
-    final packageParts = uri.substring('package:'.length).split('/');
-    if (packageParts.isEmpty)
-      return; // sanity check, this probably will never happen
+  final packageParts = uri.substring('package:'.length).split('/');
+  if (packageParts.isEmpty) return;
+
+  packageNames.add(packageParts.first);
+}
 
-    packageNames.add(packageParts.first);
+class ImportExportVisitor extends GeneralizingAstVisitor {
+  Set<String> directivePackageNames = {};
+  Set<String> docImportPackageNames = {};
+
+  @override
+  void visitDirective(Directive node) {
+    if (node is UriBasedDirective) {
+      _addPackageName(node.uri.stringValue, directivePackageNames);
+    }
+    super.visitDirective(node);
+  }
+
+  @override
+  void visitComment(Comment node) {
+    for (final docImport in node.docImports) {
+      _addPackageName(docImport.import.uri.stringValue, docImportPackageNames);
+    }
+    super.visitComment(node);
   }
 }

lib/src/pubspec_config.dart

@@ -17,7 +17,7 @@ class PubspecDepValidatorConfig {
       dependencyValidator.ignore.isNotEmpty;
 
   PubspecDepValidatorConfig({DepValidatorConfig? dependencyValidator})
-      : dependencyValidator = dependencyValidator ?? DepValidatorConfig();
+    : dependencyValidator = dependencyValidator ?? DepValidatorConfig();
 
   factory PubspecDepValidatorConfig.fromJson(Map json) =>
       _$PubspecDepValidatorConfigFromJson(json);

pubspec.yaml

@@ -4,10 +4,10 @@ description: Checks for missing, under-promoted, over-promoted, and unused depen
 homepage: https://github.com/Workiva/dependency_validator
 
 environment:
-  sdk: ^3.0.0
+  sdk: ^3.8.0
 
 dependencies:
-  analyzer: ">=7.1.0 <15.0.0"
+  analyzer: ">=8.0.0 <15.0.0"
   args: ^2.0.0
   build_config: ^1.0.0
   checked_yaml: ^2.0.1
@@ -18,7 +18,7 @@ dependencies:
   package_config: ">=2.0.0 <4.0.0"
   path: ^1.8.0
   pub_semver: ^2.0.0
-  pubspec_parse: ^1.5.0
+  pubspec_parse: ^1.6.0
   yaml: ^3.1.0
 
 dev_dependencies: