From 477fb820cf47222300ab710f7b12293730a4fc51 Mon Sep 17 00:00:00 2001 From: Ross Solomon Date: Mon, 19 Nov 2018 17:33:51 -0800 Subject: [PATCH 1/6] [no-restricted-paths] Allow exceptions to zones --- CHANGELOG.md | 4 ++ docs/rules/no-restricted-paths.md | 42 +++++++++++++++++++- src/rules/no-restricted-paths.js | 13 ++++++ tests/files/restricted-paths/server/one/a.js | 0 tests/files/restricted-paths/server/one/b.js | 0 tests/files/restricted-paths/server/two/a.js | 0 tests/src/rules/no-restricted-paths.js | 38 ++++++++++++++++++ 7 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 tests/files/restricted-paths/server/one/a.js create mode 100644 tests/files/restricted-paths/server/one/b.js create mode 100644 tests/files/restricted-paths/server/two/a.js diff --git a/CHANGELOG.md b/CHANGELOG.md index a0b8a590d9..88ccd88332 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## [Unreleased] ### Fixed - [`no-extraneous-dependencies`]: `packageDir` option with array value was clobbering package deps instead of merging them ([#1175]/[#1176], thanks [@aravindet] & [@pzhine]) +- +### Added +- [`no-restricted-paths`]: New `except` option per `zone`, allowing exceptions to be defined for a restricted zone. ## [2.14.0] - 2018-08-13 @@ -14,6 +17,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel | * e30a757 (source/pr/1151, fork/jsx) Add JSX check to namespace rule |/ * 8252344 (source/pr/1148) Add error to output when module loaded as resolver has invalid API + ### Added - [`no-useless-path-segments`]: add commonJS (CJS) support ([#1128], thanks [@1pete]) - [`namespace`]: add JSX check ([#1151], thanks [@jf248]) diff --git a/docs/rules/no-restricted-paths.md b/docs/rules/no-restricted-paths.md index bad65ab8e1..f72579bc53 100644 --- a/docs/rules/no-restricted-paths.md +++ b/docs/rules/no-restricted-paths.md @@ -9,7 +9,7 @@ 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. +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 violiate the related `from`. ### Examples @@ -37,3 +37,43 @@ The following patterns are not considered problems when configuration set to `{ ```js import baz from '../client/baz'; ``` + +--------------- + +Given the following folder structure: + +``` +my-project +├── client +│ └── foo.js +│ └── baz.js +└── server + ├── one + │ └── a.js + │ └── b.js + └── two +``` + +and the current file being linted is `my-project/server/one/a.js`. + +and the current configuration is set to: + +``` +{ "zones": [ { + "target": "./tests/files/restricted-paths/server/one", + "from": "./tests/files/restricted-paths/server", + "except": ["./tests/files/restricted-paths/server/one"] +} ] } +``` + +The following pattern is considered a problem: + +```js +import a from "../two/a.js +``` + +The following pattern is not considered a problem: + +```js +import b from "./b.js +``` diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index 5b20c40d84..ec68d9595d 100644 --- a/src/rules/no-restricted-paths.js +++ b/src/rules/no-restricted-paths.js @@ -23,6 +23,12 @@ module.exports = { properties: { target: { type: 'string' }, from: { type: 'string' }, + except: { + type: 'array', + items: { + type: 'string', + }, + }, }, additionalProperties: false, }, @@ -53,9 +59,16 @@ module.exports = { } matchingZones.forEach((zone) => { + const exceptionPaths = zone.except || [] const absoluteFrom = path.resolve(basePath, zone.from) if (containsPath(absoluteImportPath, absoluteFrom)) { + if (exceptionPaths.some((exceptionPath) => ( + containsPath(absoluteImportPath, path.resolve(basePath, exceptionPath)) + ))) { + return + } + context.report({ node, message: `Unexpected path "${importPath}" imported in restricted zone.`, diff --git a/tests/files/restricted-paths/server/one/a.js b/tests/files/restricted-paths/server/one/a.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/files/restricted-paths/server/one/b.js b/tests/files/restricted-paths/server/one/b.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/files/restricted-paths/server/two/a.js b/tests/files/restricted-paths/server/two/a.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/src/rules/no-restricted-paths.js b/tests/src/rules/no-restricted-paths.js index 13f8472cb1..bede7fd0be 100644 --- a/tests/src/rules/no-restricted-paths.js +++ b/tests/src/rules/no-restricted-paths.js @@ -28,6 +28,28 @@ ruleTester.run('no-restricted-paths', rule, { zones: [ { target: './tests/files/restricted-paths/client', from: './tests/files/restricted-paths/other' } ], } ], }), + test({ + code: 'import a from "./a.js"', + filename: testFilePath('./restricted-paths/server/one/a.js'), + options: [ { + zones: [ { + target: './tests/files/restricted-paths/server/one', + from: './tests/files/restricted-paths/server', + except: ['./tests/files/restricted-paths/server/one'], + } ], + } ], + }), + test({ + code: 'import a from "../two/a.js"', + filename: testFilePath('./restricted-paths/server/one/a.js'), + options: [ { + zones: [ { + target: './tests/files/restricted-paths/server/one', + from: './tests/files/restricted-paths/server', + except: ['./tests/files/restricted-paths/server/two'], + } ], + } ], + }), // irrelevant function calls @@ -107,5 +129,21 @@ ruleTester.run('no-restricted-paths', rule, { column: 19, } ], }), + test({ + code: 'import b from "../two/a.js"', + filename: testFilePath('./restricted-paths/server/one/a.js'), + options: [ { + zones: [ { + target: './tests/files/restricted-paths/server/one', + from: './tests/files/restricted-paths/server', + except: ['./tests/files/restricted-paths/server/one'], + } ], + } ], + errors: [ { + message: 'Unexpected path "../two/a.js" imported in restricted zone.', + line: 1, + column: 15, + } ], + }), ], }) From 832ba04ac065ed5548da6fb13c97adeeec03a5c8 Mon Sep 17 00:00:00 2001 From: Ross Solomon Date: Mon, 19 Nov 2018 17:56:34 -0800 Subject: [PATCH 2/6] Fix doc formatting --- docs/rules/no-restricted-paths.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-restricted-paths.md b/docs/rules/no-restricted-paths.md index f72579bc53..e6188384a2 100644 --- a/docs/rules/no-restricted-paths.md +++ b/docs/rules/no-restricted-paths.md @@ -69,11 +69,11 @@ and the current configuration is set to: The following pattern is considered a problem: ```js -import a from "../two/a.js +import a from '../two/a' ``` The following pattern is not considered a problem: ```js -import b from "./b.js +import b from './b' ``` From 81a4543d818f142898142612e7a17f4294effd1e Mon Sep 17 00:00:00 2001 From: Ross Solomon Date: Sat, 24 Nov 2018 19:00:25 -0800 Subject: [PATCH 3/6] Add uniqueItems schema restriction; fix doc typo --- docs/rules/no-restricted-paths.md | 2 +- src/rules/no-restricted-paths.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-restricted-paths.md b/docs/rules/no-restricted-paths.md index e6188384a2..cfd5738dc5 100644 --- a/docs/rules/no-restricted-paths.md +++ b/docs/rules/no-restricted-paths.md @@ -9,7 +9,7 @@ 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 violiate the related `from`. +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`. ### Examples diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index ec68d9595d..cf3046c507 100644 --- a/src/rules/no-restricted-paths.js +++ b/src/rules/no-restricted-paths.js @@ -28,6 +28,7 @@ module.exports = { items: { type: 'string', }, + uniqueItems: true, }, }, additionalProperties: false, From e71d7074659f38401c7b3247a20a8e6880212a07 Mon Sep 17 00:00:00 2001 From: Ross Solomon Date: Mon, 26 Nov 2018 15:18:28 -0800 Subject: [PATCH 4/6] Make exceptions relative to from paths, plus enforcement --- docs/rules/no-restricted-paths.md | 4 ++-- src/rules/no-restricted-paths.js | 23 ++++++++++++++++++++--- tests/src/rules/no-restricted-paths.js | 23 ++++++++++++++++++++--- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/docs/rules/no-restricted-paths.md b/docs/rules/no-restricted-paths.md index cfd5738dc5..3776699836 100644 --- a/docs/rules/no-restricted-paths.md +++ b/docs/rules/no-restricted-paths.md @@ -9,7 +9,7 @@ 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`. +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. ### Examples @@ -62,7 +62,7 @@ and the current configuration is set to: { "zones": [ { "target": "./tests/files/restricted-paths/server/one", "from": "./tests/files/restricted-paths/server", - "except": ["./tests/files/restricted-paths/server/one"] + "except": ["./one"] } ] } ``` diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index cf3046c507..3d133f870f 100644 --- a/src/rules/no-restricted-paths.js +++ b/src/rules/no-restricted-paths.js @@ -4,6 +4,7 @@ import path from 'path' import resolve from 'eslint-module-utils/resolve' import isStaticRequire from '../core/staticRequire' import docsUrl from '../docsUrl' +import importType from '../core/importType' module.exports = { meta: { @@ -64,9 +65,25 @@ module.exports = { const absoluteFrom = path.resolve(basePath, zone.from) if (containsPath(absoluteImportPath, absoluteFrom)) { - if (exceptionPaths.some((exceptionPath) => ( - containsPath(absoluteImportPath, path.resolve(basePath, exceptionPath)) - ))) { + if (exceptionPaths.some((exceptionPath) => { + const absoluteExceptionPath = path.resolve(absoluteFrom, exceptionPath) + const relativeExceptionPath = path.relative(absoluteFrom, absoluteExceptionPath) + + if (importType(relativeExceptionPath, context) === 'parent') { + context.report({ + node, + message: 'Restricted path exceptions must be relative to the configured ' + + '`from` path for that zone.', + }) + + return true + } + + + return ( + containsPath(absoluteImportPath, absoluteExceptionPath) + ) + })) { return } diff --git a/tests/src/rules/no-restricted-paths.js b/tests/src/rules/no-restricted-paths.js index bede7fd0be..d360cc77cb 100644 --- a/tests/src/rules/no-restricted-paths.js +++ b/tests/src/rules/no-restricted-paths.js @@ -35,7 +35,7 @@ ruleTester.run('no-restricted-paths', rule, { zones: [ { target: './tests/files/restricted-paths/server/one', from: './tests/files/restricted-paths/server', - except: ['./tests/files/restricted-paths/server/one'], + except: ['./one'], } ], } ], }), @@ -46,7 +46,7 @@ ruleTester.run('no-restricted-paths', rule, { zones: [ { target: './tests/files/restricted-paths/server/one', from: './tests/files/restricted-paths/server', - except: ['./tests/files/restricted-paths/server/two'], + except: ['./two'], } ], } ], }), @@ -136,7 +136,7 @@ ruleTester.run('no-restricted-paths', rule, { zones: [ { target: './tests/files/restricted-paths/server/one', from: './tests/files/restricted-paths/server', - except: ['./tests/files/restricted-paths/server/one'], + except: ['./one'], } ], } ], errors: [ { @@ -145,5 +145,22 @@ ruleTester.run('no-restricted-paths', rule, { column: 15, } ], }), + test({ + code: 'import b from "../two/a.js"', + filename: testFilePath('./restricted-paths/server/one/a.js'), + options: [ { + zones: [ { + target: './tests/files/restricted-paths/server/one', + from: './tests/files/restricted-paths/server', + except: ['../client/a'], + } ], + } ], + errors: [ { + message: 'Restricted path exceptions must be relative to the configured ' + + '`from` path for that zone.', + line: 1, + column: 15, + } ], + }), ], }) From 7ea3ceecb29d3666e85dd155084bc01f16e70431 Mon Sep 17 00:00:00 2001 From: Ross Solomon Date: Mon, 26 Nov 2018 16:41:17 -0800 Subject: [PATCH 5/6] Remove deep nesting of conditionals --- src/rules/no-restricted-paths.js | 68 ++++++++++++++++---------- tests/src/rules/no-restricted-paths.js | 2 +- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index 3d133f870f..9df27cc0c6 100644 --- a/src/rules/no-restricted-paths.js +++ b/src/rules/no-restricted-paths.js @@ -53,6 +53,20 @@ module.exports = { return containsPath(currentFilename, targetPath) }) + function isValidExceptionPath(absoluteFromPath, absoluteExceptionPath) { + const relativeExceptionPath = path.relative(absoluteFromPath, absoluteExceptionPath) + + return importType(relativeExceptionPath, context) !== 'parent' + } + + function reportInvalidExceptionPath(node) { + context.report({ + node, + message: 'Restricted path exceptions must be descendants of the configured ' + + '`from` path for that zone.', + }) + } + function checkForRestrictedImportPath(importPath, node) { const absoluteImportPath = resolve(importPath, context) @@ -64,34 +78,34 @@ module.exports = { const exceptionPaths = zone.except || [] const absoluteFrom = path.resolve(basePath, zone.from) - if (containsPath(absoluteImportPath, absoluteFrom)) { - if (exceptionPaths.some((exceptionPath) => { - const absoluteExceptionPath = path.resolve(absoluteFrom, exceptionPath) - const relativeExceptionPath = path.relative(absoluteFrom, absoluteExceptionPath) - - if (importType(relativeExceptionPath, context) === 'parent') { - context.report({ - node, - message: 'Restricted path exceptions must be relative to the configured ' + - '`from` path for that zone.', - }) - - return true - } - - - return ( - containsPath(absoluteImportPath, absoluteExceptionPath) - ) - })) { - return - } - - context.report({ - node, - message: `Unexpected path "${importPath}" imported in restricted zone.`, - }) + if (!containsPath(absoluteImportPath, absoluteFrom)) { + return + } + + const absoluteExceptionPaths = exceptionPaths.map((exceptionPath) => + path.resolve(absoluteFrom, exceptionPath) + ) + const hasValidExceptionPaths = absoluteExceptionPaths.some((absoluteExceptionPath) => + isValidExceptionPath(absoluteFrom, absoluteExceptionPath) + ) + + if (!hasValidExceptionPaths) { + reportInvalidExceptionPath(node) + return + } + + const pathIsExcepted = absoluteExceptionPaths.some((absoluteExceptionPath) => + containsPath(absoluteImportPath, absoluteExceptionPath) + ) + + if (pathIsExcepted) { + return } + + context.report({ + node, + message: `Unexpected path "${importPath}" imported in restricted zone.`, + }) }) } diff --git a/tests/src/rules/no-restricted-paths.js b/tests/src/rules/no-restricted-paths.js index d360cc77cb..12dc049896 100644 --- a/tests/src/rules/no-restricted-paths.js +++ b/tests/src/rules/no-restricted-paths.js @@ -156,7 +156,7 @@ ruleTester.run('no-restricted-paths', rule, { } ], } ], errors: [ { - message: 'Restricted path exceptions must be relative to the configured ' + + message: 'Restricted path exceptions must be descendants of the configured ' + '`from` path for that zone.', line: 1, column: 15, From fb6336595488407358438b5b0f1f0c9a608e8fca Mon Sep 17 00:00:00 2001 From: Ross Solomon Date: Mon, 26 Nov 2018 16:56:37 -0800 Subject: [PATCH 6/6] some => every with negation --- src/rules/no-restricted-paths.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index 9df27cc0c6..d8b6124e00 100644 --- a/src/rules/no-restricted-paths.js +++ b/src/rules/no-restricted-paths.js @@ -85,7 +85,7 @@ module.exports = { const absoluteExceptionPaths = exceptionPaths.map((exceptionPath) => path.resolve(absoluteFrom, exceptionPath) ) - const hasValidExceptionPaths = absoluteExceptionPaths.some((absoluteExceptionPath) => + const hasValidExceptionPaths = absoluteExceptionPaths.every((absoluteExceptionPath) => isValidExceptionPath(absoluteFrom, absoluteExceptionPath) )