[New] no-restricted-paths: support arrays for from and target options

[AdriAt360]

Fixes #2142

Context

Sometimes I want to set certain paths in batch to prohibit relying on the same path, or a certain path to prohibit relying on multiple paths.

Changes

In this PR

• I update the schema of this rule
  • before: from and target are only a string
  • after: these fields can be either a string or an array of string
• I change the behavior + add tests
  • all the elements in from and target are taken into account the same way it was before
  • I also ensure the homogeneity of the from elements to stay aligned with the current homogeneity checks between from and except
• I update the related doc and add new examples
• I update the changelog

Notes

• I advise to read this PR commit by commit as there are quite a few changes that are easier to review step by step, e.g. extracting functions before using them inside loops
• I chose to support both string and array to avoid a breaking change

[ljharb]

I'll make these changes in a rebase. Thanks!

[AdriAt360]

@ljharb my pleasure!
thanks for your review and suggestions 🙏🏽
how do you want to proceed for the next steps: as I have no write access, I guess you will merge this pull request yourself and close the related issue when done?

[ljharb]

Yes, I'll merge it myself, and the fixes #2142 i edited into your OP will cause github to auto-close it upon merging.

[AdriAt360]

@ljharb indeed, I missed your edit, all perfect then =)

[ljharb]

@AdriAt360 hm, looks like this PR is causing the native windows tests to fail: https://ci.appveyor.com/project/ljharb/eslint-plugin-import/build/job/xqkwnmajs2svu218

[AdriAt360]

@ljharb as there are multiple tests with this name and I can't reproduce locally, I just added some comments to identify the failing test d3fa030
I hope it will help investigating and will keep you posted

[ljharb]

looks like number 7: https://ci.appveyor.com/project/ljharb/eslint-plugin-import/builds/43753304/job/dshd09a4o6tucs0a

[AdriAt360]

@ljharb still no way to reproduce on my side, so I added another test to help me investigate

[AdriAt360]

@ljharb ok, seems that this specific glob pattern (./some/path/*) is not accepted by windows
I updated and added some tests to check some other common patterns

[AdriAt360]

@ljharb I guess I found the issue, but as it impacts all the tests, you might want to give an opinion
as stated in https://stackoverflow.com/a/39111164/2814072, path.resolve always resolves to an absolute path where path.join might or not
problem is:

• in test/utils, the testFilePath function used path.join, but in the source code of the no-restricted-paths rule, only path.resolve is used
• in case of glob patterns, minimatch will match the file only if the file path is absolute, as the glob pattern is
  
• I guess on Windows, path.join resolved to a relative path, explaining the failing tests

If this hypothesis is correct, the pipeline should be all green now, so the next step would be to clean the tests I added to investigate here, unless you prefer keeping them

[AdriAt360]

@ljharb ok, wrong hypothesis
another hint could be process.cwd()

More details here

• from https://www.geeksforgeeks.org/node-js-process-cwd-method/, I can see that the current working directory resolves with back-slashes
  
• from https://www.npmjs.com/package/minimatch, I can see that the glob pattern should contain only forward-slashes
  
• but in our situation
  • target is defined correctly (only /)
  • if no basePath is provided, process.cwd() is used as basePath to build targetPath
  • meaning on Windows native, the target path pattern could resolve with \ in some conditions (I guess, don't know why it could not be everytime)
    I opened this other PR to validate this hypothesis (expected output: the new test I added there is red too), I'll close it

As it seems be working with the from and except parts, I change to use Minimatch.match as it's done there, it should fix this for real: 4392f46

(by the way I reverted the previous commit too)

if this is really the root cause, then I guess there should be a dedicated [Fix] commit for this problem, as you already did for the error message, so I can clean and force-push here

[AdriAt360]

hi there @ljharb 👋🏽
as my previous attempt wasn't successful either, here is another try to fix the existing issue on the Windows Native pipeline

• I decided to use the resolve function the same way it's done for the imports de33169
• I also fixed the tests accordingly 2f9ba90

I hope this will make the pipeline green 🤞🏽

edit: seems that it doesn't work either ><

[AdriAt360]

@ljharb how do you want to proceed next here?

• as it was already broken (confirmed by [Tests] no-restricted-paths: skip failing tests on windows #2472), I think it would not be more harmful to merge this current version (without my last 2 commits obviously)
• in the meantime, I'm not fond of ignoring a broken pipeline
• but I must admit I run out of new ideas for this windows native pipeline problem

[ljharb]

hmm. let's reopen #2472, and update it so that it skips the broken tests in windows. then we can land that, rebase this, and land it.

[AdriAt360]

@ljharb I just commented the failing test and removed my previous investigation commits -> this PR seems ready to be merged
the AppVeyor build failed but it seems like a timeout on a random test outside this PR's scope: could you rerun this check?
By the way, feel free to rebase if you feel the 2 commits 786b9b3 and 1196129 should be merged into only one

[AdriAt360]

@ljharb it seems that this PR is ready to be merged, but feel free to tell if there is still something waiting on what I could be helpful :)