feat: enables matching transitive dependencies in 'required' rules (#975

) ## Description - [x] adds the `reachable` attribute to the 'required' rules schema - [x] adds reachable detection to the enrich step - [x] processes `reachable` in validating against 'required' rules - [x] update the documentation ## Motivation and Context See #973 ## How Has This Been Tested? - [x] green ci - [x] additional automated non-regression tests ## Example usage ```javascript /** @type {import('dependency-cruiser').IConfiguration} */ export default { required: [ { name: "must-reach-meta-cjs", comment: "all modules much reach, somehow, meta.cjs", module: { path: "^src/", }, to: { path: "^src/meta[.]cjs$", reachable: true, } } ], options: { // ... } } ``` ## 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) - [ ] Breaking change (fix or feature that would cause existing functionality to change)
sverweij · Dec 12, 2024 · dd81580 · dd81580
1 parent fdbb72a
commit dd81580
Show file tree

Hide file tree

Showing 11 changed files with 317 additions and 18 deletions.
diff --git a/doc/rules-reference.md b/doc/rules-reference.md
@@ -93,6 +93,9 @@ controller _must depend on_ the base controller'.
 modules the rule applies to and the `to` describes what dependencies that module
 should exactly have.
 
+To check for _direct_ dependencies (controllers must directly depend on the
+base controller):
+
 ```javascript
 {
   required: [
@@ -102,18 +105,48 @@ should exactly have.
         "Each controller should inherit from the framework's base controller",
       module: {
         // every module that matches the pattern specified in path & pathNot ...
-        path: "-controller\\.js$",
-        pathNot: "framework/base-controller\\.js$",
+        path: "-controller[.]js$",
+        pathNot: "framework/base-controller[.]js$",
+      },
+      to: {
+        // ... must depend at least once on the framework's base controller
+        path: "^src/framework/base-controller[.]js$",
+      },
+    },
+  ];
+}
+```
+
+To check for _indirect_ ('transitive') dependencies (controllers must depend
+on the base controller, either directly or via other modules) add the `reachable`
+attribute to `to` part of the rule, like so:
+
+```javascript
+{
+  required: [
+    {
+      name: "controllers-transitively-inherit-from-base",
+      comment:
+        "Each controller should inherit from the framework's base controller",
+      module: {
+        // every module that matches the pattern specified in path & pathNot ...
+        path: "-controller[.]js$",
+        pathNot: "framework/base-controller[.]js$",
       },
       to: {
         // ... must depend at least once on the framework's base controller
-        path: "^src/framework/base-controller\\.js$",
+        path: "^src/framework/base-controller[.]js$",
+        // ... either directly or indirectly
+        reachable: true,
       },
     },
   ];
 }
 ```
 
+> `reachable: false` is the same as leaving the reachable attribute out; so it
+> does 'nothing'.
+
 ### `extends`
 
 This takes one or more file path to other dependency-cruiser-configs. When

diff --git a/src/enrich/derive/reachable.mjs b/src/enrich/derive/reachable.mjs
@@ -13,18 +13,30 @@ function getReachableRules(pRuleSet) {
       (pRuleSet?.allowed ?? []).filter((pRule) =>
         Object.hasOwn(pRule?.to ?? {}, "reachable"),
       ),
+    )
+    .concat(
+      (pRuleSet?.required ?? []).filter((pRule) =>
+        Object.hasOwn(pRule?.to ?? {}, "reachable"),
+      ),
     );
 }
 
 function isModuleInRuleFrom(pRule) {
-  return (pModule) =>
-    (!pRule.from.path || pModule.source.match(pRule.from.path)) &&
-    (!pRule.from.pathNot || !pModule.source.match(pRule.from.pathNot));
+  return (pModule) => {
+    const lRuleFrom = pRule.from ?? pRule.module;
+    if (lRuleFrom) {
+      return (
+        (!lRuleFrom.path || pModule.source.match(lRuleFrom.path)) &&
+        (!lRuleFrom.pathNot || !pModule.source.match(lRuleFrom.pathNot))
+      );
+    }
+    return false;
+  };
 }
 
 function isModuleInRuleTo(pRule, pModuleTo, pModuleFrom) {
   const lGroups = pModuleFrom
-    ? extractGroups(pRule.from, pModuleFrom.source)
+    ? extractGroups(pRule.from ?? pRule.module, pModuleFrom.source)
     : [];
 
   return (
@@ -91,7 +103,11 @@ function hasCapturingGroups(pRule) {
 function shouldAddReachable(pRule, pModuleTo, pGraph) {
   let lReturnValue = false;
 
-  if (pRule.to.reachable === false || pRule.name === "not-in-allowed") {
+  if (
+    pRule.to.reachable === false ||
+    pRule.name === "not-in-allowed" ||
+    pRule.module
+  ) {
     if (hasCapturingGroups(pRule)) {
       const lModulesFrom = pGraph.filter(isModuleInRuleFrom(pRule));
 

diff --git a/src/schema/configuration.schema.json b/src/schema/configuration.schema.json
diff --git a/src/schema/configuration.schema.mjs b/src/schema/configuration.schema.mjs
diff --git a/src/schema/cruise-result.schema.json b/src/schema/cruise-result.schema.json
diff --git a/src/schema/cruise-result.schema.mjs b/src/schema/cruise-result.schema.mjs
diff --git a/src/validate/violates-required-rule.mjs b/src/validate/violates-required-rule.mjs
@@ -2,7 +2,9 @@ import {
   matchesToPath,
   matchesModulePath,
   matchesModulePathNot,
+  matchToModulePath,
 } from "./matchers.mjs";
+import { matchesReachesRule } from "./match-module-rule-helpers.mjs";
 import { extractGroups } from "#utl/regex-util.mjs";
 
 /**
@@ -20,11 +22,19 @@ export default function violatesRequiredRule(pRule, pModule) {
     matchesModulePath(pRule, pModule) &&
     matchesModulePathNot(pRule, pModule)
   ) {
-    const lGroups = extractGroups(pRule.module, pModule.source);
+    if (pRule.to.reachable) {
+      lReturnValue = !matchesReachesRule(pRule, pModule);
+    }
 
-    lReturnValue = !pModule.dependencies.some((pDependency) =>
-      matchesToPath(pRule, pDependency, lGroups),
-    );
+    if (lReturnValue || !pRule.to.reachable) {
+      const lGroups = extractGroups(pRule.module, pModule.source);
+      const lMatchesSelf = matchToModulePath(pRule, pModule, lGroups);
+      lReturnValue =
+        !lMatchesSelf &&
+        !pModule.dependencies.some((pDependency) =>
+          matchesToPath(pRule, pDependency, lGroups),
+        );
+    }
   }
   return lReturnValue;
 }
diff --git a/test/enrich/derive/reachable.spec.mjs b/test/enrich/derive/reachable.spec.mjs
@@ -114,7 +114,17 @@ describe("[U] enrich/derive/reachable/index - reachability detection", () => {
     deepEqual(addReachability(GRAPH, normalize({})), GRAPH);
   });
 
-  it('returns the reachability annotated graph when a rule set with forbidden "reachable" in it', () => {
+  it("returns the input graph when passed a rule set with only a rul that doesn't have a 'from' or a 'module'", () => {
+    deepEqual(
+      addReachability(
+        GRAPH,
+        normalize({ required: [{ thing: {}, to: { reachable: true } }] }),
+      ),
+      GRAPH,
+    );
+  });
+
+  it('returns the reachability annotated graph when a rule set with forbidden "reachable" in it (forbidden rule)', () => {
     const lForbiddenReachabilityRuleSetHajoo = {
       forbidden: [
         {
@@ -134,8 +144,8 @@ describe("[U] enrich/derive/reachable/index - reachability detection", () => {
     const lForbiddenReachabilityRuleSetHajoo = {
       allowed: [
         {
-          from: { path: "src/[^.]+\\.js" },
-          to: { path: "./src/hajoo\\.js$", reachable: true },
+          from: { path: "src/[^.]+[.]js" },
+          to: { path: "./src/hajoo[.]js$", reachable: true },
         },
       ],
     };
@@ -274,6 +284,78 @@ describe("[U] enrich/derive/reachable/index - reachability detection", () => {
     );
   });
 
+  it('returns the reachability annotated graph when a rule set with allowed "reachable" in it ("required" rules)', () => {
+    const lRequiredReachabilityRuleSetHajoo = {
+      required: [
+        {
+          name: "must-reach-hajoo-somehow",
+          module: { path: "src/[^.]+[.]js" },
+          to: { path: "./src/hajoo[.]js$", reachable: true },
+        },
+      ],
+    };
+    const lExpected = [
+      {
+        source: "./src/index.js",
+        dependencies: [
+          {
+            resolved: "./src/intermediate.js",
+          },
+        ],
+        reaches: [
+          {
+            asDefinedInRule: "must-reach-hajoo-somehow",
+            modules: [
+              {
+                source: "./src/hajoo.js",
+                via: [
+                  { name: "./src/intermediate.js", dependencyTypes: [] },
+                  { name: "./src/hajoo.js", dependencyTypes: [] },
+                ],
+              },
+            ],
+          },
+        ],
+      },
+      {
+        source: "./src/intermediate.js",
+        dependencies: [
+          {
+            resolved: "./src/index.js",
+          },
+          {
+            resolved: "./src/hajoo.js",
+          },
+        ],
+        reaches: [
+          {
+            asDefinedInRule: "must-reach-hajoo-somehow",
+            modules: [
+              {
+                source: "./src/hajoo.js",
+                via: [{ name: "./src/hajoo.js", dependencyTypes: [] }],
+              },
+            ],
+          },
+        ],
+      },
+      {
+        source: "./src/hajoo.js",
+        dependencies: [],
+        reachable: [
+          {
+            value: true,
+            matchedFrom: "./src/index.js",
+            asDefinedInRule: "must-reach-hajoo-somehow",
+          },
+        ],
+      },
+    ];
+    const lNormalizedGraph = normalize(lRequiredReachabilityRuleSetHajoo);
+    const lResult = addReachability(GRAPH, lNormalizedGraph);
+    deepEqual(lResult, lExpected);
+  });
+
   it('returns the reachability annotated graph when a rule set with allowed "reachable" in it (without a rule name - multiple matches)', () => {
     const lForbiddenReachabilityRuleSetHajoo = {
       allowed: [