From 345cfb008825bd03eee3806fd62d00df2ecb0436 Mon Sep 17 00:00:00 2001 From: Guillaume Clochard Date: Sat, 4 Jul 2020 21:11:41 +0200 Subject: [PATCH] [New] Add forbid options for no-internal-modules --- docs/rules/no-internal-modules.md | 11 ++++-- src/rules/no-internal-modules.js | 36 ++++++++++++++++---- tests/src/rules/no-internal-modules.js | 47 ++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/docs/rules/no-internal-modules.md b/docs/rules/no-internal-modules.md index 7bbb2edd16..7b00af15cc 100644 --- a/docs/rules/no-internal-modules.md +++ b/docs/rules/no-internal-modules.md @@ -4,7 +4,10 @@ Use this rule to prevent importing the submodules of other modules. ## Rule Details -This rule has one option, `allow` which is an array of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns that whitelist paths and import statements that can be imported with reaching. +This rule has two options that are arrays of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns: + +- `allow` that include paths and import statements that can be imported with reaching. +- `forbid` that exclude paths and import statements that can be imported with reaching. ### Examples @@ -13,6 +16,8 @@ Given the following folder structure: ``` my-project ├── actions +│ └── special +│ └── getAdmin.js │ └── getUser.js │ └── updateUser.js ├── reducer @@ -33,7 +38,8 @@ And the .eslintrc file: ... "rules": { "import/no-internal-modules": [ "error", { - "allow": [ "**/actions/*", "source-map-support/*" ] + "allow": [ "**/actions/*", "source-map-support/*" ], + "forbid": [ "**/actions/special/*" ], } ] } } @@ -49,6 +55,7 @@ The following patterns are considered problems: import { settings } from './app/index'; // Reaching to "./app/index" is not allowed import userReducer from './reducer/user'; // Reaching to "./reducer/user" is not allowed import configureStore from './redux/configureStore'; // Reaching to "./redux/configureStore" is not allowed +import getAdmin from './actions/special/getAdmin'; // Reaching to "./actions/special/getAdmin" is not allowed export { settings } from './app/index'; // Reaching to "./app/index" is not allowed export * from './reducer/user'; // Reaching to "./reducer/user" is not allowed diff --git a/src/rules/no-internal-modules.js b/src/rules/no-internal-modules.js index bd13ab07d0..a7b3afa60a 100644 --- a/src/rules/no-internal-modules.js +++ b/src/rules/no-internal-modules.js @@ -22,6 +22,12 @@ module.exports = { type: 'string', }, }, + forbid: { + type: 'array', + items: { + type: 'string', + }, + }, }, additionalProperties: false, }, @@ -31,12 +37,18 @@ module.exports = { create: function noReachingInside(context) { const options = context.options[0] || {} const allowRegexps = (options.allow || []).map(p => minimatch.makeRe(p)) + const forbidRegexps = (options.forbid || []).map(p => minimatch.makeRe(p)) // test if reaching to this destination is allowed function reachingAllowed(importPath) { return allowRegexps.some(re => re.test(importPath)) } + // test if reaching to this destination is forbidden + function reachingForbidden(importPath) { + return forbidRegexps.some(re => re.test(importPath)) + } + // minimatch patterns are expected to use / path separators, like import // statements, so normalize paths to use the same function normalizeSep(somePath) { @@ -58,21 +70,31 @@ module.exports = { }, []) const nonScopeSteps = steps.filter(step => step.indexOf('@') !== 0) + if (nonScopeSteps.length <= 1) return false // before trying to resolve, see if the raw import (with relative - // segments resolved) matches an allowed pattern + // segments resolved) matches an forbidden pattern const justSteps = steps.join('/') - if (reachingAllowed(justSteps) || reachingAllowed(`/${justSteps}`)) return false + + if (reachingForbidden(justSteps) || reachingForbidden(`/${justSteps}`)) return true + + const resolved = resolve(importPath, context) // if the import statement doesn't match directly, try to match the // resolved path if the import is resolvable - const resolved = resolve(importPath, context) - if (!resolved || reachingAllowed(normalizeSep(resolved))) return false + if (resolved && reachingForbidden(normalizeSep(resolved))) return true + + // path is not explicitly forbidden + + // if path not explicitly allowed, consider it forbidden + if ( + !reachingAllowed(justSteps) + && !reachingAllowed(`/${justSteps}`) + && (resolved && !reachingAllowed(normalizeSep(resolved))) + ) return true - // this import was not allowed by the allowed paths, and reaches - // so it is a violation - return true + return false } function checkImportForReaching(importPath, node) { diff --git a/tests/src/rules/no-internal-modules.js b/tests/src/rules/no-internal-modules.js index da9a4ca1a0..454d80d27c 100644 --- a/tests/src/rules/no-internal-modules.js +++ b/tests/src/rules/no-internal-modules.js @@ -14,6 +14,13 @@ ruleTester.run('no-internal-modules', rule, { filename: testFilePath('./internal-modules/plugins/plugin.js'), options: [], }), + test({ + code: 'import a from "./plugin2"', + filename: testFilePath('./internal-modules/plugins/plugin.js'), + options: [ { + forbid: [ '**/something/*' ], + } ], + }), test({ code: 'const a = require("./plugin2")', filename: testFilePath('./internal-modules/plugins/plugin.js'), @@ -184,6 +191,46 @@ ruleTester.run('no-internal-modules', rule, { }, ], }), + test({ + code: 'import a from "./plugin2/index.js"', + filename: testFilePath('./internal-modules/plugins/plugin.js'), + options: [ { + forbid: [ '**/plugin2/*' ], + } ], + errors: [ { + message: 'Reaching to "./plugin2/index.js" is not allowed.', + line: 1, + column: 15, + } ], + }), + test({ + code: 'export * from "jquery/dist/jquery"', + filename: testFilePath('./internal-modules/plugins/plugin2/internal.js'), + options: [ { + allow: [ 'jquery/dist/*' ], + forbid: [ 'jquery/dist/jquery' ], + } ], + errors: [ { + message: 'Reaching to "jquery/dist/jquery" is not allowed.', + line: 1, + column: 15, + } ], + }), + test({ + code: 'import a from "../api/service/index"', + filename: testFilePath('./internal-modules/plugins/plugin.js'), + options: [ { + allow: [ '**' ], + forbid: [ '**/api/**' ], + } ], + errors: [ + { + message: 'Reaching to "../api/service/index" is not allowed.', + line: 1, + column: 15, + }, + ], + }), // exports test({ code: 'export * from "./plugin2/index.js";\nexport * from "./plugin2/app/index"',