feat!: makes entries in 'cycle' contain more information (BREAKING fo…

…r API users) (#888)

## Description

- makes getCycle to emit richer content - both a _name_ and
_dependencyTypes_ for now, instead of a string.

This is a breaking change on the API _only_ - rules keep the same
interface and behavior.

## Motivation and Context

So we can define rules on the additional attributes - prerequisite to
fixing #695

## How Has This Been Tested?

- [x] green ci
- [x] additional automated non-regression 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to change) - on the API only.

.c8rc.json

@@ -1,7 +1,7 @@
 {
   "checkCoverage": true,
   "statements": 99.89,
-  "branches": 98.75,
+  "branches": 98.77,
   "functions": 100,
   "lines": 99.89,
   "exclude": [

Makefile

@@ -20,6 +20,7 @@ SCHEMA_SOURCES=tools/schema/baseline-violations.schema.mjs \
 	tools/schema/compound-reaches-type.mjs \
 	tools/schema/configuration.schema.mjs \
 	tools/schema/cruise-result.schema.mjs \
+	tools/schema/mini-dependency-type.mjs \
 	tools/schema/dependencies.mjs \
 	tools/schema/dependency-type.mjs \
 	tools/schema/folders.mjs \

src/cache/cache.mjs

@@ -30,7 +30,7 @@ const EMPTY_CACHE = {
 
 export default class Cache {
   /**
-   * @param {import("../../types/cache-options.js").cacheStrategyType=} pCacheStrategy
+   * @param {import("../../types/cache-options.mjs").cacheStrategyType=} pCacheStrategy
    * @param {boolean=} pCompress
    */
   constructor(pCacheStrategy, pCompress) {
@@ -43,9 +43,9 @@ export default class Cache {
   }
 
   /**
-   * @param {import("../../types/strict-options.js").IStrictCruiseOptions} pCruiseOptions
-   * @param {import("../../types/dependency-cruiser.js").ICruiseResult} pCachedCruiseResult
-   * @param {import("../../types/dependency-cruiser.js").IRevisionData=} pRevisionData
+   * @param {import("../../types/strict-options.mjs").IStrictCruiseOptions} pCruiseOptions
+   * @param {import("../../types/dependency-cruiser.mjs").ICruiseResult} pCachedCruiseResult
+   * @param {import("../../types/dependency-cruiser.mjs").IRevisionData=} pRevisionData
    * @returns {Promise<boolean>}
    */
   async canServeFromCache(pCruiseOptions, pCachedCruiseResult, pRevisionData) {
@@ -68,8 +68,6 @@ export default class Cache {
         this.revisionData,
       ) &&
       optionsAreCompatible(
-        // @ts-expect-error ts(2345) - it's indeed not strict cruise options,
-        // but it will do for now (_it works_)
         pCachedCruiseResult.summary.optionsUsed,
         pCruiseOptions,
       )
@@ -78,7 +76,7 @@ export default class Cache {
 
   /**
    * @param {string} pCacheFolder
-   * @returns {Promise<import("../../types/dependency-cruiser.js").ICruiseResult>}
+   * @returns {Promise<import("../../types/dependency-cruiser.mjs").ICruiseResult>}
    */
   async read(pCacheFolder) {
     try {
@@ -133,8 +131,8 @@ export default class Cache {
 
   /**
    * @param {string} pCacheFolder
-   * @param {import("../../types/dependency-cruiser.js").ICruiseResult} pCruiseResult
-   * @param {import("../../types/dependency-cruiser.js").IRevisionData=} pRevisionData
+   * @param {import("../../types/dependency-cruiser.mjs").ICruiseResult} pCruiseResult
+   * @param {import("../../types/dependency-cruiser.mjs").IRevisionData=} pRevisionData
    */
   async write(pCacheFolder, pCruiseResult, pRevisionData) {
     const lRevisionData = pRevisionData ?? this.revisionData;

src/enrich/summarize/is-same-violation.mjs

@@ -1,13 +1,19 @@
+function pluckName({ name }) {
+  return name;
+}
+
 export default function isSameViolation(pLeftViolation, pRightViolation) {
   let lReturnValue = false;
 
   if (pLeftViolation.rule.name === pRightViolation.rule.name) {
     if (pRightViolation.cycle && pLeftViolation.cycle) {
       lReturnValue =
         pLeftViolation.cycle.length === pRightViolation.cycle.length &&
-        pLeftViolation.cycle.every((pModule) =>
-          pRightViolation.cycle.includes(pModule)
-        );
+        pLeftViolation.cycle
+          .map(pluckName)
+          .every((pModule) =>
+            pRightViolation.cycle.map(pluckName).includes(pModule),
+          );
     } else {
       lReturnValue =
         pLeftViolation.from === pRightViolation.from &&

src/graph-utl/indexed-module-graph.mjs

@@ -6,12 +6,13 @@
  * @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 {import("../../types/shared-types.d.mjs").IMiniDependency} IMiniDependency
  *
  * @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 {
   /**
    * @param {IModuleOrFolder} pModule
@@ -55,7 +56,6 @@ export default class IndexedModuleGraph {
   }
 
   /**
-   *
    * @param {string} pName - the name of the module to find transitive dependents of.
    * @param {number} pMaxDepth - the maximum depth to search for dependents.
    *                  Defaults to 0 (no maximum). To only get direct dependents.
@@ -98,7 +98,6 @@ export default class IndexedModuleGraph {
   }
 
   /**
-   *
    * @param {string} pName - the name of the module to find transitive dependencies of.
    * @param {number} pMaxDepth - the maximum depth to search for dependencies
    *                  Defaults to 0 (no maximum). To only get direct dependencies.
@@ -172,47 +171,70 @@ export default class IndexedModuleGraph {
     return lReturnValue;
   }
 
+  /**
+   *
+   * @param {IEdge} pEdge
+   * @returns {IMiniDependency}
+   */
+  #geldEdge(pEdge) {
+    let lReturnValue = {};
+    lReturnValue.name = pEdge.name;
+    lReturnValue.dependencyTypes = pEdge.dependencyTypes
+      ? pEdge.dependencyTypes
+      : [];
+    return lReturnValue;
+  }
+
   /**
    * 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
+   * @param {IEdge} pCurrentDependency
+   *                                The 'to' node to be traversed as a dependency
+   *                                object of the previous 'from' traversed
    * @param {Set<string>=} pVisited  Technical parameter - best to leave out of direct calls
-   * @return {string[]}             see description above
+   * @return {Array<IMiniDependency>}             see description above
    */
-  #getCycle(pInitialSource, pCurrentSource, pVisited) {
+  #getCycleNew(pInitialSource, pCurrentDependency, pVisited) {
     let lVisited = pVisited || new Set();
-    const lCurrentVertex = this.findVertexByName(pCurrentSource);
-    const lDependencies = lCurrentVertex.dependencies.filter(
+    const lCurrentVertex = this.findVertexByName(pCurrentDependency.name);
+    const lEdges = lCurrentVertex.dependencies.filter(
       (pDependency) => !lVisited.has(pDependency.name),
     );
-    const lInitialAsDependency = lDependencies.find(
+    const lInitialAsDependency = lEdges.find(
       (pDependency) => pDependency.name === pInitialSource,
     );
     if (lInitialAsDependency) {
-      return pInitialSource === pCurrentSource
-        ? [lInitialAsDependency.name]
-        : [pCurrentSource, lInitialAsDependency.name];
+      return pInitialSource === pCurrentDependency.name
+        ? [this.#geldEdge(lInitialAsDependency)]
+        : [
+            this.#geldEdge(pCurrentDependency),
+            this.#geldEdge(lInitialAsDependency),
+          ];
     }
-    return lDependencies.reduce(
+    return lEdges.reduce(
       /**
-       * @param {Array<string>} pAll
+       * @param {Array<IMiniDependency>} pAll
        * @param {IEdge} pDependency
-       * @returns {Array<string>}
+       * @returns {Array<IMiniDependency>}
        */
       (pAll, pDependency) => {
-        if (!pAll.includes(pCurrentSource)) {
-          const lCycle = this.#getCycle(
+        if (!pAll.some((pSome) => pSome.name === pCurrentDependency.name)) {
+          const lCycle = this.#getCycleNew(
             pInitialSource,
-            pDependency.name,
+            pDependency,
             lVisited.add(pDependency.name),
           );
 
-          if (lCycle.length > 0 && !lCycle.includes(pCurrentSource)) {
-            return pAll.concat(pCurrentSource).concat(lCycle);
+          if (
+            lCycle.length > 0 &&
+            !lCycle.some((pSome) => pSome.name === pCurrentDependency.name)
+          ) {
+            return pAll
+              .concat(this.#geldEdge(pCurrentDependency))
+              .concat(lCycle);
           }
         }
         return pAll;
@@ -229,9 +251,17 @@ export default class IndexedModuleGraph {
    *                                (source uniquely identifying a node)
    * @param {string} pCurrentSource The 'source' attribute of the 'to' node to
    *                                be traversed
-   * @return {string[]}             see description above
+   * @return {Array<IMiniDependency>}   see description above
    */
   getCycle(pInitialSource, pCurrentSource) {
-    return this.#getCycle(pInitialSource, pCurrentSource);
+    const lInitialNode = this.findVertexByName(pInitialSource);
+    const lCurrentDependency = lInitialNode.dependencies.find(
+      (pDependency) => pDependency.name === pCurrentSource,
+    );
+
+    if (!lCurrentDependency) {
+      return [];
+    }
+    return this.#getCycleNew(pInitialSource, lCurrentDependency);
   }
 }

src/report/anon/anonymize-path.mjs

@@ -2,7 +2,7 @@ import { anonymizePathElement } from "./anonymize-path-element.mjs";
 
 export const WHITELIST_RE =
   // eslint-disable-next-line security/detect-unsafe-regex
-  /^(|[.]+|~|bin|apps?|cli|src|libs?|configs?|components?|fixtures?|helpers?|i18n|index\.(jsx?|[mc]js|d\.ts|tsx?|vue|coffee|ls)|_?_?mocks?_?_?|node_modules|packages?|package\.json|scripts?|services?|sources?|specs?|_?_?tests?_?_?|types?|uti?ls?)$/;
+  /^(|[.]+|~|bin|apps?|cli|src|libs?|configs?|components?|fixtures?|helpers?|i18n|index\.(jsx?|[mc]js|d\.ts|tsx?|vue|coffee|ls)|_?_?mocks?_?_?|node_modules|packages?|package\.json|scripts?|services?|sources?|specs?|_?_?tests?_?_?|types?|uti?ls?|tools)$/;
 
 /**
  * Kind of smartly anonymizes paths, by

src/report/anon/index.mjs

@@ -2,15 +2,26 @@ import has from "lodash/has.js";
 import { anonymizePath, WHITELIST_RE } from "./anonymize-path.mjs";
 
 function anonymizePathArray(pPathArray, pWordList) {
+  // the coverage ignore is here because the || [] branch isn't taken when running
+  // tests and with the current setup of the anonymize module that's not going
+  // to change. Still want to keep the branch from robustness perspective though.
+  /* c8 ignore next 1 */
   return (pPathArray || []).map((pPath) => anonymizePath(pPath, pWordList));
 }
 
+function anonymizeCycleArray(pCycleArray, pWordList, pAttribute = "name") {
+  return (pCycleArray || []).map((pCycle) => ({
+    ...pCycle,
+    [pAttribute]: anonymizePath(pCycle.name, pWordList),
+  }));
+}
+
 function anonymizeDependencies(pDependencies, pWordList) {
   return pDependencies.map((pDependency) => ({
     ...pDependency,
     resolved: anonymizePath(pDependency.resolved, pWordList),
     module: anonymizePath(pDependency.module, pWordList),
-    cycle: anonymizePathArray(pDependency.cycle, pWordList),
+    cycle: anonymizeCycleArray(pDependency.cycle, pWordList),
   }));
 }
 
@@ -74,7 +85,7 @@ function anonymizeFolders(pFolders, pWordList) {
           name: anonymizePath(pDependency.name, pWordList),
         };
         if (lReturnDependencies.cycle) {
-          lReturnDependencies.cycle = anonymizePathArray(
+          lReturnDependencies.cycle = anonymizeCycleArray(
             pDependency.cycle,
             pWordList,
           );
@@ -98,7 +109,7 @@ function anonymizeViolations(pViolations, pWordList) {
       ...pViolation,
       from: anonymizePath(pViolation.from, pWordList),
       to: anonymizePath(pViolation.to, pWordList),
-      cycle: anonymizePathArray(pViolation.cycle, pWordList),
+      cycle: anonymizeCycleArray(pViolation.cycle, pWordList),
     };
     if (pViolation.via) {
       lReturnValue.via = anonymizePathArray(pViolation.via, pWordList);
@@ -145,7 +156,7 @@ function sanitizeWordList(pWordList) {
  * - summary.violations.to
  * - summary.violations.cycle[m]
  *
- * (note: the algorith _removes_ elements from pWordList to prevent duplicates,
+ * (note: the algorithm _removes_ elements from pWordList to prevent duplicates,
  * so if the word list is precious to you - pass a clone)
  *
  * @param {import("../../../types/cruise-result.mjs").ICruiseResult} pResults - the output of a dependency-cruise adhering to ../schema/cruise-result.schema.json

src/report/anon/random-string.mjs

@@ -42,7 +42,7 @@ function getRandomChar(pChar) {
 
 /**
  * Returns a random string with the same length as pString
- * acii characters - respecting case & numbers + leaving separators
+ * ascii characters - respecting case & numbers + leaving separators
  * (-, _, .) in place
  *
  * hello => tbkwd

src/report/azure-devops.mjs

@@ -42,7 +42,9 @@ function formatDependencyViolation(pViolation) {
  * @returns {string}
  */
 function formatCycleViolation(pViolation) {
-  return `${pViolation.from} -> ${pViolation.cycle.join(" -> ")}`;
+  return `${pViolation.from} -> ${pViolation.cycle
+    .map(({ name }) => name)
+    .join(" -> ")}`;
 }
 
 /**

src/report/error-html/utl.mjs

@@ -30,7 +30,7 @@ function mergeCountsIntoRule(pRule, pViolationCounts) {
 }
 
 function formatCycleTo(pViolation) {
-  return pViolation.cycle.join(" &rightarrow;<br/>");
+  return pViolation.cycle.map(({ name }) => name).join(" &rightarrow;<br/>");
 }
 
 function formatReachabilityTo(pViolation) {

src/report/error.mjs

@@ -17,6 +17,15 @@ const SEVERITY2CHALK = new Map([
 
 const EXTRA_PATH_INFORMATION_INDENT = 6;
 
+function formatExtraCycleInformation(pCycle) {
+  return EOL.concat(
+    wrapAndIndent(
+      pCycle.map(({ name }) => name).join(` ${figures.arrowRight} ${EOL}`),
+      EXTRA_PATH_INFORMATION_INDENT,
+    ),
+  );
+}
+
 function formatExtraPathInformation(pExtra) {
   return EOL.concat(
     wrapAndIndent(
@@ -39,7 +48,7 @@ function formatDependencyViolation(pViolation) {
 function formatCycleViolation(pViolation) {
   return `${chalk.bold(pViolation.from)} ${
     figures.arrowRight
-  } ${formatExtraPathInformation(pViolation.cycle)}`;
+  } ${formatExtraCycleInformation(pViolation.cycle)}`;
 }
 
 function formatReachabilityViolation(pViolation) {