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

no-restricted-paths support glob patterns #2219

Merged
merged 1 commit into from
Sep 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

### Added
- [`no-unused-modules`]: add eslint v8 support ([#2194], thanks [@coderaiser])
- [`no-restricted-paths`]: add/restore glob pattern support ([#2219], thanks [@stropho])

## [2.24.2] - 2021-08-24

Expand Down Expand Up @@ -903,6 +904,7 @@ for info on changes for earlier releases.

[`memo-parser`]: ./memo-parser/README.md

[#2219]: https://github.com/import-js/eslint-plugin-import/pull/2219
[#2196]: https://github.com/import-js/eslint-plugin-import/pull/2196
[#2194]: https://github.com/import-js/eslint-plugin-import/pull/2194
[#2184]: https://github.com/import-js/eslint-plugin-import/pull/2184
Expand Down
50 changes: 48 additions & 2 deletions docs/rules/no-restricted-paths.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,18 @@ In order to prevent such scenarios this rule allows you to define restricted zon

This rule has one option. The option is an object containing the definition of all restricted `zones` and the optional `basePath` which is used to resolve relative paths within.
The default value for `basePath` is the current working directory.
Each zone consists of the `target` path and a `from` path. The `target` is the path where the restricted imports should be applied. The `from` path defines the folder that is not allowed to be used in an import. An optional `except` may be defined for a zone, allowing exception paths that would otherwise violate the related `from`. Note that `except` is relative to `from` and cannot backtrack to a parent directory.
You may also specify an optional `message` for a zone, which will be displayed in case of the rule violation.

Each zone consists of the `target` path, a `from` path, and an optional `except` and `message` attribute.
- `target` is the path where the restricted imports should be applied. It can be expressed by
- directory string path that matches all its containing files
- glob pattern matching all the targeted files
- `from` path defines the folder that is not allowed to be used in an import. It can be expressed by
- directory string path that matches all its containing files
- glob pattern matching all the files restricted to be imported
- `except` may be defined for a zone, allowing exception paths that would otherwise violate the related `from`. Note that it does not alter the behaviour of `target` in any way.
- in case `from` is a glob pattern, `except` must be an array of glob patterns as well
- in case `from` is a directory path, `except` is relative to `from` and cannot backtrack to a parent directory.
- `message` - will be displayed in case of the rule violation.

### Examples

Expand Down Expand Up @@ -77,4 +87,40 @@ The following pattern is not considered a problem:

```js
import b from './b'

```

---------------

Given the following folder structure:

```
my-project
├── client
└── foo.js
└── sub-module
└── bar.js
└── baz.js

```

and the current configuration is set to:

```
{ "zones": [ {
"target": "./tests/files/restricted-paths/client/!(sub-module)/**/*",
"from": "./tests/files/restricted-paths/client/sub-module/**/*",
} ] }
```

The following import is considered a problem in `my-project/client/foo.js`:

```js
import a from './sub-module/baz'
```

The following import is not considered a problem in `my-project/client/sub-module/bar.js`:

```js
import b from './baz'
```
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
"find-up": "^2.0.0",
"has": "^1.0.3",
"is-core-module": "^2.6.0",
"is-glob": "^4.0.1",
"minimatch": "^3.0.4",
"object.values": "^1.1.4",
"pkg-up": "^2.0.0",
Expand Down
88 changes: 70 additions & 18 deletions src/rules/no-restricted-paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import path from 'path';

import resolve from 'eslint-module-utils/resolve';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import isGlob from 'is-glob';
import { Minimatch, default as minimatch } from 'minimatch';
import docsUrl from '../docsUrl';
import importType from '../core/importType';

Expand Down Expand Up @@ -56,6 +58,10 @@ module.exports = {
const matchingZones = restrictedPaths.filter((zone) => {
const targetPath = path.resolve(basePath, zone.target);

if (isGlob(targetPath)) {
return minimatch(currentFilename, targetPath);
}

return containsPath(currentFilename, targetPath);
});

Expand All @@ -72,18 +78,59 @@ module.exports = {
});
}

const zoneExceptions = matchingZones.map((zone) => {
const exceptionPaths = zone.except || [];
const absoluteFrom = path.resolve(basePath, zone.from);
const absoluteExceptionPaths = exceptionPaths.map((exceptionPath) => path.resolve(absoluteFrom, exceptionPath));
const hasValidExceptionPaths = absoluteExceptionPaths
.every((absoluteExceptionPath) => isValidExceptionPath(absoluteFrom, absoluteExceptionPath));
function reportInvalidExceptionGlob(node) {
context.report({
node,
message: 'Restricted path exceptions must be glob patterns when`from` is a glob pattern',
});
}

const makePathValidator = (zoneFrom, zoneExcept = []) => {
const absoluteFrom = path.resolve(basePath, zoneFrom);
const isGlobPattern = isGlob(zoneFrom);
let isPathRestricted;
let hasValidExceptions;
let isPathException;
let reportInvalidException;

if (isGlobPattern) {
const mm = new Minimatch(absoluteFrom);
isPathRestricted = (absoluteImportPath) => mm.match(absoluteImportPath);

hasValidExceptions = zoneExcept.every(isGlob);

if (hasValidExceptions) {
const exceptionsMm = zoneExcept.map((except) => new Minimatch(except));
isPathException = (absoluteImportPath) => exceptionsMm.some((mm) => mm.match(absoluteImportPath));
}

reportInvalidException = reportInvalidExceptionGlob;
} else {
isPathRestricted = (absoluteImportPath) => containsPath(absoluteImportPath, absoluteFrom);

const absoluteExceptionPaths = zoneExcept
.map((exceptionPath) => path.resolve(absoluteFrom, exceptionPath));
hasValidExceptions = absoluteExceptionPaths
.every((absoluteExceptionPath) => isValidExceptionPath(absoluteFrom, absoluteExceptionPath));

if (hasValidExceptions) {
isPathException = (absoluteImportPath) => absoluteExceptionPaths.some(
(absoluteExceptionPath) => containsPath(absoluteImportPath, absoluteExceptionPath),
);
}

reportInvalidException = reportInvalidExceptionPath;
}

return {
absoluteExceptionPaths,
hasValidExceptionPaths,
isPathRestricted,
hasValidExceptions,
isPathException,
reportInvalidException,
};
});
};

const validators = [];

function checkForRestrictedImportPath(importPath, node) {
const absoluteImportPath = resolve(importPath, context);
Expand All @@ -93,22 +140,27 @@ module.exports = {
}

matchingZones.forEach((zone, index) => {
const absoluteFrom = path.resolve(basePath, zone.from);

if (!containsPath(absoluteImportPath, absoluteFrom)) {
return;
if (!validators[index]) {
validators[index] = makePathValidator(zone.from, zone.except);
}

const { hasValidExceptionPaths, absoluteExceptionPaths } = zoneExceptions[index];
const {
isPathRestricted,
hasValidExceptions,
isPathException,
reportInvalidException,
} = validators[index];

if (!hasValidExceptionPaths) {
reportInvalidExceptionPath(node);
if (!isPathRestricted(absoluteImportPath)) {
return;
}

const pathIsExcepted = absoluteExceptionPaths
.some((absoluteExceptionPath) => containsPath(absoluteImportPath, absoluteExceptionPath));
if (!hasValidExceptions) {
reportInvalidException(node);
return;
}

const pathIsExcepted = isPathException(absoluteImportPath);
if (pathIsExcepted) {
return;
}
Expand Down
72 changes: 71 additions & 1 deletion tests/src/rules/no-restricted-paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,23 @@ ruleTester.run('no-restricted-paths', rule, {
zones: [ { target: './tests/files/restricted-paths/server', from: './tests/files/restricted-paths/other' } ],
} ],
}),
test({
code: 'import a from "../client/a.js"',
filename: testFilePath('./restricted-paths/server/b.js'),
options: [ {
zones: [ { target: '**/*', from: './tests/files/restricted-paths/other' } ],
} ],
}),
test({
code: 'import a from "../client/a.js"',
filename: testFilePath('./restricted-paths/client/b.js'),
options: [ {
zones: [ {
target: './tests/files/restricted-paths/!(client)/**/*',
from: './tests/files/restricted-paths/client/**/*',
} ],
} ],
}),
test({
code: 'const a = require("../client/a.js")',
filename: testFilePath('./restricted-paths/server/b.js'),
Expand Down Expand Up @@ -61,7 +78,17 @@ ruleTester.run('no-restricted-paths', rule, {
} ],
} ],
}),

test({
code: 'import A from "../two/a.js"',
filename: testFilePath('./restricted-paths/server/one/a.js'),
options: [ {
zones: [ {
target: '**/*',
from: './tests/files/restricted-paths/server/**/*',
except: ['**/a.js'],
} ],
} ],
}),

// irrelevant function calls
test({ code: 'notrequire("../server/b.js")' }),
Expand Down Expand Up @@ -93,6 +120,18 @@ ruleTester.run('no-restricted-paths', rule, {
column: 15,
} ],
}),
test({
code: 'import b from "../server/b.js"',
filename: testFilePath('./restricted-paths/client/a.js'),
options: [ {
zones: [ { target: './tests/files/restricted-paths/client/**/*', from: './tests/files/restricted-paths/server' } ],
} ],
errors: [ {
message: 'Unexpected path "../server/b.js" imported in restricted zone.',
line: 1,
column: 15,
} ],
}),
test({
code: 'import a from "../client/a"\nimport c from "./c"',
filename: testFilePath('./restricted-paths/server/b.js'),
Expand Down Expand Up @@ -190,5 +229,36 @@ ruleTester.run('no-restricted-paths', rule, {
column: 15,
} ],
}),
test({
code: 'import A from "../two/a.js"',
filename: testFilePath('./restricted-paths/server/one/a.js'),
options: [ {
zones: [ {
target: '**/*',
from: './tests/files/restricted-paths/server/**/*',
} ],
} ],
errors: [ {
message: 'Unexpected path "../two/a.js" imported in restricted zone.',
line: 1,
column: 15,
} ],
}),
test({
code: 'import A from "../two/a.js"',
filename: testFilePath('./restricted-paths/server/one/a.js'),
options: [ {
zones: [ {
target: '**/*',
from: './tests/files/restricted-paths/server/**/*',
except: ['a.js'],
} ],
} ],
errors: [ {
message: 'Restricted path exceptions must be glob patterns when`from` is a glob pattern',
line: 1,
column: 15,
} ],
}),
],
});