From 658697adefe4d7661143c8a50070da7fe85b51b7 Mon Sep 17 00:00:00 2001 From: sverweij Date: Sat, 16 Dec 2023 15:05:46 +0100 Subject: [PATCH] refactor(graph-utl): normalizes name attribute on construction to simplify processing --- src/enrich/derive/circular.mjs | 14 +- .../derive/metrics/get-module-metrics.mjs | 2 +- src/graph-utl/indexed-module-graph.mjs | 140 +++++++++++++----- .../__mocks__/cycle-input-graphs.mjs | 3 +- test/graph-utl/indexed-module-graph.spec.mjs | 12 +- 5 files changed, 113 insertions(+), 58 deletions(-) diff --git a/src/enrich/derive/circular.mjs b/src/enrich/derive/circular.mjs index 5255ff244..0e444c4d9 100644 --- a/src/enrich/derive/circular.mjs +++ b/src/enrich/derive/circular.mjs @@ -4,17 +4,13 @@ function addCircularityCheckToDependency( pIndexedGraph, pFrom, pToDep, - pDependencyName + pDependencyName, ) { let lReturnValue = { ...pToDep, circular: false, }; - const lCycle = pIndexedGraph.getCycle( - pFrom, - pToDep[pDependencyName], - pDependencyName - ); + const lCycle = pIndexedGraph.getCycle(pFrom, pToDep[pDependencyName]); if (lCycle.length > 0) { lReturnValue = { @@ -35,7 +31,7 @@ function addCircularityCheckToDependency( export default function detectAndAddCycles( pNodes, pIndexedNodes, - { pSourceAttribute, pDependencyName } + { pSourceAttribute, pDependencyName }, ) { return pNodes.map((pModule) => ({ ...pModule, @@ -44,8 +40,8 @@ export default function detectAndAddCycles( pIndexedNodes, pModule[pSourceAttribute], pToDep, - pDependencyName - ) + pDependencyName, + ), ), })); } diff --git a/src/enrich/derive/metrics/get-module-metrics.mjs b/src/enrich/derive/metrics/get-module-metrics.mjs index 389dcc0eb..4b9695f43 100644 --- a/src/enrich/derive/metrics/get-module-metrics.mjs +++ b/src/enrich/derive/metrics/get-module-metrics.mjs @@ -20,7 +20,7 @@ function addInstabilityToDependency(pAllModules) { return (pDependency) => ({ ...pDependency, instability: - (lIndexedModules.findModuleByName(pDependency.resolved) || {}) + (lIndexedModules.findVertexByName(pDependency.resolved) || {}) .instability || 0, }); } diff --git a/src/graph-utl/indexed-module-graph.mjs b/src/graph-utl/indexed-module-graph.mjs index f96c1cda3..3e515c252 100644 --- a/src/graph-utl/indexed-module-graph.mjs +++ b/src/graph-utl/indexed-module-graph.mjs @@ -1,20 +1,56 @@ +// @ts-check /* eslint-disable security/detect-object-injection */ +/** + * @typedef {import("../../types/cruise-result.d.mts").IFolderDependency} IFolderDependency + * @typedef {import("../../types/cruise-result.d.mts").IDependency} IDependency + * @typedef {import("../../types/cruise-result.d.mts").IFolder} IFolder + * @typedef {import("../../types/cruise-result.d.mts").IModule} IModule + * @typedef {import("../../types/shared-types.d.mts").DependencyType} DependencyType + * + * @typedef {(IDependency|IFolderDependency) & {name:string; dependencyTypes?: DependencyType[]}} IEdge + * @typedef {IModule|IFolder} IModuleOrFolder + * @typedef {IModuleOrFolder & {dependencies: IEdge[]}} IVertex + * @typedef {{name:string; dependencyTypes: string[];}} IGeldedDependency + */ export default class IndexedModuleGraph { - init(pModules, pIndexAttribute) { + /** + * @param {IModuleOrFolder} pModule + * @returns {IVertex} + */ + #normalizeModule(pModule) { + return { + ...pModule, + dependencies: (pModule?.dependencies ?? []).map((pDependency) => ({ + ...pDependency, + name: pDependency.name ? pDependency.name : pDependency.resolved, + })), + }; + } + + #init(pModules, pIndexAttribute) { this.indexedGraph = new Map( - pModules.map((pModule) => [pModule[pIndexAttribute], pModule]) + pModules.map((pModule) => [ + pModule[pIndexAttribute], + this.#normalizeModule(pModule), + ]), ); } + /** + * @param {import("../../types/dependency-cruiser.mjs").IModule[]} pModules + * @param {string} pIndexAttribute + */ constructor(pModules, pIndexAttribute = "source") { - this.init(pModules, pIndexAttribute); + this.#init(pModules, pIndexAttribute); } /** * @param {string} pName - * @return {import("..").IModule} + * @return {IVertex} */ - findModuleByName(pName) { + findVertexByName(pName) { + // @ts-expect-error tsc seems to think indexedGraph can be undefined. However, + // in the constructor it's always set to a Map, and the init method is private return this.indexedGraph.get(pName); } @@ -33,16 +69,17 @@ export default class IndexedModuleGraph { pName, pMaxDepth = 0, pDepth = 0, - pVisited = new Set() + pVisited = new Set(), ) { /** @type {string[]} */ let lReturnValue = []; - const lModule = this.findModuleByName(pName); + const lModule = this.findVertexByName(pName); if (lModule && (!pMaxDepth || pDepth <= pMaxDepth)) { let lDependents = lModule.dependents || []; const lVisited = pVisited.add(pName); + // @TODO this works for modules, but not for folders yet if (lDependents.length > 0) { lDependents .filter((pDependent) => !lVisited.has(pDependent)) @@ -51,8 +88,8 @@ export default class IndexedModuleGraph { pDependent, pMaxDepth, pDepth + 1, - lVisited - ) + lVisited, + ), ); } lReturnValue = Array.from(lVisited); @@ -75,11 +112,11 @@ export default class IndexedModuleGraph { pName, pMaxDepth = 0, pDepth = 0, - pVisited = new Set() + pVisited = new Set(), ) { /** @type {string[]} */ let lReturnValue = []; - const lModule = this.findModuleByName(pName); + const lModule = this.findVertexByName(pName); if (lModule && (!pMaxDepth || pDepth <= pMaxDepth)) { let lDependencies = lModule.dependencies; @@ -87,15 +124,15 @@ export default class IndexedModuleGraph { if (lDependencies.length > 0) { lDependencies - .map(({ resolved }) => resolved) + .map(({ name }) => name) .filter((pDependency) => !lVisited.has(pDependency)) .forEach((pDependency) => this.findTransitiveDependencies( pDependency, pMaxDepth, pDepth + 1, - lVisited - ) + lVisited, + ), ); } lReturnValue = Array.from(lVisited); @@ -111,14 +148,14 @@ export default class IndexedModuleGraph { */ getPath(pFrom, pTo, pVisited = new Set()) { let lReturnValue = []; - const lFromNode = this.findModuleByName(pFrom); + const lFromNode = this.findVertexByName(pFrom); pVisited.add(pFrom); if (lFromNode) { const lDirectUnvisitedDependencies = lFromNode.dependencies - .filter((pDependency) => !pVisited.has(pDependency.resolved)) - .map((pDependency) => pDependency.resolved); + .filter((pDependency) => !pVisited.has(pDependency.name)) + .map((pDependency) => pDependency.name); if (lDirectUnvisitedDependencies.includes(pTo)) { lReturnValue = [pFrom, pTo]; } else { @@ -143,37 +180,58 @@ export default class IndexedModuleGraph { * (source uniquely identifying a node) * @param {string} pCurrentSource The 'source' attribute of the 'to' node to * be traversed - * @param {string} pDependencyName The attribute name of the dependency to use. - * defaults to "resolved" (which is in use for - * modules). For folders pass "name" + * @param {Set=} pVisited Technical parameter - best to leave out of direct calls * @return {string[]} see description above */ - getCycle(pInitialSource, pCurrentSource, pDependencyName, pVisited) { + #getCycle(pInitialSource, pCurrentSource, pVisited) { let lVisited = pVisited || new Set(); - const lCurrentNode = this.findModuleByName(pCurrentSource); - const lDependencies = lCurrentNode.dependencies.filter( - (pDependency) => !lVisited.has(pDependency[pDependencyName]) + const lCurrentVertex = this.findVertexByName(pCurrentSource); + const lDependencies = lCurrentVertex.dependencies.filter( + (pDependency) => !lVisited.has(pDependency.name), ); - const lMatch = lDependencies.find( - (pDependency) => pDependency[pDependencyName] === pInitialSource + const lInitialAsDependency = lDependencies.find( + (pDependency) => pDependency.name === pInitialSource, ); - if (lMatch) { - return [pCurrentSource, lMatch[pDependencyName]]; + if (lInitialAsDependency) { + return pInitialSource === pCurrentSource + ? [lInitialAsDependency.name] + : [pCurrentSource, lInitialAsDependency.name]; } - return lDependencies.reduce((pAll, pDependency) => { - if (!pAll.includes(pCurrentSource)) { - const lCycle = this.getCycle( - pInitialSource, - pDependency[pDependencyName], - pDependencyName, - lVisited.add(pDependency[pDependencyName]) - ); + return lDependencies.reduce( + /** + * @param {Array} pAll + * @param {IEdge} pDependency + * @returns {Array} + */ + (pAll, pDependency) => { + if (!pAll.includes(pCurrentSource)) { + const lCycle = this.#getCycle( + pInitialSource, + pDependency.name, + lVisited.add(pDependency.name), + ); - if (lCycle.length > 0 && !lCycle.includes(pCurrentSource)) { - return pAll.concat(pCurrentSource).concat(lCycle); + if (lCycle.length > 0 && !lCycle.includes(pCurrentSource)) { + return pAll.concat(pCurrentSource).concat(lCycle); + } } - } - return pAll; - }, []); + return pAll; + }, + [], + ); + } + + /** + * Returns the first non-zero length path from pInitialSource to pInitialSource + * Returns the empty array if there is no such path + * + * @param {string} pInitialSource The 'source' attribute of the node to be tested + * (source uniquely identifying a node) + * @param {string} pCurrentSource The 'source' attribute of the 'to' node to + * be traversed + * @return {string[]} see description above + */ + getCycle(pInitialSource, pCurrentSource) { + return this.#getCycle(pInitialSource, pCurrentSource); } } diff --git a/test/graph-utl/__mocks__/cycle-input-graphs.mjs b/test/graph-utl/__mocks__/cycle-input-graphs.mjs index ffb451aba..b7bf7961e 100644 --- a/test/graph-utl/__mocks__/cycle-input-graphs.mjs +++ b/test/graph-utl/__mocks__/cycle-input-graphs.mjs @@ -28,7 +28,8 @@ export default { source: "d", dependencies: [ { - resolved: "d", + resolved: "e", + dependencyTypes: ["aliased", "aliased-subpath-import", "local"], }, ], }, diff --git a/test/graph-utl/indexed-module-graph.spec.mjs b/test/graph-utl/indexed-module-graph.spec.mjs index f5d0fcab0..86b6f0a6b 100644 --- a/test/graph-utl/indexed-module-graph.spec.mjs +++ b/test/graph-utl/indexed-module-graph.spec.mjs @@ -5,22 +5,22 @@ import unIndexedModulesWithoutDependents from "./__mocks__/un-indexed-modules-wi import cycleInputGraphs from "./__mocks__/cycle-input-graphs.mjs"; import IndexedModuleGraph from "#graph-utl/indexed-module-graph.mjs"; -describe("[U] graph-utl/indexed-module-graph - findModuleByName", () => { +describe("[U] graph-utl/indexed-module-graph - findVertexByName", () => { it("searching any module in an empty graph yields undefined", () => { const graph = new IndexedModuleGraph([]); - equal(graph.findModuleByName("any-name"), undefined); + equal(graph.findVertexByName("any-name"), undefined); }); it("searching a non-exiting module in a real graph yields undefined", () => { const graph = new IndexedModuleGraph(unIndexedModules); - equal(graph.findModuleByName("any-name"), undefined); + equal(graph.findVertexByName("any-name"), undefined); }); it("searching for an existing module yields that module", () => { const graph = new IndexedModuleGraph(unIndexedModules); - deepEqual(graph.findModuleByName("src/report/dot/default-theme.js"), { + deepEqual(graph.findVertexByName("src/report/dot/default-theme.js"), { source: "src/report/dot/default-theme.js", dependencies: [], dependents: ["src/report/dot/theming.js"], @@ -408,7 +408,7 @@ describe("[U] graph-utl/indexed-module-graph - getPath", () => { function getCycle(pGraph, pFrom, pToDep) { const lIndexedGraph = new IndexedModuleGraph(pGraph); - return lIndexedGraph.getCycle(pFrom, pToDep, "resolved"); + return lIndexedGraph.getCycle(pFrom, pToDep); } describe("[U] graph-utl/indexed-module-graph - getCycle", () => { @@ -416,7 +416,7 @@ describe("[U] graph-utl/indexed-module-graph - getCycle", () => { deepEqual(getCycle(cycleInputGraphs.A_B, "a", "b"), []); }); it("detects self circular (c <-> c)", () => { - deepEqual(getCycle(cycleInputGraphs.C_C, "c", "c"), ["c", "c"]); + deepEqual(getCycle(cycleInputGraphs.C_C, "c", "c"), ["c"]); }); it("detects 1 step circular (d <-> e)", () => { deepEqual(getCycle(cycleInputGraphs.D_E_D, "d", "e"), ["e", "d"]);