Skip to content

Commit

Permalink
Merge branch 'main' into patch-1
Browse files Browse the repository at this point in the history
  • Loading branch information
jablko committed Sep 7, 2021
2 parents 2f20877 + 7784948 commit dfe9346
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 27 deletions.
11 changes: 10 additions & 1 deletion .eslintrc
Expand Up @@ -25,6 +25,15 @@
"eqeqeq": [2, "allow-null"],
"func-call-spacing": 2,
"indent": [2, 2],
"keyword-spacing": ["error", {
before: true,
after: true,
overrides: {
return: { after: true },
throw: { after: true },
case: { after: true }
}
}],
"max-len": [1, 99, 2],
"no-cond-assign": [2, "always"],
"no-return-assign": [2, "always"],
Expand All @@ -47,7 +56,7 @@
"named": "never",
"asyncArrow": "always",
}],

"eslint-plugin/consistent-output": [
"error",
"always",
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
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
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
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
2 changes: 1 addition & 1 deletion resolvers/webpack/index.js
Expand Up @@ -84,7 +84,7 @@ exports.resolve = function (source, file, settings) {
if (configPath) {
try {
webpackConfig = require(configPath);
} catch(e) {
} catch (e) {
console.log('Error resolving webpackConfig', e);
throw e;
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/importType.js
Expand Up @@ -75,7 +75,7 @@ export function isScopedMain(name) {
}

function isRelativeToParent(name) {
return/^\.\.$|^\.\.[\\/]/.test(name);
return /^\.\.$|^\.\.[\\/]/.test(name);
}

const indexFiles = ['.', './', './index', './index.js'];
Expand Down
88 changes: 70 additions & 18 deletions src/rules/no-restricted-paths.js
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
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,
} ],
}),
],
});
4 changes: 2 additions & 2 deletions utils/module-require.js
Expand Up @@ -18,12 +18,12 @@ exports.default = function moduleRequire(p) {
const eslintPath = require.resolve('eslint');
const eslintModule = createModule(eslintPath);
return require(Module._resolveFilename(p, eslintModule));
} catch(err) { /* ignore */ }
} catch (err) { /* ignore */ }

try {
// try relative to entry point
return require.main.require(p);
} catch(err) { /* ignore */ }
} catch (err) { /* ignore */ }

// finally, try from here
return require(p);
Expand Down
2 changes: 1 addition & 1 deletion utils/resolve.js
Expand Up @@ -42,7 +42,7 @@ function tryRequire(target, sourceFile) {
} else {
resolved = require.resolve(target);
}
} catch(e) {
} catch (e) {
// If the target does not exist then just return undefined
return undefined;
}
Expand Down

0 comments on commit dfe9346

Please sign in to comment.