From 7ab1d175494d8310a7c25dbc722125a652a1dad3 Mon Sep 17 00:00:00 2001 From: fraxken Date: Fri, 16 Aug 2024 03:10:58 +0200 Subject: [PATCH] feat(EntryFilesAnalyser): implement digraph-js --- docs/api/EntryFilesAnalyser.md | 14 +++- index.js | 12 +-- package.json | 1 + src/EntryFilesAnalyser.js | 98 +++++++++++++++++++------ test/EntryFilesAnalyser.spec.js | 40 +++++++++- test/fixtures/entryFiles/recursive/A.js | 4 + test/fixtures/entryFiles/recursive/B.js | 4 + types/api.d.ts | 25 +++++-- 8 files changed, 156 insertions(+), 42 deletions(-) create mode 100644 test/fixtures/entryFiles/recursive/A.js create mode 100644 test/fixtures/entryFiles/recursive/B.js diff --git a/docs/api/EntryFilesAnalyser.md b/docs/api/EntryFilesAnalyser.md index dcff432..2a1d8e9 100644 --- a/docs/api/EntryFilesAnalyser.md +++ b/docs/api/EntryFilesAnalyser.md @@ -30,8 +30,20 @@ Default files extensions are `.js`, `.cjs`, `.mjs` and `.node` ```ts declare class EntryFilesAnalyser { + public dependencies: DiGraph; + constructor(options?: EntryFilesAnalyserOptions); - analyse(entryFiles: (string | URL)[]): AsyncGenerator; + + /** + * Asynchronously analyze a set of entry files yielding analysis reports. + */ + analyse( + entryFiles: Iterable, + options?: { + fileOptions?: RuntimeFileOptions; + rootPath?: string | URL; + } + ): AsyncGenerator; } ``` diff --git a/index.js b/index.js index 765e892..e636dbb 100644 --- a/index.js +++ b/index.js @@ -12,10 +12,10 @@ function runASTAnalysis( options = Object.create(null) ) { process.emitWarning( - 'The runASTAnalysis API is deprecated and will be removed in v8. Please use the AstAnalyser class instead.', + "The runASTAnalysis API is deprecated and will be removed in v8. Please use the AstAnalyser class instead.", { - code: 'DeprecationWarning', - detail: 'The runASTAnalysis API is deprecated and will be removed in v8. Please use the AstAnalyser class instead.' + code: "DeprecationWarning", + detail: "The runASTAnalysis API is deprecated and will be removed in v8. Please use the AstAnalyser class instead." } ); @@ -43,10 +43,10 @@ async function runASTAnalysisOnFile( options = {} ) { process.emitWarning( - 'The runASTAnalysisOnFile API is deprecated and will be removed in v8. Please use the AstAnalyser class instead.', + "The runASTAnalysisOnFile API is deprecated and will be removed in v8. Please use the AstAnalyser class instead.", { - code: 'DeprecationWarning', - detail: 'The runASTAnalysisOnFile API is deprecated and will be removed in v8. Please use the AstAnalyser class instead.' + code: "DeprecationWarning", + detail: "The runASTAnalysisOnFile API is deprecated and will be removed in v8. Please use the AstAnalyser class instead." } ); diff --git a/package.json b/package.json index 56ef214..74777e6 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "dependencies": { "@nodesecure/estree-ast-utils": "^1.3.1", "@nodesecure/sec-literal": "^1.2.0", + "digraph-js": "^2.2.3", "estree-walker": "^3.0.1", "frequency-set": "^1.0.2", "is-minified-code": "^2.0.0", diff --git a/src/EntryFilesAnalyser.js b/src/EntryFilesAnalyser.js index ff7aa5c..526137e 100644 --- a/src/EntryFilesAnalyser.js +++ b/src/EntryFilesAnalyser.js @@ -3,6 +3,9 @@ import fs from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; +// Import Third-party Dependencies +import { DiGraph } from "digraph-js"; + // Import Internal Dependencies import { AstAnalyser } from "./AstAnalyser.js"; @@ -10,6 +13,8 @@ import { AstAnalyser } from "./AstAnalyser.js"; const kDefaultExtensions = ["js", "cjs", "mjs", "node"]; export class EntryFilesAnalyser { + #rootPath = null; + constructor(options = {}) { this.astAnalyzer = options.astAnalyzer ?? new AstAnalyser(); const rawAllowedExtensions = options.loadExtensions @@ -21,12 +26,22 @@ export class EntryFilesAnalyser { async* analyse( entryFiles, - analyseFileOptions + options = {} ) { - this.analyzedDeps = new Set(); + const { fileOptions, rootPath = null } = options; + this.#rootPath = fileURLToPathExtended(rootPath); + this.dependencies = new DiGraph(); + + for (const file of new Set(entryFiles)) { + let filePath = path.normalize(fileURLToPathExtended(file)); + if (path.isAbsolute(filePath) && this.#rootPath !== null) { + filePath = path.relative(this.#rootPath, filePath); + } - for (const file of entryFiles) { - yield* this.#analyseFile(file, analyseFileOptions); + yield* this.#analyseFile( + filePath, + fileOptions + ); } } @@ -34,10 +49,18 @@ export class EntryFilesAnalyser { file, options ) { - const filePath = file instanceof URL ? fileURLToPath(file) : file; - const report = await this.astAnalyzer.analyseFile(file, options); - - yield { url: filePath, ...report }; + this.dependencies.addVertex({ + id: file, + adjacentTo: [] + }); + + const absoluteFilePath = this.#rootPath === null ? + file : path.join(this.#rootPath, file); + const report = await this.astAnalyzer.analyseFile( + absoluteFilePath, + options + ); + yield { file, ...report }; if (!report.ok) { return; @@ -45,25 +68,40 @@ export class EntryFilesAnalyser { for (const [name] of report.dependencies) { const depPath = await this.#getInternalDepPath( - name, - path.dirname(filePath) + path.join(path.dirname(absoluteFilePath), name) ); + if (depPath === null) { + continue; + } - if (depPath && !this.analyzedDeps.has(depPath)) { - this.analyzedDeps.add(depPath); + const dependency = this.#rootPath === null ? + depPath : + path.relative(this.#rootPath, depPath); - yield* this.#analyseFile(depPath, options); + if (!this.dependencies.hasVertex(dependency)) { + this.dependencies.addVertex({ + id: dependency, + adjacentTo: [] + }); + + yield* this.#analyseFile(dependency, options); } + + this.dependencies.addEdge({ + from: file, + to: dependency + }); } } - async #getInternalDepPath(name, basePath) { - const depPath = path.join(basePath, name); - const existingExt = path.extname(name); + async #getInternalDepPath( + filePath + ) { + const fileExtension = path.extname(filePath); - if (existingExt === "") { + if (fileExtension === "") { for (const ext of this.allowedExtensions) { - const depPathWithExt = `${depPath}.${ext}`; + const depPathWithExt = `${filePath}.${ext}`; const fileExist = await this.#fileExists(depPathWithExt); if (fileExist) { @@ -72,22 +110,24 @@ export class EntryFilesAnalyser { } } else { - if (!this.allowedExtensions.has(existingExt.slice(1))) { + if (!this.allowedExtensions.has(fileExtension.slice(1))) { return null; } - const fileExist = await this.#fileExists(depPath); + const fileExist = await this.#fileExists(filePath); if (fileExist) { - return depPath; + return filePath; } } return null; } - async #fileExists(path) { + async #fileExists( + filePath + ) { try { - await fs.access(path, fs.constants.R_OK); + await fs.access(filePath, fs.constants.R_OK); return true; } @@ -100,3 +140,15 @@ export class EntryFilesAnalyser { } } } + +function fileURLToPathExtended( + file +) { + if (file === null) { + return null; + } + + return file instanceof URL ? + fileURLToPath(file) : + file; +} diff --git a/test/EntryFilesAnalyser.spec.js b/test/EntryFilesAnalyser.spec.js index d3b02fc..cf6ca26 100644 --- a/test/EntryFilesAnalyser.spec.js +++ b/test/EntryFilesAnalyser.spec.js @@ -1,6 +1,7 @@ // Import Node.js Dependencies import { describe, it } from "node:test"; import assert from "node:assert"; +import path from "node:path"; import { fileURLToPath } from "node:url"; // Import Internal Dependencies @@ -23,7 +24,7 @@ describe("EntryFilesAnalyser", () => { const reports = await fromAsync(generator); assert.deepEqual( - reports.map((report) => report.url), + reports.map((report) => report.file), [ entryUrl, new URL("deps/dep1.js", FIXTURE_URL), @@ -47,7 +48,7 @@ describe("EntryFilesAnalyser", () => { const reports = await fromAsync(generator); assert.deepEqual( - reports.map((report) => report.url), + reports.map((report) => report.file), [ entryUrl, new URL("deps/invalidDep.js", FIXTURE_URL), @@ -74,7 +75,7 @@ describe("EntryFilesAnalyser", () => { const reports = await fromAsync(generator); assert.deepEqual( - reports.map((report) => report.url), + reports.map((report) => report.file), [ entryUrl, new URL("deps/default.js", FIXTURE_URL), @@ -100,7 +101,7 @@ describe("EntryFilesAnalyser", () => { const reports = await fromAsync(generator); assert.deepEqual( - reports.map((report) => report.url), + reports.map((report) => report.file), [ entryUrl, new URL("deps/default.jsx", FIXTURE_URL), @@ -108,6 +109,37 @@ describe("EntryFilesAnalyser", () => { ].map((url) => fileURLToPath(url)) ); }); + + it("should detect recursive dependencies using DiGraph", async() => { + const entryFilesAnalyser = new EntryFilesAnalyser(); + const entryUrl = new URL("recursive/A.js", FIXTURE_URL); + + const generator = entryFilesAnalyser.analyse( + [entryUrl], + { + rootPath: FIXTURE_URL + } + ); + await fromAsync(generator); + + assert.deepEqual( + [...entryFilesAnalyser.dependencies.findCycles()], + [ + ["recursive/A.js", "recursive/B.js"].map((str) => path.normalize(str)) + ] + ); + + assert.deepEqual( + [ + ...entryFilesAnalyser.dependencies.getDeepChildren( + path.normalize("recursive/A.js"), 1 + ) + ], + [ + path.normalize("recursive/B.js") + ] + ); + }); }); // TODO: replace with Array.fromAsync when droping Node.js 20 diff --git a/test/fixtures/entryFiles/recursive/A.js b/test/fixtures/entryFiles/recursive/A.js new file mode 100644 index 0000000..337101a --- /dev/null +++ b/test/fixtures/entryFiles/recursive/A.js @@ -0,0 +1,4 @@ +import { bar } from "./B.js"; + +export const foo = "bar"; +console.log(bar); diff --git a/test/fixtures/entryFiles/recursive/B.js b/test/fixtures/entryFiles/recursive/B.js new file mode 100644 index 0000000..c14c582 --- /dev/null +++ b/test/fixtures/entryFiles/recursive/B.js @@ -0,0 +1,4 @@ +import { foo } from "./A.js"; + +export const bar = "foo"; +console.log(foo); diff --git a/types/api.d.ts b/types/api.d.ts index 7b62f64..4e8b4c8 100644 --- a/types/api.d.ts +++ b/types/api.d.ts @@ -1,8 +1,12 @@ +// Third-party +import { DiGraph } from "digraph-js"; +import { Statement } from "meriyah"; + +// Internal import { Warning, WarningName } from "./warnings.js"; -import { Statement } from "meriyah"; export { AstAnalyser, @@ -122,11 +126,6 @@ declare class AstAnalyser { ): ReportOnFile; } -interface EntryFilesAnalyserOptions { - astAnalyzer?: AstAnalyser; - loadExtensions?: (defaults: string[]) => string[]; -} - declare class SourceFile { constructor(source: string, options: any); addDependency( @@ -144,7 +143,14 @@ declare class SourceFile { walk(node: any): "skip" | null; } +interface EntryFilesAnalyserOptions { + astAnalyzer?: AstAnalyser; + loadExtensions?: (defaults: string[]) => string[]; +} + declare class EntryFilesAnalyser { + public dependencies: DiGraph; + constructor(options?: EntryFilesAnalyserOptions); /** @@ -152,8 +158,11 @@ declare class EntryFilesAnalyser { */ analyse( entryFiles: Iterable, - options?: RuntimeFileOptions - ): AsyncGenerator; + options?: { + fileOptions?: RuntimeFileOptions; + rootPath?: string | URL; + } + ): AsyncGenerator; } declare class JsSourceParser implements SourceParser {