Skip to content

Commit

Permalink
Update: no-restricted-imports rule with custom messages (fixes #11843)
Browse files Browse the repository at this point in the history
  • Loading branch information
lexholden committed May 8, 2021
1 parent ee3a3ea commit 2a56777
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 23 deletions.
11 changes: 11 additions & 0 deletions docs/rules/no-restricted-imports.md
Expand Up @@ -75,6 +75,17 @@ or like this if you need to restrict only certain imports from a module:
}]
```

Or like this if you want to apply a custom message to pattern matches:

```json
"no-restricted-imports": ["error", {
"patterns": [{
"group": ["import1/private/*", "import2/*", "!import2/good"],
"message": "import1 private modules and most of import2 are deprecated, please don't use them in new code"
}]
}]
```

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.

To restrict the use of all Node.js core imports (via https://github.com/nodejs/node/tree/master/lib):
Expand Down
101 changes: 78 additions & 23 deletions lib/rules/no-restricted-imports.js
Expand Up @@ -10,21 +10,49 @@

const ignore = require("ignore");

const arrayOfStrings = {
const arrayOfStringsOrObjects = {
type: "array",
items: { type: "string" },
items: {
anyOf: [
{ type: "string" },
{
type: "object",
properties: {
name: { type: "string" },
message: {
type: "string",
minLength: 1
},
importNames: {
type: "array",
items: {
type: "string"
}
}
},
additionalProperties: false,
required: ["name"]
}
]
},
uniqueItems: true
};

const arrayOfStringsOrObjects = {
const arrayOfStringsOrObjectPatterns = {
type: "array",
items: {
anyOf: [
{ type: "string" },
{
type: "object",
properties: {
name: { type: "string" },
group: {
type: "array",
items: {
type: "string"
},
minLength: 1
},
message: {
type: "string",
minLength: 1
Expand All @@ -37,7 +65,7 @@ const arrayOfStringsOrObjects = {
}
},
additionalProperties: false,
required: ["name"]
required: ["group"]
}
]
},
Expand All @@ -61,6 +89,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 +110,7 @@ module.exports = {
type: "object",
properties: {
paths: arrayOfStringsOrObjects,
patterns: arrayOfStrings
patterns: arrayOfStringsOrObjectPatterns
},
additionalProperties: false
}],
Expand All @@ -98,13 +128,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 +140,35 @@ module.exports = {
return memo;
}, {});

const restrictedPatternsMatcher = ignore().add(restrictedPatterns);
// Handle patterns too, either as strings or groups
const restrictedPatterns = (isPathAndPatternsObject ? options[0].patterns : []) || [];
const ungroupedRestrictedPatterns = [];
const restrictedPatternGroups = restrictedPatterns.reduce((groups, importSource) => {
if (typeof importSource === "string") {

// Nothing, no message for a string pattern, so just add to default ignoreGroup
ungroupedRestrictedPatterns.push(importSource);
} else {

// Multiple patterns grouped that probably have a message, so create a new ignore
groups.push({
matcher: ignore().add(importSource.group),
customMessage: importSource.message
});
}
return groups;
}, []);

if (ungroupedRestrictedPatterns.length > 0) {
restrictedPatternGroups.push({
matcher: ignore().add(ungroupedRestrictedPatterns)
});
}

// 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 +235,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 +303,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
14 changes: 14 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,16 @@ 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 withGitignores from \"foo/bar\";",
options: [{ patterns: ["foo/*", "!foo/baz"] }],
Expand Down

0 comments on commit 2a56777

Please sign in to comment.