Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .dependency-cruiser.mjs
Original file line number Diff line number Diff line change
@@ -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: [
Expand Down
2 changes: 1 addition & 1 deletion configs/.dependency-cruiser-show-metrics-config.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import baseConfig from "../.dependency-cruiser.mjs";
/** @type {import('../').IConfiguration} */
/** @type {import('../types/configuration.mjs').IConfiguration} */
export default {
...baseConfig,
forbidden: [
Expand Down
2 changes: 1 addition & 1 deletion configs/.dependency-cruiser-unlimited.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import baseConfig from "../.dependency-cruiser.mjs";
/** @type {import('../').IConfiguration} */
/** @type {import('../types/configuration.mjs').IConfiguration} */
export default {
...baseConfig,
options: {
Expand Down
18 changes: 16 additions & 2 deletions doc/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 3 additions & 5 deletions doc/options-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
56 changes: 28 additions & 28 deletions doc/rules-reference.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff 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"
}
}
}
91 changes: 79 additions & 12 deletions src/extract/tsc/extract-typescript-deps.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable security/detect-object-injection */
/* eslint-disable unicorn/prevent-abbreviations */
/* eslint-disable max-lines */
/* eslint-disable no-inline-comments */
Expand Down Expand Up @@ -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,
)
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);

Expand Down
8 changes: 4 additions & 4 deletions test/cache/cache.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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" } },
Expand All @@ -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: {
Expand All @@ -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: {
Expand All @@ -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: {
Expand Down
10 changes: 5 additions & 5 deletions test/cache/content-strategy.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down Expand Up @@ -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" },
Expand All @@ -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: [],
Expand Down Expand Up @@ -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" },
Expand All @@ -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: [
Expand Down
Loading