Feature request: match src-dst import paths using a callback

[tlaitinen]

Many thanks for this wonderful tool. Have you considered allowing to specify a callback in a rule instead of a pair of regular expressions? With a callback, users could encode arbitrary rules when regular expressions fall short or are challenging to implement.

Context

We have a complex code base and we want to implement very strict recursive import restrictions.

Expected Behavior

{
  "name": "complex-rule",
  "comment": "A comment about the rule",
  "severity": "error",
  "test": (src,dst) => { /* return true if the import src-dst is allowed */ }
}

Current Behavior

This question is just to find out what you think about the possibility of using callbacks, so I'm not including a detailed example at this point.

Possible Solution

If you are open to such an idea, we can provide a proof-of-concept implementation.

Considered alternatives

It may be possible to encode some of the rules we'd like to have using the current rule set, but there are probably reasonable ideas that cannot be implemented using the available rules.

[sverweij]

Hi @tlaitinen thanks for this suggestion. I'd be interested in the types of rules you want to endeavour. Callbacks are powerful, but also foot guns for security and maintenance, so if possible I'd like to avoid having to use them.

[tlaitinen]

Callbacks can certainly be powerful and dangerous. The configuration file .dependency-cruiser.js is evaluated by dependency-cruiser, so it looks like allowing callbacks would not compromise security further. But, if we manage to implement the restrictions we’d like to have using the current rules, there’s no need for callbacks.

We try to keep our React components as self-contained as possible, that is, they should not import from their parent components, and this restriction should be applied recursively. For example, it should not be possible to import src/modules/foo/components/Bar from src/modules/foo/components/Bar/components/Baz. The same principle should apply to Baz’ subcomponents; importing src/modules/foo/components/Bar/components/Baz should not be possible from src/modules/foo/components/Bar/components/Baz/components/Qux. However, we might consider allowing importing some TypeScript types from the parent component, that is, importing src/modules/foo/components/Bar/components/Baz/types from src/modules/foo/components/Bar/components/Baz/components/Qux could be allowed, but only from the parent - not from grandparent (importing src/modules/foo/components/Bar/types from src/modules/foo/components/Baz/components/Qux would not be okay).

With the current rules, we can add multiple instances of this rule to implement the “no imports from parent” at different nesting depths. For example, restricting some parent imports from the first level of subcomponents works with the following rule:

    {
      name: 'no-parent-imports-inside-module-components',
      comment: `Module components should be isolated from parent files so that
      theres a possibility to extract them out of the module in the future. This means
      that the components shouldn't import anything from their parent component.`,
      severity: 'error',
      from: {
        path: '/modules/([^/]+)/components/([^/]+)(.+components/).+$',
      },
      to: {
        path: '/modules/$1/components/$2/($2.tsx?|index.tsx?)',
        pathNot: '/modules/$1/components/$2$3',
      },
    },

But maybe there’s an easier way to implement such a recursive restriction without adding multiple rules for each nesting depth? Our max nesting depth is currently 5. 🙂

However, regexes are sometimes hard to decipher and test. With callbacks, rules could be composed of multiple functions and could be more readable. Also, we could write tests for functions before they are composed in dependency-cruiser’s rules. Granted, we have a regression test suite that imports dependency from dependency-cruiser to assert some valid and invalid imports. This approach works acceptably, but we need to patch dependency-cruiser with the following patch to expose the function.

diff --git a/src/main/index.js b/src/main/index.js
index afd68c2846f3815ebdeec80339e316eec9ad74d0..fc1c242ce3267199b379c7134368855ac72cb346 100644
--- a/src/main/index.js
+++ b/src/main/index.js
@@ -25,6 +25,7 @@ const {
 } = require("./options/normalize");
 const normalizeResolveOptions = require("./resolve-options/normalize");
 const reportWrap = require("./report-wrap");
+const { dependency } = require("../validate");
 
 function validateResultAgainstSchema(pResult) {
   const ajv = new Ajv();
@@ -135,4 +136,5 @@ module.exports = {
   format,
   allExtensions: meta.allExtensions,
   getAvailableTranspilers: meta.getAvailableTranspilers,
+  dependency
 };

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.