Skip to content

Commit

Permalink
feat!: makes entries in reachability via's contain more information (…
Browse files Browse the repository at this point in the history
…BREAKING for API users) (#895)

## Description

- Same as #888, but for the path detection algorithm underlying
'reaches' and 'reachable' rules

The addition to the _rule_ will be in a separate PR

## Motivation and Context

- fixing #763 - this PR is a prerequisite to implement the feature
requested in there
- feature parity with similar restrictions on cycles

## How Has This Been Tested?

- [x] green ci
- [x] updated automated regression tests
- [x] dog-fooding on dependency-cruiser

## 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) => for API only

## Checklist

- [x] 📖

  - My change doesn't require a documentation update, or ...
  - it _does_ and I have updated it

- [x] ⚖️
- The contribution will be subject to [The MIT
license](https://github.com/sverweij/dependency-cruiser/blob/main/LICENSE),
and I'm OK with that.
  - The contribution is my own original work.
- I am ok with the stuff in
[**CONTRIBUTING.md**](https://github.com/sverweij/dependency-cruiser/blob/main/.github/CONTRIBUTING.md).
  • Loading branch information
sverweij authored Dec 25, 2023
1 parent 05d9795 commit e8cda75
Show file tree
Hide file tree
Showing 32 changed files with 638 additions and 167 deletions.
19 changes: 17 additions & 2 deletions src/config-utl/extract-known-violations.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import makeAbsolute from "./make-absolute.mjs";
* @returns {IViolation}
*/
function makeForwardCompatible(pKnownViolation) {
let lReturnValue = pKnownViolation;
if (Boolean(pKnownViolation.cycle)) {
return {
lReturnValue = {
...pKnownViolation,
cycle: pKnownViolation.cycle.map((pModule) => {
if (Boolean(pModule.name)) {
Expand All @@ -34,7 +35,21 @@ function makeForwardCompatible(pKnownViolation) {
}),
};
}
return pKnownViolation;
if (Boolean(pKnownViolation.via)) {
lReturnValue = {
...pKnownViolation,
via: pKnownViolation.via.map((pModule) => {
if (Boolean(pModule.name)) {
return pModule;
}
return {
name: pModule,
dependencyTypes: [],
};
}),
};
}
return lReturnValue;
}

export default async function extractKnownViolations(pKnownViolationsFileName) {
Expand Down
22 changes: 10 additions & 12 deletions src/enrich/derive/reachable.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,18 @@ function mergeReachableProperties(pModule, pRule, pPath, pModuleFrom) {
const lIndexExistingReachable = lReachables.findIndex(
(pReachable) => pReachable.asDefinedInRule === pRule.name,
);
const lIsReachable = pPath.length > 1;
const lIsReachable = pPath.length > 0;

if (lIndexExistingReachable > -1) {
lReachables[lIndexExistingReachable].value =
lReachables[lIndexExistingReachable].value || lIsReachable;
return lReachables;
} else {
return lReachables.concat({
value: lIsReachable,
asDefinedInRule: pRule.name,
matchedFrom: pModuleFrom,
});
}
return lReachables.concat({
value: lIsReachable,
asDefinedInRule: pRule.name,
matchedFrom: pModuleFrom,
});
}

function mergeReachesProperties(pFromModule, pToModule, pRule, pPath) {
Expand All @@ -63,12 +62,11 @@ function mergeReachesProperties(pFromModule, pToModule, pRule, pPath) {
via: pPath,
});
return lReaches;
} else {
return lReaches.concat({
asDefinedInRule: pRule.name,
modules: [{ source: pToModule.source, via: pPath }],
});
}
return lReaches.concat({
asDefinedInRule: pRule.name,
modules: [{ source: pToModule.source, via: pPath }],
});
}

function shouldAddReaches(pRule, pModule) {
Expand Down
68 changes: 36 additions & 32 deletions src/graph-utl/indexed-module-graph.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -139,38 +139,6 @@ export default class IndexedModuleGraph {
return lReturnValue;
}

/**
* @param {string} pFrom
* @param {string} pTo
* @param {Set<string>} pVisited
* @returns {string[]}
*/
getPath(pFrom, pTo, pVisited = new Set()) {
let lReturnValue = [];
const lFromNode = this.findVertexByName(pFrom);

pVisited.add(pFrom);

if (lFromNode) {
const lDirectUnvisitedDependencies = lFromNode.dependencies
.filter((pDependency) => !pVisited.has(pDependency.name))
.map((pDependency) => pDependency.name);
if (lDirectUnvisitedDependencies.includes(pTo)) {
lReturnValue = [pFrom, pTo];
} else {
for (const lFrom of lDirectUnvisitedDependencies) {
let lCandidatePath = this.getPath(lFrom, pTo, pVisited);
// eslint-disable-next-line max-depth
if (lCandidatePath.length > 0) {
lReturnValue = [pFrom].concat(lCandidatePath);
break;
}
}
}
}
return lReturnValue;
}

/**
*
* @param {IEdge} pEdge
Expand All @@ -185,6 +153,42 @@ export default class IndexedModuleGraph {
return lReturnValue;
}

/**
* @param {string} pFrom
* @param {string} pTo
* @param {Set<string>} pVisited
* @returns {Array<IMiniDependency>}
*/
getPath(pFrom, pTo, pVisited = new Set()) {
const lFromNode = this.findVertexByName(pFrom);

pVisited.add(pFrom);

if (!lFromNode) {
return [];
}

const lDirectUnvisitedDependencies = lFromNode.dependencies
.filter((pDependency) => !pVisited.has(pDependency.name))
.map(this.#geldEdge);
const lFoundDependency = lDirectUnvisitedDependencies.find(
(pDependency) => pDependency.name === pTo,
);

if (lFoundDependency) {
return [lFoundDependency];
}

for (const lNewFrom of lDirectUnvisitedDependencies) {
let lCandidatePath = this.getPath(lNewFrom.name, pTo, pVisited);

if (lCandidatePath.length > 0) {
return [lNewFrom].concat(lCandidatePath);
}
}
return [];
}

/**
* Returns the first non-zero length path from pInitialSource to pInitialSource
* Returns the empty array if there is no such path
Expand Down
25 changes: 16 additions & 9 deletions src/report/anon/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ function anonymizePathArray(pPathArray, pWordList) {
return (pPathArray || []).map((pPath) => anonymizePath(pPath, pWordList));
}

function anonymizeCycleArray(pCycleArray, pWordList, pAttribute = "name") {
return (pCycleArray || []).map((pCycle) => ({
...pCycle,
[pAttribute]: anonymizePath(pCycle.name, pWordList),
function anonymizeMiniDependencyArray(
pMiniDependencyArray,
pWordList,
pAttribute = "name",
) {
return (pMiniDependencyArray || []).map((pMiniDependency) => ({
...pMiniDependency,
[pAttribute]: anonymizePath(pMiniDependency.name, pWordList),
}));
}

Expand All @@ -21,15 +25,15 @@ function anonymizeDependencies(pDependencies, pWordList) {
...pDependency,
resolved: anonymizePath(pDependency.resolved, pWordList),
module: anonymizePath(pDependency.module, pWordList),
cycle: anonymizeCycleArray(pDependency.cycle, pWordList),
cycle: anonymizeMiniDependencyArray(pDependency.cycle, pWordList),
}));
}

function anonymizeReachesModule(pWordList) {
return (pModule) => ({
...pModule,
source: anonymizePath(pModule.source, pWordList),
via: anonymizePathArray(pModule.via, pWordList),
via: anonymizeMiniDependencyArray(pModule.via, pWordList),
});
}

Expand Down Expand Up @@ -85,7 +89,7 @@ function anonymizeFolders(pFolders, pWordList) {
name: anonymizePath(pDependency.name, pWordList),
};
if (lReturnDependencies.cycle) {
lReturnDependencies.cycle = anonymizeCycleArray(
lReturnDependencies.cycle = anonymizeMiniDependencyArray(
pDependency.cycle,
pWordList,
);
Expand All @@ -109,10 +113,13 @@ function anonymizeViolations(pViolations, pWordList) {
...pViolation,
from: anonymizePath(pViolation.from, pWordList),
to: anonymizePath(pViolation.to, pWordList),
cycle: anonymizeCycleArray(pViolation.cycle, pWordList),
cycle: anonymizeMiniDependencyArray(pViolation.cycle, pWordList),
};
if (pViolation.via) {
lReturnValue.via = anonymizePathArray(pViolation.via, pWordList);
lReturnValue.via = anonymizeMiniDependencyArray(
pViolation.via,
pWordList,
);
}
return lReturnValue;
});
Expand Down
6 changes: 3 additions & 3 deletions src/report/azure-devops.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ function formatCycleViolation(pViolation) {
* @returns {string}
*/
function formatReachabilityViolation(pViolation) {
return `${pViolation.from} -> ${pViolation.to} (via ${pViolation.via.join(
" -> ",
)})`;
return `${pViolation.from} -> ${pViolation.to} (via ${pViolation.via
.map(({ name }) => name)
.join(" -> ")})`;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/report/error-html/utl.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ function formatCycleTo(pViolation) {
}

function formatReachabilityTo(pViolation) {
return `${pViolation.to}<br/>${pViolation.via.join(" &rightarrow;<br/>")}`;
return `${pViolation.to}<br/>${pViolation.via
.map(({ name }) => name)
.join(" &rightarrow;<br/>")}`;
}

function formatDependencyTo(pViolation) {
Expand Down
19 changes: 6 additions & 13 deletions src/report/error.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,12 @@ const SEVERITY2CHALK = new Map([

const EXTRA_PATH_INFORMATION_INDENT = 6;

function formatExtraCycleInformation(pCycle) {
function formatMiniDependency(pMiniDependency) {
return EOL.concat(
wrapAndIndent(
pCycle.map(({ name }) => name).join(` ${figures.arrowRight} ${EOL}`),
EXTRA_PATH_INFORMATION_INDENT,
),
);
}

function formatExtraPathInformation(pExtra) {
return EOL.concat(
wrapAndIndent(
pExtra.join(` ${figures.arrowRight} ${EOL}`),
pMiniDependency
.map(({ name }) => name)
.join(` ${figures.arrowRight} ${EOL}`),
EXTRA_PATH_INFORMATION_INDENT,
),
);
Expand All @@ -48,13 +41,13 @@ function formatDependencyViolation(pViolation) {
function formatCycleViolation(pViolation) {
return `${chalk.bold(pViolation.from)} ${
figures.arrowRight
} ${formatExtraCycleInformation(pViolation.cycle)}`;
} ${formatMiniDependency(pViolation.cycle)}`;
}

function formatReachabilityViolation(pViolation) {
return `${chalk.bold(pViolation.from)} ${figures.arrowRight} ${chalk.bold(
pViolation.to,
)}${formatExtraPathInformation(pViolation.via)}`;
)}${formatMiniDependency(pViolation.via)}`;
}

function formatInstabilityViolation(pViolation) {
Expand Down
6 changes: 3 additions & 3 deletions src/report/teamcity.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ function formatCycleViolation(pViolation) {
}

function formatReachabilityViolation(pViolation) {
return `${formatDependencyViolation(pViolation)} ${pViolation.via.join(
" -> ",
)}`;
return `${formatDependencyViolation(pViolation)} ${pViolation.via
.map(({ name }) => name)
.join(" -> ")}`;
}

function formatInstabilityViolation(pViolation) {
Expand Down
2 changes: 1 addition & 1 deletion src/schema/baseline-violations.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/schema/baseline-violations.schema.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/schema/configuration.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/schema/configuration.schema.mjs

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/schema/cruise-result.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/schema/cruise-result.schema.mjs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[
{
"from": "src/foo/index.js",
"to": "src/utl/quux.js",
"rule": {
"severity": "error",
"name": "utl-not-reachable-from-extract"
},
"via": [
{ "name": "src/foo/index.js", "dependencyTypes": [] },
{ "name": "src/foo/quuz.js", "dependencyTypes": [] },
{ "name": "src/utl/quux.js", "dependencyTypes": [] }
],
"cycle": []
},
{
"from": "src/foo/index.js",
"to": "src/utl/grault.js",
"rule": {
"severity": "error",
"name": "utl-not-reachable-from-extract"
},
"via": [
{ "name": "src/foo/index.js", "dependencyTypes": [] },
{ "name": "src/foo/corge.js", "dependencyTypes": [] },
{ "name": "src/utl/grault.js", "dependencyTypes": [] }
],
"cycle": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[
{
"from": "src/foo/index.js",
"to": "src/utl/quux.js",
"rule": {
"severity": "error",
"name": "utl-not-reachable-from-extract"
},
"via": ["src/foo/index.js", "src/foo/quuz.js", "src/utl/quux.js"],
"cycle": []
},
{
"from": "src/foo/index.js",
"to": "src/utl/grault.js",
"rule": {
"severity": "error",
"name": "utl-not-reachable-from-extract"
},
"via": ["src/foo/index.js", "src/foo/corge.js", "src/utl/grault.js"],
"cycle": []
}
]
Loading

0 comments on commit e8cda75

Please sign in to comment.