refactor(graph-utl): normalizes name attribute of dependencies on con…

…struction to simplify processing (#887)

## Description

- normalizes name attribute on construction to simplify processing.
Before you had to specify whether you'd use _resolved_ (for regular
modules) or _name_ (for folders) when invoking any method that needed
these (e.g. getCycle).
- refactors the getCycle attribute a bit so it emits the correct output
for self-referencing modules
- makes some methods that were meant to be private really private
- renames some things to be a bit closer to default graph theory
nomenclature

## How Has This Been Tested?

- [x] green ci

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [x] Refactor (non-breaking change which fixes an issue without
changing functionality)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

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,
+      ),
     ),
   }));
 }

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,
   });
 }

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,27 +112,27 @@ 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;
       const lVisited = pVisited.add(pName);
 
       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<string>=} 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<string>} pAll
+       * @param {IEdge} pDependency
+       * @returns {Array<string>}
+       */
+      (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);
   }
 }

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"],
         },
       ],
     },

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,15 +408,15 @@ 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", () => {
   it("leaves non circular dependencies alone", () => {
     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"]);