diff --git a/configs/.dependency-cruiser-show-metrics-config.json b/configs/.dependency-cruiser-show-metrics-config.json index 60f684ae1..775f3114c 100644 --- a/configs/.dependency-cruiser-show-metrics-config.json +++ b/configs/.dependency-cruiser-show-metrics-config.json @@ -22,6 +22,16 @@ "to": { "moreUnstable": true } + }, + { + "name": "no-folder-cycles", + "scope": "folder", + "severity": "warn", + "comment": "This folder is part of a circular relationship. You might want to refactor that a bit.", + "from": {}, + "to": { + "circular": true + } } ], "options": { diff --git a/src/main/options/normalize.js b/src/main/options/normalize.js index 64e677f9e..9dcc8709e 100644 --- a/src/main/options/normalize.js +++ b/src/main/options/normalize.js @@ -1,6 +1,7 @@ /* eslint-disable security/detect-object-injection */ -const _clone = require("lodash/clone"); -const _has = require("lodash/has"); +const get = require("lodash/get"); +const clone = require("lodash/clone"); +const has = require("lodash/has"); const normalizeREProperties = require("../utl/normalize-re-properties"); const defaults = require("./defaults.js"); @@ -67,7 +68,12 @@ function normalizeCollapse(pCollapse) { } function hasMetricsRule(pRule) { - return _has(pRule, "to.moreUnstable"); + // TODO: philosophy: is a rule with 'folder' in it a metrics rule? + // Or is it a misuse to ensure folder derivations (like cycles) get + // kicked off? + return ( + has(pRule, "to.moreUnstable") || get(pRule, "scope", "module") === "folder" + ); } function ruleSetHasMetricsRule(pRuleSet) { @@ -107,7 +113,7 @@ function normalizeCruiseOptions(pOptions) { lReturnValue.maxDepth = Number.parseInt(lReturnValue.maxDepth, 10); lReturnValue.moduleSystems = uniq(lReturnValue.moduleSystems.sort()); - if (_has(lReturnValue, "collapse")) { + if (has(lReturnValue, "collapse")) { lReturnValue.collapse = normalizeCollapse(lReturnValue.collapse); } // TODO: further down the execution path code still relies on .doNotFollow @@ -133,9 +139,9 @@ function normalizeCruiseOptions(pOptions) { } function normalizeFormatOptions(pFormatOptions) { - const lFormatOptions = _clone(pFormatOptions); + const lFormatOptions = clone(pFormatOptions); - if (_has(lFormatOptions, "collapse")) { + if (has(lFormatOptions, "collapse")) { lFormatOptions.collapse = normalizeCollapse(lFormatOptions.collapse); } return normalizeFilterOptions(lFormatOptions, [ diff --git a/src/validate/match-folder-dependency-rule.js b/src/validate/match-folder-dependency-rule.js index 75781bdf5..498c1c7cb 100644 --- a/src/validate/match-folder-dependency-rule.js +++ b/src/validate/match-folder-dependency-rule.js @@ -5,9 +5,8 @@ function match(pFromFolder, pToFolder) { return (pRule) => // TODO: add path rules - they need to be frippled from the ones // already in place for modules - // TODO: same for cycles - but these will additionally have to be - // yognated with an adapted cycle detection for folders - matchers.toIsMoreUnstable(pRule, pFromFolder, pToFolder); + matchers.toIsMoreUnstable(pRule, pFromFolder, pToFolder) && + matchers.propertyEquals(pRule, pToFolder, "circular"); } const isInteresting = (pRule) => diff --git a/test/validate/match-folder-rule.spec.mjs b/test/validate/match-folder-rule.spec.mjs index 8e6a8c1c3..27f331b0a 100644 --- a/test/validate/match-folder-rule.spec.mjs +++ b/test/validate/match-folder-rule.spec.mjs @@ -3,11 +3,15 @@ import matchFolderRule from "../../src/validate/match-folder-dependency-rule.js" const EMPTY_RULE = { scope: "folder", from: {}, to: {} }; const SDP_RULE = { scope: "folder", from: {}, to: { moreUnstable: true } }; +const CYCLE_RULE = { scope: "folder", from: {}, to: { circular: true } }; -describe("[I] validate/match-folder-dependency-rule - match", () => { +describe("[I] validate/match-folder-dependency-rule - match generic", () => { it("empty rule => match all the things (empty from & to)", () => { expect(matchFolderRule.match({}, {})(EMPTY_RULE)).to.equal(true); }); +}); + +describe("[I] validate/match-folder-dependency-rule - match SDP", () => { it("rule with a restriction on the to doesn't match when the criterium is not met (data missing)", () => { expect(matchFolderRule.match({}, {})(SDP_RULE)).to.equal(false); }); @@ -22,3 +26,19 @@ describe("[I] validate/match-folder-dependency-rule - match", () => { ).to.equal(true); }); }); + +describe("[I] validate/match-folder-dependency-rule - match cycle", () => { + it("rule with a restriction on the to doesn't match when the criterium is not met (data missing)", () => { + expect(matchFolderRule.match({}, {})(CYCLE_RULE)).to.equal(false); + }); + it("rule with a restriction on the to doesn't match when the criterium is not met (data there)", () => { + expect(matchFolderRule.match({}, { circular: false })(CYCLE_RULE)).to.equal( + false + ); + }); + it("rule with a restriction on the to doesn't match when the criterium is met (data there)", () => { + expect(matchFolderRule.match({}, { circular: true })(CYCLE_RULE)).to.equal( + true + ); + }); +});