Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: no-restricted-imports custom message for patterns (fixes #11843) #14580

Merged
34 changes: 33 additions & 1 deletion docs/rules/no-restricted-imports.md
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
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 = {
lexholden marked this conversation as resolved.
Show resolved Hide resolved
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
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
}]
lexholden marked this conversation as resolved.
Show resolved Hide resolved
}, {
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