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

Feature request: Add a way to match a rule based on the relative path from "from" to "to" #330

Open
forty opened this issue Jul 6, 2020 · 6 comments
Labels

Comments

@forty
Copy link

forty commented Jul 6, 2020

I would like to be able to write rules such as

    {
        name: 'no-parent',
        severity: 'error',
        comment: 'cannot require from parent directory',
        from: {
            path: '^src/' 
        },
        to: {
           relativePath: '^../'
        }
    }

and

    {
        name: 'no-inside',
        severity: 'error',
        comment: 'cannot require for than 1 level deep inside a folder',
        from: {
            path: '^src/' 
        },
        to: {
           relativePathNot: '^(\.\.?/)([^/]+)$|^(\.\.?/)([^/]+)/index.ts$'
        }
    }

The rule would match if path.relative(path.dirname(from), to) matches the regex (or doesn't match the regex for the relativePathNot)

Context

I'm trying to do something similar to the "no-inside" rule above. Basically want to consider all folder as module boundaries. So for example:
A.ts can require ./B.ts (or ./B/index.ts), ../B.ts, ../../../B.ts but NOT ./b/B.ts or ../b/B.ts

I tried to do my best with regexp, but I'm not really sure it's possible to simulate that with regex alone :)

Expected Behavior

Be able to match based on relative paths

Current Behavior

Only absolute path can be user

Possible Solution

Add this new matcher property as suggested above

Considered alternatives

many complex regexp with all the possible depth of folder....

@forty forty changed the title Add a way to match a rule based on the relative path from "from" to "to" Feature request: Add a way to match a rule based on the relative path from "from" to "to" Jul 6, 2020
@jomi-se
Copy link

jomi-se commented Jul 9, 2020

Would be useful to have this. Maybe a way to ease the implementation would be to add an extra property such as relativeTo: true

@sverweij
Copy link
Owner

Hi @forty thanks for this suggestion. Could capture groups/ group matching fit your use cases? With this you have the from's path or any part of it you need to at your disposal. Below is an example

I'm reasonably confident the end result you had in mind is achievable with capture groups as well - but let me know if you need help with it!

So how does it work? You can but brackets around parts of the regular expression in the from part of the rule. The substring matches are available for later use in the to part of the rule with $n variables. E.g. this rule makes sure modules only require from the same folder or deeper:

{
  "allowed": [
    {
      "from": { 
        "path": "(.+/)[^/]+$" // will match e.g. src/cool/stuff/index.ts
      },
      "to": {
        // e.g. for the module src/cool/stuff/index.ts $1 contains src/cool/stuff/ . 
        // For that module the paths src/other/stuff or src/cool won't match.
        // Anything in and below src/cool/stuff will, though
        "path": "$1" 
      }
    }
  ]
}

Or, written as a forbidden rule, which might be more comfortable in many set ups:

{
  "forbidden": [
    {
      "name": "no-parents",
      "from": { "path": "(.+/)[^/]+$" },
      "to": {
        "pathNot": "$1"
      }
    }
  ]
}

If your modules also use external or core modules, you might want to expand the regular expression in the to part a bit, e.g. by appending node_modules and ^[^/]+$ respectively, so you'd get "pathNot": "$1|node_modules|^[^/]+$"

@forty
Copy link
Author

forty commented Jul 16, 2020

Hi @sverweij , thanks a lot for your answer.

What I need is a bit more complex than preventing parents from being imported. I what to allow importing siblings, and all ancestors siblings (but not their descendants)

So /a/b/c.js can import /x.js /a/x.js /a/b/x.js and /a/b/c/x.js (with x being any value) and that's it

@sverweij
Copy link
Owner

sverweij commented Jul 19, 2020

@forty you'd like to achieve something like this?

image

Let me see if I can construct some rules for this. If I infer correctly two things are not allowed:

  1. going deeper than one step into child folders ('no grand child folders')
  2. going into folders that are siblings of your parents ('not your aunts and her children')

1. no grand children

Lets first try and write an RE that catches the (generic case of) c.js:

  • (.+/)[^/]+$ => this will catch all files and place its folder in capture group $1

Everything like $1/xxx.js and $1/somefolder/yyy.js is okidoki, $1/deeper/nested/zzz.js isn't, though. In a regex that deeper nested stuff will look like $1/[^/]+/[^/]+/.+$. The forbidden rule will hence look like this:

{
  forbidden: [
   {
      "name": "no-grand-child-folders",
      "from": {
        "path": "(.+)/[^/]+$"
      },
      "to": {
        "path": "$1/[^/]+/[^/]+/.+$"
      }
    }
  ]
}

2. not your aunts and their children

Let's for easy reasoning say c.js resides in grand-mom/mom/c.js

We still need to be able to catch c.js, but we now also need a capture group that speaks of c.js's aunt's & uncles.

  • (.+)/([^/]+)/c.js => the first capture group ($1) will now contain c.js' grand-mom, the second one c.js' mom.

Now we want to select any aunt & their children, which is simply put /grand-parent/not-mom/yaddayadda. We can do this by saying

  • $1/[^/]+/.+$ => all children, grand-children, grand-grand-children, .... of grand-mom
  • that do not start with $1/$2/. In a rule:
{
    {
      "name": "not-your-aunts-and-children",
      "from": {
        "path": "(.+)/([^/]+)/[^/]+$"
      },
      "to": {
        "path": "$1/[^/]+/.+$", // it _is_ a child of your grand-mom ...
        "pathNot": "$1/$2/" // but it's not your mother
      }
    }
  ]
}

Edit note: I'm typing this directly in an edit box on the GH website and I'l have to hit comment in order to save,
before I've actually tested this against a real code base, otherwise my edits disappear in the void - I'll edit once again when I've tested this

Applying these rules to a part of dependency-cruiser's code base yields this (I've set the severities of not-your-aunts-and-children to info and the other one to error, so they show up more clearly and only show one part for conciseness):

image

The report for whole dependency-cruiser looks like what you see below.

image

If this indeed fits your use case (=> let me know!) I'm going to stick this as an example somewhere in the documentation.

@forty
Copy link
Author

forty commented Jul 21, 2020

Wow that is a great answer, thanks a lot. But this is not exactly what I'm trying to do, sorry it's not clear.

I'll try another way of explaining, using relative path (as that's what I'm asking): I want the relative path off all import to match ^(\.\.?/)+[^/]+$ (so you can import a path with any number of single or double points, but only one non dot part).

Example okay:

  • import * as a from './a' // (assuming a/index.js exists)
  • import * as a from './a.js'
  • import * as a from '../a.js'
  • import * as a from '../../../../../../a.js'

Example not okay:

  • import * as a from './b/a'
  • import * as a from './b/a.js'
  • import * as a from '../b/a.js'
  • import * as a from '../../../../../../b/a.js'
  • import * as a from '../../../../../../'

My idea being that if b/ (in the not okay examples) wants to expose a, then it should export it (via index.js) and external modules should use b.a instead. It allows all folder to be "modules", that protects/hides everything that is inside them from all files that are not inside them.

It kind of force to have one index.js in all folders.

In the case of dependency cruiser for example, it would force derive to have an index.js, exporting 3 const: circulal orphan and reachable.Then enrich-modules.js could only import ./derive and use derive.circurlar.
Same for the utl folder, which would have to export the function insides from index.js and then other files would only import ../utl and use utl.function. Apart from that, I think everything in the graph should be fine with my rule.

Does that make sense?

@forty
Copy link
Author

forty commented Jul 21, 2020

Here is a graph of my project, where I have been enforcing this rule "manually" (ie during code reviews ;))
graph

Note that logging/, assetEngine/ and server/ could really be logging.ts assetEngine.ts and server.ts, but I'm expecting them to grow so there are already in folders ready for the index.ts to be split.

Notice how dsp/handleRequest.ts can import utils/index.ts but should not be able to import utils/http.ts (indead it has to do import {http} from '../../utils')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants