feat(extract): adds recognition of jsdoc 'bracket' imports (#969)

## Description - adds recognition for the 'older' bracket style imports in jsdoc (so things like `/** @type {import('thing')} */`, `/** @typedef {import('thing')} Thing */`, `/** @param {import('thing')} pParameter blablabla */`, ...) - updates documentation to reflect that it isn't a 'future feature' anymore - 🏕️ drive-by: updates jsdoc type references in dependency-cruiser's own sources TODO - [x] Add support for more exotic ways to import (e.g. `/** @type {Partial<import('bla')>} */`, `/** @type{boolean|import('bla')|import('ble')} */`, `/** {Set<string,Partial<import('bla')>} */`, ...) - [x] Verify with [the list of jsdoc tag types typescript supports](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html) there's no other corner cases that can bite us. - [x] so... we currently seem to occasionally resolve these type only imports to the real implementation. Is that an issue? > [!NOTE] > an iteration of this pull request has been published [as a beta on npmjs](https://www.npmjs.com/package/dependency-cruiser/v/16.7.0-beta-2) ## Motivation and Context - necessary to be more complete than just `@import` tags (see #964 and #965) - needs to be introduced at the same time as those to prevent a breaking change ## How Has This Been Tested? - [x] green ci - [x] additional unit tests ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Documentation only change - [ ] Refactor (non-breaking change which fixes an issue without changing functionality) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change)
sverweij · Dec 1, 2024 · 0d49477 · 0d49477
1 parent fedead6
commit 0d49477
Show file tree

Hide file tree

Showing 14 changed files with 403 additions and 69 deletions.
diff --git a/.dependency-cruiser.mjs b/.dependency-cruiser.mjs
@@ -1,9 +1,10 @@
+/* eslint-disable max-lines */
 import { fileURLToPath } from "node:url";
 
 const defaultStrictRules = fileURLToPath(
   new URL("configs/recommended-strict.cjs", import.meta.url),
 );
-/** @type {import('./').IConfiguration} */
+/** @type {import('./types/configuration.mjs').IConfiguration} */
 export default {
   extends: defaultStrictRules,
   forbidden: [

diff --git a/configs/.dependency-cruiser-show-metrics-config.mjs b/configs/.dependency-cruiser-show-metrics-config.mjs
@@ -1,5 +1,5 @@
 import baseConfig from "../.dependency-cruiser.mjs";
-/** @type {import('../').IConfiguration} */
+/** @type {import('../types/configuration.mjs').IConfiguration} */
 export default {
   ...baseConfig,
   forbidden: [

diff --git a/configs/.dependency-cruiser-unlimited.mjs b/configs/.dependency-cruiser-unlimited.mjs
@@ -1,5 +1,5 @@
 import baseConfig from "../.dependency-cruiser.mjs";
-/** @type {import('../').IConfiguration} */
+/** @type {import('../types/configuration.mjs').IConfiguration} */
 export default {
   ...baseConfig,
   options: {

diff --git a/doc/faq.md b/doc/faq.md
@@ -576,10 +576,24 @@ _teamcity_ reporters (he _dot_ and _ddot_ reporters already did before).
 **A**: From version 5.4.0 or higher you can add an _exoticRequireStrings_ key in
 your configuration with the wrapper(s) and/ or re-definitions of require:
 
-```json
-"exoticRequireStrings": ["window.require", "need", "tryRequire"]
+```javascript
+exoticRequireStrings: ["window.require", "need", "tryRequire"];
 ```
 
+### Q: I'm using jsdoc/ tsdoc comments to declare dependencies - how do I make sure dependency-cruiser picks those up?
+
+**A** From version 16.7.0 you can add this to your configuration:
+
+```javascript
+//...
+detectJSDocImports: true; // implies `parser: tsc`
+// ...
+```
+
+As only the `tsc` TypeScript parser supports this, it will need `typescript`
+to be installed (dependency-cruiser will automatically use it). For more
+information see [detectJSDocImports in the options reference](./options-reference#detectjsdocimports-detect-dependencies-in-jsdoc-comments)
+
 ### Q: Can I get code completion for .dependency-cruiser.js?
 
 **A**: Yes.

diff --git a/doc/options-reference.md b/doc/options-reference.md
@@ -791,12 +791,10 @@ you can provide the parameters like so:
 
 If you have dependencies in JSDoc comments that you want to take into account
 you can set this option to `true`. This will make dependency-cruiser look at
-TypeScript 5.5+ [`@import` tags](https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag).
+TypeScript 5.5+ [`@import` tags](https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag) as well as to bracket style imports (e.g. `/** @type {import('./thing').SomeType} */`)
+that have been part of TypeScript jsdoc/ tsdoc specification for a long time.
 
-In the near future it will also look to bracket style imports (e.g. `/** @type {import('./thing').SomeType} */`)
-in all JSDoc tags they can occur in (e.g. `@param`, `@returns`, `@type`, `@typedef` etc).
-
-As currently on the TypeScript compiler (`tsc`) can detect these imports, switching
+As currently only the TypeScript compiler (`tsc`) can detect these imports, switching
 on this option implies dependency-cruiser will set `options.parser` to `tsc` so
 it uses the TypeScript compiler to parse not only TypeScript but also JavaScript.
 

diff --git a/doc/rules-reference.md b/doc/rules-reference.md
diff --git a/package.json b/package.json
@@ -303,4 +303,4 @@
     "vue-template-compiler": ">=2.0.0 <3.0.0",
     "@vue/compiler-sfc": ">=3.0.0 <4.0.0"
   }
-}
+}
diff --git a/src/extract/tsc/extract-typescript-deps.mjs b/src/extract/tsc/extract-typescript-deps.mjs
@@ -1,3 +1,4 @@
+/* eslint-disable security/detect-object-injection */
 /* eslint-disable unicorn/prevent-abbreviations */
 /* eslint-disable max-lines */
 /* eslint-disable no-inline-comments */
@@ -263,7 +264,7 @@ function extractJSDocImportTags(pJSDocTags) {
     .filter(
       (pTag) =>
         pTag.tagName.escapedText === "import" &&
-        pTag.moduleSpecifier?.kind &&
+        pTag.moduleSpecifier &&
         typescript.SyntaxKind[pTag.moduleSpecifier.kind] === "StringLiteral" &&
         pTag.moduleSpecifier.text,
     )
@@ -275,10 +276,82 @@ function extractJSDocImportTags(pJSDocTags) {
     }));
 }
 
+function isJSDocImport(pTypeNode) {
+  // import('./hello.mjs') within jsdoc
+  return (
+    typescript.SyntaxKind[pTypeNode?.kind] === "LastTypeNode" &&
+    typescript.SyntaxKind[pTypeNode.argument?.kind] === "LiteralType" &&
+    typescript.SyntaxKind[pTypeNode.argument?.literal?.kind] ===
+      "StringLiteral" &&
+    pTypeNode.argument.literal.text
+  );
+}
+
+function keyInJSDocIsIgnorable(pKey) {
+  return [
+    "parent",
+    "pos",
+    "end",
+    "flags",
+    "emitNode",
+    "modifierFlagsCache",
+    "transformFlags",
+    "id",
+    "flowNode",
+    "symbol",
+    "original",
+  ].includes(pKey);
+}
+
+export function walkJSDoc(pObject, pCollection = new Set()) {
+  if (isJSDocImport(pObject)) {
+    pCollection.add(pObject.argument.literal.text);
+  } else if (Array.isArray(pObject)) {
+    pObject.forEach((pValue) => walkJSDoc(pValue, pCollection));
+  } else if (typeof pObject === "object") {
+    for (const lKey in pObject) {
+      if (!keyInJSDocIsIgnorable(lKey) && pObject[lKey]) {
+        walkJSDoc(pObject[lKey], pCollection);
+      }
+    }
+  }
+}
+
+export function getJSDocImports(pTagNode) {
+  const lCollection = new Set();
+  walkJSDoc(pTagNode, lCollection);
+  return Array.from(lCollection);
+}
+
+function extractJSDocBracketImports(pJSDocTags) {
+  // https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html
+  return pJSDocTags
+    .filter(
+      (pTag) =>
+        pTag.tagName.escapedText !== "import" &&
+        typescript.SyntaxKind[pTag.typeExpression?.kind] === "FirstJSDocNode",
+    )
+    .flatMap((pTag) => getJSDocImports(pTag))
+    .map((pImportName) => ({
+      module: pImportName,
+      moduleSystem: "es6",
+      exoticallyRequired: false,
+      dependencyTypes: ["type-only", "import", "jsdoc", "jsdoc-bracket-import"],
+    }));
+}
+
 function extractJSDocImports(pJSDocNodes) {
-  return pJSDocNodes
-    .filter((pJSDocLine) => pJSDocLine.tags)
-    .flatMap((pJSDocLine) => extractJSDocImportTags(pJSDocLine.tags));
+  // https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag
+  const lJSDocNodesWithTags = pJSDocNodes.filter(
+    (pJSDocLine) => pJSDocLine.tags,
+  );
+  const lJSDocImportTags = lJSDocNodesWithTags.flatMap((pJSDocLine) =>
+    extractJSDocImportTags(pJSDocLine.tags),
+  );
+  const lJSDocBracketImports = lJSDocNodesWithTags.flatMap((pJSDocLine) =>
+    extractJSDocBracketImports(pJSDocLine.tags),
+  );
+  return lJSDocImportTags.concat(lJSDocBracketImports);
 }
 
 /**
@@ -338,14 +411,8 @@ function walk(pResult, pExoticRequireStrings, pDetectJSDocImports) {
       });
     }
 
-    // /** @import thing from './module' */
-    // /** @import {thing} from './module' */
-    // /** @import * as thing from './module' */
-    // see https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag
-    //
-    // TODO: all the kinds of tags that can have import statements as type declarations
-    //      (e.g. @type, @param, @returns, @typedef, @property, @prop, @arg, ...)
-    // https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html
+    // /** @import thing from './module' */ etc
+    // /** @type {import('module').thing}*/ etc
     if (pDetectJSDocImports && pASTNode.jsDoc) {
       const lJSDocImports = extractJSDocImports(pASTNode.jsDoc);
 

diff --git a/test/cache/cache.spec.mjs b/test/cache/cache.spec.mjs
@@ -82,7 +82,7 @@ describe("[I] cache/cache - writeCache", () => {
   });
 
   it("writes the passed cruise options to the cache folder (which is created when it doesn't exist yet) - content based cached strategy", async () => {
-    /** @type {import("../..").ICruiseResult} */
+    /** @type {import("../../types/cruise-result.mjs").ICruiseResult} */
     const lDummyCacheContents = {
       modules: [],
       summary: { optionsUsed: { baseDir: "test/cache/__mocks__/cache" } },
@@ -102,7 +102,7 @@ describe("[I] cache/cache - canServeFromCache", () => {
     OUTPUTS_FOLDER,
     "serve-from-cache-compatible",
   );
-  /** @type import("../..").ICruiseResult */
+  /** @type import("../../types/cruise-result.mjs").ICruiseResult */
   const lMinimalCruiseResult = {
     modules: [],
     summary: {
@@ -117,7 +117,7 @@ describe("[I] cache/cache - canServeFromCache", () => {
     revisionData: { cacheFormatVersion: 16.2, SHA1: "dummy-sha", changes: [] },
   };
 
-  /** @type import("../..").ICruiseResult */
+  /** @type import("../../types/cruise-result.mjs").ICruiseResult */
   const lNoVersionCruiseResult = {
     modules: [],
     summary: {
@@ -132,7 +132,7 @@ describe("[I] cache/cache - canServeFromCache", () => {
     revisionData: { SHA1: "dummy-sha", changes: [] },
   };
 
-  /** @type import("../..").ICruiseResult */
+  /** @type import("../../types/cruise-result.mjs").ICruiseResult */
   const lOldVersionCruiseResult = {
     modules: [],
     summary: {

diff --git a/test/cache/content-strategy.spec.mjs b/test/cache/content-strategy.spec.mjs
@@ -254,7 +254,7 @@ describe("[U] cache/content-strategy - prepareRevisionDataForSaving", () => {
         violations: [],
       },
     };
-    /** @type {import("../..").IRevisionData} */
+    /** @type {import("../../types/cruise-result.mjs").IRevisionData} */
     const lEmptyRevisionData = {
       SHA1: "shwoop",
       changes: [],
@@ -284,7 +284,7 @@ describe("[U] cache/content-strategy - prepareRevisionDataForSaving", () => {
   });
 
   it("adds checksums to modules in the cruise result", () => {
-    /** @type {import("../..").ICruiseResult} */
+    /** @type {import("../../types/cruise-result.mjs").ICruiseResult} */
     const lEmptyCruiseResult = {
       modules: [
         { source: "foo.js" },
@@ -305,7 +305,7 @@ describe("[U] cache/content-strategy - prepareRevisionDataForSaving", () => {
         violations: [],
       },
     };
-    /** @type {import("../..").IRevisionData} */
+    /** @type {import("../../types/cruise-result.mjs").IRevisionData} */
     const lEmptyRevisionData = {
       SHA1: "shwoop",
       changes: [],
@@ -343,7 +343,7 @@ describe("[U] cache/content-strategy - prepareRevisionDataForSaving", () => {
   });
 
   it("removes changes from the revision data that aren't different anymore from the cruise result", () => {
-    /** @type {import("../..").ICruiseResult} */
+    /** @type {import("../../types/cruise-result.mjs").ICruiseResult} */
     const lCruiseResult = {
       modules: [
         { source: "foo.js" },
@@ -364,7 +364,7 @@ describe("[U] cache/content-strategy - prepareRevisionDataForSaving", () => {
         violations: [],
       },
     };
-    /** @type {import("../..").IRevisionData} */
+    /** @type {import("../../types/cruise-result.mjs").IRevisionData} */
     const lRevisionData = {
       SHA1: "shwoop",
       changes: [
-Original file line number
+Diff line change
@@ Expand Up / @@ -303,4 +303,4 @@ @@
         "vue-template-compiler": ">=2.0.0 <3.0.0",
         "@vue/compiler-sfc": ">=3.0.0 <4.0.0"
       }
-    }
+    }