From 2a56777ab5165d702632d55a9c17de06324f49e1 Mon Sep 17 00:00:00 2001 From: lexholden Date: Sat, 8 May 2021 15:41:20 -0400 Subject: [PATCH 1/6] Update: no-restricted-imports rule with custom messages (fixes #11843) --- docs/rules/no-restricted-imports.md | 11 +++ lib/rules/no-restricted-imports.js | 101 +++++++++++++++++------ tests/lib/rules/no-restricted-imports.js | 14 ++++ 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/docs/rules/no-restricted-imports.md b/docs/rules/no-restricted-imports.md index 0f6841340c9..d6da17256f4 100644 --- a/docs/rules/no-restricted-imports.md +++ b/docs/rules/no-restricted-imports.md @@ -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): diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index c205dad8bdb..650aa1ef69a 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -10,13 +10,35 @@ 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: [ @@ -24,7 +46,13 @@ const arrayOfStringsOrObjects = { { type: "object", properties: { - name: { type: "string" }, + group: { + type: "array", + items: { + type: "string" + }, + minLength: 1 + }, message: { type: "string", minLength: 1 @@ -37,7 +65,7 @@ const arrayOfStringsOrObjects = { } }, additionalProperties: false, - required: ["name"] + required: ["group"] } ] }, @@ -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 @@ -80,7 +110,7 @@ module.exports = { type: "object", properties: { paths: arrayOfStringsOrObjects, - patterns: arrayOfStrings + patterns: arrayOfStringsOrObjectPatterns }, additionalProperties: false }], @@ -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 }; @@ -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. @@ -184,17 +235,19 @@ 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 } }); } @@ -202,11 +255,12 @@ module.exports = { /** * 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); } /** @@ -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 { diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 363229712c9..ad4f0496d50 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -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: [{ @@ -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"] }], From 49cd91f1fd63ac1f57c1883f8dabde4c1080c34b Mon Sep 17 00:00:00 2001 From: Alex Holden Date: Wed, 19 May 2021 09:12:44 -0400 Subject: [PATCH 2/6] Update docs/rules/no-restricted-imports.md Co-authored-by: Milos Djermanovic --- docs/rules/no-restricted-imports.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/no-restricted-imports.md b/docs/rules/no-restricted-imports.md index d6da17256f4..f71800ef3b9 100644 --- a/docs/rules/no-restricted-imports.md +++ b/docs/rules/no-restricted-imports.md @@ -75,7 +75,7 @@ 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: +or like this if you want to apply a custom message to pattern matches: ```json "no-restricted-imports": ["error", { From 88599ae8ac267148e1acf71dfeeae1b3d022cfde Mon Sep 17 00:00:00 2001 From: Alex Holden Date: Wed, 19 May 2021 09:16:31 -0400 Subject: [PATCH 3/6] Update docs/rules/no-restricted-imports.md Co-authored-by: Milos Djermanovic --- docs/rules/no-restricted-imports.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/no-restricted-imports.md b/docs/rules/no-restricted-imports.md index f71800ef3b9..9f658467d93 100644 --- a/docs/rules/no-restricted-imports.md +++ b/docs/rules/no-restricted-imports.md @@ -86,7 +86,7 @@ or like this if you want to apply a custom message to pattern matches: }] ``` -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. +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): From 2e0cfde383b49f8e709f97701430bcb7e0006d92 Mon Sep 17 00:00:00 2001 From: lexholden Date: Wed, 19 May 2021 09:28:45 -0400 Subject: [PATCH 4/6] Code review feedback - better test cases and schema change --- lib/rules/no-restricted-imports.js | 58 ++++++++---------------- tests/lib/rules/no-restricted-imports.js | 20 ++++++++ 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index 650aa1ef69a..414164d29f7 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -39,11 +39,17 @@ const arrayOfStringsOrObjects = { }; const arrayOfStringsOrObjectPatterns = { - type: "array", - items: { - anyOf: [ - { type: "string" }, - { + anyOf: [ + { + type: "array", + items: { + type: "string" + }, + uniqueItems: true + }, + { + type: "array", + items: { type: "object", properties: { group: { @@ -51,25 +57,20 @@ const arrayOfStringsOrObjectPatterns = { items: { type: "string" }, - minLength: 1 + minItems: 1, + uniqueItems: true }, message: { type: "string", minLength: 1 - }, - importNames: { - type: "array", - items: { - type: "string" - } } }, additionalProperties: false, required: ["group"] - } - ] - }, - uniqueItems: true + }, + uniqueItems: true + } + ] }; module.exports = { @@ -142,28 +143,9 @@ module.exports = { // 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) - }); - } + 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) { diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index ad4f0496d50..f54d4df0354 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -255,6 +255,26 @@ ruleTester.run("no-restricted-imports", rule, { 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"] }], From ca2187847db89fd87cd61c20f41e635646517fb1 Mon Sep 17 00:00:00 2001 From: lexholden Date: Wed, 19 May 2021 09:31:07 -0400 Subject: [PATCH 5/6] Doc updates --- docs/rules/no-restricted-imports.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-restricted-imports.md b/docs/rules/no-restricted-imports.md index 9f658467d93..65de662fb75 100644 --- a/docs/rules/no-restricted-imports.md +++ b/docs/rules/no-restricted-imports.md @@ -80,8 +80,11 @@ 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" + "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." }] }] ``` From 20dabca7d6942aa36c8ad2a0dd790da117111553 Mon Sep 17 00:00:00 2001 From: lexholden Date: Thu, 20 May 2021 08:31:07 -0400 Subject: [PATCH 6/6] Added correct/incorrect examples to docs --- docs/rules/no-restricted-imports.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/rules/no-restricted-imports.md b/docs/rules/no-restricted-imports.md index 65de662fb75..f1577d7d89d 100644 --- a/docs/rules/no-restricted-imports.md +++ b/docs/rules/no-restricted-imports.md @@ -163,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 @@ -196,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.