Skip to content

Commit

Permalink
Update: no-restricted-imports custom message for patterns (fixes #11843…
Browse files Browse the repository at this point in the history 
…) (#14580)

* Update: no-restricted-imports rule with custom messages (fixes #11843)

* Update docs/rules/no-restricted-imports.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/rules/no-restricted-imports.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Code review feedback - better test cases and schema change

* Doc updates

* Added correct/incorrect examples to docs

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
@lexholden @mdjermanovic
lexholden and mdjermanovic committed May 21, 2021
1 parent 967b1c4 commit 52655dd
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 25 deletions.
34 changes: 33 additions & 1 deletion docs/rules/no-restricted-imports.md
View file
Expand Up @@ -75,7 +75,21 @@ or like this if you need to restrict only certain imports from a module:
}]
```

The custom message will be appended to the default error message. Please note that you may not specify custom error messages for restricted patterns as a particular import may match more than one pattern.
or like this if you want to apply a custom message to pattern matches:

```json
"no-restricted-imports": ["error", {
"patterns": [{
"group": ["import1/private/*"],
"message": "usage of import1 private modules not allowed."
}, {
"group": ["import2/*", "!import2/good"],
"message": "import2 is deprecated, except the modules in import2/good."
}]
}]
```

The custom message will be appended to the default error message.

To restrict the use of all Node.js core imports (via https://github.com/nodejs/node/tree/master/lib):

Expand Down Expand Up @@ -149,6 +163,15 @@ import { DisallowedObject as AllowedObject } from "foo";
import * as Foo from "foo";
```

```js
/*eslint no-restricted-imports: ["error", { patterns: [{
group: ["lodash/*"],
message: "Please use the default import from 'lodash' instead."
}]}]*/

import pick from 'lodash/pick';
```

Examples of **correct** code for this rule:

```js
Expand Down Expand Up @@ -182,6 +205,15 @@ import DisallowedObject from "foo"
import { AllowedObject as DisallowedObject } from "foo";
```

```js
/*eslint no-restricted-imports: ["error", { patterns: [{
group: ["lodash/*"],
message: "Please use the default import from 'lodash' instead."
}]}]*/

import lodash from 'lodash';
```

## When Not To Use It

Don't use this rule or don't include a module in the list for this rule if you want to be able to import a module in your project without an ESLint error or warning.
85 changes: 61 additions & 24 deletions lib/rules/no-restricted-imports.js
View file
Expand Up @@ -10,12 +10,6 @@

const ignore = require("ignore");

const arrayOfStrings = {
type: "array",
items: { type: "string" },
uniqueItems: true
};

const arrayOfStringsOrObjects = {
type: "array",
items: {
Expand Down Expand Up @@ -44,6 +38,41 @@ const arrayOfStringsOrObjects = {
uniqueItems: true
};

const arrayOfStringsOrObjectPatterns = {
anyOf: [
{
type: "array",
items: {
type: "string"
},
uniqueItems: true
},
{
type: "array",
items: {
type: "object",
properties: {
group: {
type: "array",
items: {
type: "string"
},
minItems: 1,
uniqueItems: true
},
message: {
type: "string",
minLength: 1
}
},
additionalProperties: false,
required: ["group"]
},
uniqueItems: true
}
]
};

module.exports = {
meta: {
type: "suggestion",
Expand All @@ -61,6 +90,8 @@ module.exports = {
pathWithCustomMessage: "'{{importSource}}' import is restricted from being used. {{customMessage}}",

patterns: "'{{importSource}}' import is restricted from being used by a pattern.",
// eslint-disable-next-line eslint-plugin/report-message-format
patternWithCustomMessage: "'{{importSource}}' import is restricted from being used by a pattern. {{customMessage}}",

everything: "* import is invalid because '{{importNames}}' from '{{importSource}}' is restricted.",
// eslint-disable-next-line eslint-plugin/report-message-format
Expand All @@ -80,7 +111,7 @@ module.exports = {
type: "object",
properties: {
paths: arrayOfStringsOrObjects,
patterns: arrayOfStrings
patterns: arrayOfStringsOrObjectPatterns
},
additionalProperties: false
}],
Expand All @@ -98,13 +129,6 @@ module.exports = {
(Object.prototype.hasOwnProperty.call(options[0], "paths") || Object.prototype.hasOwnProperty.call(options[0], "patterns"));

const restrictedPaths = (isPathAndPatternsObject ? options[0].paths : context.options) || [];
const restrictedPatterns = (isPathAndPatternsObject ? options[0].patterns : []) || [];

// if no imports are restricted we don"t need to check
if (Object.keys(restrictedPaths).length === 0 && restrictedPatterns.length === 0) {
return {};
}

const restrictedPathMessages = restrictedPaths.reduce((memo, importSource) => {
if (typeof importSource === "string") {
memo[importSource] = { message: null };
Expand All @@ -117,7 +141,16 @@ module.exports = {
return memo;
}, {});

const restrictedPatternsMatcher = ignore().add(restrictedPatterns);
// Handle patterns too, either as strings or groups
const restrictedPatterns = (isPathAndPatternsObject ? options[0].patterns : []) || [];
const restrictedPatternGroups = restrictedPatterns.length > 0 && typeof restrictedPatterns[0] === "string"
? [{ matcher: ignore().add(restrictedPatterns) }]
: restrictedPatterns.map(({ group, message }) => ({ matcher: ignore().add(group), customMessage: message }));

// if no imports are restricted we don"t need to check
if (Object.keys(restrictedPaths).length === 0 && restrictedPatternGroups.length === 0) {
return {};
}

/**
* Report a restricted path.
Expand Down Expand Up @@ -184,29 +217,32 @@ module.exports = {
/**
* Report a restricted path specifically for patterns.
* @param {node} node representing the restricted path reference
* @param {Object} group contains a Ignore instance for paths, and the customMessage to show if it fails
* @returns {void}
* @private
*/
function reportPathForPatterns(node) {
function reportPathForPatterns(node, group) {
const importSource = node.source.value.trim();

context.report({
node,
messageId: "patterns",
messageId: group.customMessage ? "patternWithCustomMessage" : "patterns",
data: {
importSource
importSource,
customMessage: group.customMessage
}
});
}

/**
* Check if the given importSource is restricted by a pattern.
* @param {string} importSource path of the import
* @param {Object} group contains a Ignore instance for paths, and the customMessage to show if it fails
* @returns {boolean} whether the variable is a restricted pattern or not
* @private
*/
function isRestrictedPattern(importSource) {
return restrictedPatterns.length > 0 && restrictedPatternsMatcher.ignores(importSource);
function isRestrictedPattern(importSource, group) {
return group.matcher.ignores(importSource);
}

/**
Expand Down Expand Up @@ -249,10 +285,11 @@ module.exports = {
}

checkRestrictedPathAndReport(importSource, importNames, node);

if (isRestrictedPattern(importSource)) {
reportPathForPatterns(node);
}
restrictedPatternGroups.forEach(group => {
if (isRestrictedPattern(importSource, group)) {
reportPathForPatterns(node, group);
}
});
}

return {
Expand Down
34 changes: 34 additions & 0 deletions tests/lib/rules/no-restricted-imports.js
View file
Expand Up @@ -37,6 +37,10 @@ ruleTester.run("no-restricted-imports", rule, {
code: "import withGitignores from \"foo/bar\";",
options: [{ patterns: ["foo/*", "!foo/bar"] }]
},
{
code: "import withPatterns from \"foo/bar\";",
options: [{ patterns: [{ group: ["foo/*", "!foo/bar"], message: "foo is forbidden, use bar instead" }] }]
},
{
code: "import AllowedObject from \"foo\";",
options: [{
Expand Down Expand Up @@ -241,6 +245,36 @@ ruleTester.run("no-restricted-imports", rule, {
column: 1,
endColumn: 36
}]
}, {
code: "import withPatterns from \"foo/baz\";",
options: [{ patterns: [{ group: ["foo/*", "!foo/bar"], message: "foo is forbidden, use foo/bar instead" }] }],
errors: [{
message: "'foo/baz' import is restricted from being used by a pattern. foo is forbidden, use foo/bar instead",
type: "ImportDeclaration",
line: 1,
column: 1,
endColumn: 36
}]
}, {
code: "import withPatterns from \"foo/baz\";",
options: [{ patterns: [{ group: ["foo/bar", "foo/baz"], message: "some foo subimports are restricted" }] }],
errors: [{
message: "'foo/baz' import is restricted from being used by a pattern. some foo subimports are restricted",
type: "ImportDeclaration",
line: 1,
column: 1,
endColumn: 36
}]
}, {
code: "import withPatterns from \"foo/bar\";",
options: [{ patterns: [{ group: ["foo/bar"] }] }],
errors: [{
message: "'foo/bar' import is restricted from being used by a pattern.",
type: "ImportDeclaration",
line: 1,
column: 1,
endColumn: 36
}]
}, {
code: "import withGitignores from \"foo/bar\";",
options: [{ patterns: ["foo/*", "!foo/baz"] }],
Expand Down

0 comments on commit 52655dd

Please sign in to comment.