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!: makes entries in 'cycle' contain more information (BREAKING for API users) #888

Merged
merged 14 commits into from
Dec 17, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .c8rc.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"checkCoverage": true,
"statements": 99.89,
"branches": 98.75,
"branches": 98.77,
"functions": 100,
"lines": 99.89,
"exclude": [
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
16 changes: 7 additions & 9 deletions src/cache/cache.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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,
)
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 9 additions & 3 deletions src/enrich/summarize/is-same-violation.mjs
Original file line number Diff line number Diff line change
@@ -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 &&
Expand Down
76 changes: 53 additions & 23 deletions src/graph-utl/indexed-module-graph.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/report/anon/anonymize-path.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 15 additions & 4 deletions src/report/anon/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}));
}

Expand Down Expand Up @@ -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,
);
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/report/anon/random-string.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion src/report/azure-devops.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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(" -> ")}`;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/report/error-html/utl.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 10 additions & 1 deletion src/report/error.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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) {
Expand Down