From 7784948fc5024476f0861b7b8bb20603f28cf809 Mon Sep 17 00:00:00 2001 From: stropho <3704482+stropho@users.noreply.github.com> Date: Mon, 6 Sep 2021 13:22:12 +0200 Subject: [PATCH] [New] `no-restricted-paths`: add/restore glob pattern support Fixes #2123. --- CHANGELOG.md | 2 + docs/rules/no-restricted-paths.md | 50 ++++++++++++++- package.json | 1 + src/rules/no-restricted-paths.js | 88 ++++++++++++++++++++------ tests/src/rules/no-restricted-paths.js | 72 ++++++++++++++++++++- 5 files changed, 192 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc7dca959..198551583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -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 diff --git a/docs/rules/no-restricted-paths.md b/docs/rules/no-restricted-paths.md index bfcb9af23..c9390754e 100644 --- a/docs/rules/no-restricted-paths.md +++ b/docs/rules/no-restricted-paths.md @@ -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 @@ -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' ``` diff --git a/package.json b/package.json index 7537df99a..b845b609d 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index 058aa43ea..e5bc6bc85 100644 --- a/src/rules/no-restricted-paths.js +++ b/src/rules/no-restricted-paths.js @@ -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'; @@ -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); }); @@ -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); @@ -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; } diff --git a/tests/src/rules/no-restricted-paths.js b/tests/src/rules/no-restricted-paths.js index 3ee728c5c..11934599e 100644 --- a/tests/src/rules/no-restricted-paths.js +++ b/tests/src/rules/no-restricted-paths.js @@ -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'), @@ -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")' }), @@ -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'), @@ -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, + } ], + }), ], });