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
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:
lexholden marked this conversation as resolved.
Show resolved Hide resolved

```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"
}]
lexholden marked this conversation as resolved.
Show resolved Hide resolved
}]
```

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.
lexholden marked this conversation as resolved.
Show resolved Hide resolved

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 = {
lexholden marked this conversation as resolved.
Show resolved Hide resolved
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)
});
}
lexholden marked this conversation as resolved.
Show resolved Hide resolved

// 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
}]
lexholden marked this conversation as resolved.
Show resolved Hide resolved
}, {
code: "import withGitignores from \"foo/bar\";",
options: [{ patterns: ["foo/*", "!foo/baz"] }],
Expand Down