From ba96189fdd86656bfdbff4feea5e7ceaf37e32b5 Mon Sep 17 00:00:00 2001 From: Brandon Scott Date: Sun, 26 Jun 2022 12:00:59 -0400 Subject: [PATCH 1/6] feat: add importNames support for restricted import patterns Fixes #14274 --- lib/rules/no-restricted-imports.js | 62 ++++++++++++++++++---- tests/lib/rules/no-restricted-imports.js | 66 ++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 11 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index 3bb45d715ff..bc3bd680507 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -58,6 +58,12 @@ const arrayOfStringsOrObjectPatterns = { items: { type: "object", properties: { + importNames: { + type: "array", + items: { + type: "string" + } + }, group: { type: "array", items: { @@ -159,9 +165,10 @@ module.exports = { } // relative paths are supported for this rule - const restrictedPatternGroups = restrictedPatterns.map(({ group, message, caseSensitive }) => ({ + const restrictedPatternGroups = restrictedPatterns.map(({ group, message, caseSensitive, importNames }) => ({ matcher: ignore({ allowRelativePaths: true, ignorecase: !caseSensitive }).add(group), - customMessage: message + customMessage: message, + importNames })); // if no imports are restricted we don't need to check @@ -234,20 +241,53 @@ 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 + * @param {Object} group contains an Ignore instance for paths, the customMessage to show on failure, + * and any restricted import names that have been specified in the config + * @param {Map} importNames Map of import names that are being imported * @returns {void} * @private */ - function reportPathForPatterns(node, group) { + function reportPathForPatterns(node, group, importNames) { const importSource = node.source.value.trim(); - context.report({ - node, - messageId: group.customMessage ? "patternWithCustomMessage" : "patterns", - data: { - importSource, - customMessage: group.customMessage + const customMessage = group.customMessage; + const restrictedImportNames = group.importNames || []; + + /* + * If we are not restricting to any specific import names and just the pattern itself, + * report the error and move on + */ + if (restrictedImportNames.length < 1) { + context.report({ + node, + messageId: customMessage ? "patternWithCustomMessage" : "patterns", + data: { + importSource, + customMessage + } + }); + return; + } + + restrictedImportNames.forEach(importName => { + if (!importNames.has(importName)) { + return; } + + const specifiers = importNames.get(importName); + + specifiers.forEach(specifier => { + context.report({ + node, + messageId: customMessage ? "importNameWithCustomMessage" : "importName", + loc: specifier.loc, + data: { + importSource, + customMessage, + importName + } + }); + }); }); } @@ -304,7 +344,7 @@ module.exports = { checkRestrictedPathAndReport(importSource, importNames, node); restrictedPatternGroups.forEach(group => { if (isRestrictedPattern(importSource, group)) { - reportPathForPatterns(node, group); + reportPathForPatterns(node, group, importNames); } }); } diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 07d05cbff60..27694b08ab0 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -262,6 +262,37 @@ ruleTester.run("no-restricted-imports", rule, { importNames: ["DisallowedObject"] }] }] + }, + { + code: "import { Bar } from '../../my/relative-module';", + options: [{ + patterns: [{ + group: ["**/my/relative-module"], + importNames: ["Foo"] + }] + }] + }, + { + + // Default import should not be reported - just drop the importNames option for this behavior + code: "import Foo from '../../my/relative-module';", + options: [{ + patterns: [{ + group: ["**/my/relative-module"], + importNames: ["Foo"] + }] + }] + }, + { + + // Star import should not be reported - just drop the importNames option for this behavior + code: "import * as Foo from '../../my/relative-module';", + options: [{ + patterns: [{ + group: ["**/my/relative-module"], + importNames: ["Foo"] + }] + }] } ], invalid: [{ @@ -1094,6 +1125,41 @@ ruleTester.run("no-restricted-imports", rule, { column: 1, endColumn: 45 }] + }, + { + code: "import { Foo } from '../../my/relative-module';", + options: [{ + patterns: [{ + group: ["**/my/relative-module"], + importNames: ["Foo"] + }] + }], + errors: [{ + type: "ImportDeclaration", + line: 1, + column: 10, + endColumn: 13 + }] + }, + { + code: "import { Foo, Bar } from '../../my/relative-module';", + options: [{ + patterns: [{ + group: ["**/my/relative-module"], + importNames: ["Foo", "Bar"] + }] + }], + errors: [{ + type: "ImportDeclaration", + line: 1, + column: 10, + endColumn: 13 + }, { + type: "ImportDeclaration", + line: 1, + column: 15, + endColumn: 18 + }] } ] }); From 00e026284e1d3eaeca4d781ca250a41c7cc2aba7 Mon Sep 17 00:00:00 2001 From: Brandon Scott Date: Tue, 28 Jun 2022 19:58:37 -0400 Subject: [PATCH 2/6] refactor: Report * import as error, add new messageIds, update docs --- docs/src/rules/no-restricted-imports.md | 12 ++++++++ lib/rules/no-restricted-imports.js | 8 +++-- tests/lib/rules/no-restricted-imports.js | 37 +++++++++++++++--------- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/docs/src/rules/no-restricted-imports.md b/docs/src/rules/no-restricted-imports.md index 5c03814855d..01c6f7a68a6 100644 --- a/docs/src/rules/no-restricted-imports.md +++ b/docs/src/rules/no-restricted-imports.md @@ -109,6 +109,18 @@ Pattern matches can also be configured to be case-sensitive: }] ``` +Pattern matches can restrict specific import names only, similar to the `paths` option: + +```json +"no-restricted-imports": ["error", { + "patterns": [{ + "group": ["utils/*"], + "importNames": ["isEmpty"], + "message": "Use 'isEmpty' from lodash instead." + }] +}] +``` + To restrict the use of all Node.js core imports (via ): ```json diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index bc3bd680507..7b347f83c97 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -108,6 +108,10 @@ module.exports = { // eslint-disable-next-line eslint-plugin/report-message-format -- Custom message might not end in a period patternWithCustomMessage: "'{{importSource}}' import is restricted from being used by a pattern. {{customMessage}}", + patternAndImportName: "'{{importName}}' import from '{{importSource}}' is restricted from being used by a pattern.", + // eslint-disable-next-line eslint-plugin/report-message-format -- Custom message might not end in a period + patternAndImportNameWithCustomMessage: "'{{importName}}' import from '{{importSource}}' 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 -- Custom message might not end in a period everythingWithCustomMessage: "* import is invalid because '{{importNames}}' from '{{importSource}}' is restricted. {{customMessage}}", @@ -257,7 +261,7 @@ module.exports = { * If we are not restricting to any specific import names and just the pattern itself, * report the error and move on */ - if (restrictedImportNames.length < 1) { + if (restrictedImportNames.length < 1 || importNames.has("*")) { context.report({ node, messageId: customMessage ? "patternWithCustomMessage" : "patterns", @@ -279,7 +283,7 @@ module.exports = { specifiers.forEach(specifier => { context.report({ node, - messageId: customMessage ? "importNameWithCustomMessage" : "importName", + messageId: customMessage ? "patternAndImportNameWithCustomMessage" : "patternAndImportName", loc: specifier.loc, data: { importSource, diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 27694b08ab0..a26cbe6d70c 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -282,17 +282,6 @@ ruleTester.run("no-restricted-imports", rule, { importNames: ["Foo"] }] }] - }, - { - - // Star import should not be reported - just drop the importNames option for this behavior - code: "import * as Foo from '../../my/relative-module';", - options: [{ - patterns: [{ - group: ["**/my/relative-module"], - importNames: ["Foo"] - }] - }] } ], invalid: [{ @@ -1146,19 +1135,39 @@ ruleTester.run("no-restricted-imports", rule, { options: [{ patterns: [{ group: ["**/my/relative-module"], - importNames: ["Foo", "Bar"] + importNames: ["Foo", "Bar"], + message: "Import from @/utils instead." }] }], errors: [{ type: "ImportDeclaration", line: 1, column: 10, - endColumn: 13 + endColumn: 13, + message: "'Foo' import from '../../my/relative-module' is restricted from being used by a pattern. Import from @/utils instead." }, { type: "ImportDeclaration", line: 1, column: 15, - endColumn: 18 + endColumn: 18, + message: "'Bar' import from '../../my/relative-module' is restricted from being used by a pattern. Import from @/utils instead." + }] + }, + { + + // Star import should be reported for consistency with `paths` option (see: https://github.com/eslint/eslint/pull/16059#discussion_r908749964) + code: "import * as Foo from '../../my/relative-module';", + options: [{ + patterns: [{ + group: ["**/my/relative-module"], + importNames: ["Foo"] + }] + }], + errors: [{ + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 49 }] } ] From 10547f7f1675df110e144f922da72dd6d1835ebe Mon Sep 17 00:00:00 2001 From: Brandon Scott Date: Wed, 29 Jun 2022 12:45:32 -0400 Subject: [PATCH 3/6] docs: add code examples for patterns with importNames --- docs/src/rules/no-restricted-imports.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/src/rules/no-restricted-imports.md b/docs/src/rules/no-restricted-imports.md index 01c6f7a68a6..3bcdb04e1b0 100644 --- a/docs/src/rules/no-restricted-imports.md +++ b/docs/src/rules/no-restricted-imports.md @@ -219,6 +219,16 @@ import pick from 'lodash/pick'; import pick from 'fooBar'; ``` +```js +/*eslint no-restricted-imports: ["error", { patterns: [{ + group: ["utils/*"], + importNames: ['isEmpty'], + message: "Use 'isEmpty' from lodash instead." +}]}]*/ + +import { isEmpty } from 'utils/collection-utils'; +``` + Examples of **correct** code for this rule: ::: correct @@ -274,6 +284,16 @@ import lodash from 'lodash'; import pick from 'food'; ``` +```js +/*eslint no-restricted-imports: ["error", { patterns: [{ + group: ["utils/*"], + importNames: ['isEmpty'], + message: "Use 'isEmpty' from lodash instead." +}]}]*/ + +import { hasValues } from 'utils/collection-utils'; +``` + ## 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. From 462e7c6dc330420cc72f2514531cd312a1970db6 Mon Sep 17 00:00:00 2001 From: Brandon Scott Date: Wed, 29 Jun 2022 19:55:31 -0400 Subject: [PATCH 4/6] refactor: handle star import case separately w/ new messageIds --- lib/rules/no-restricted-imports.js | 23 ++++++++++++++++++++++- tests/lib/rules/no-restricted-imports.js | 12 ++++++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index 7b347f83c97..fdb9f41341b 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -112,6 +112,10 @@ module.exports = { // eslint-disable-next-line eslint-plugin/report-message-format -- Custom message might not end in a period patternAndImportNameWithCustomMessage: "'{{importName}}' import from '{{importSource}}' is restricted from being used by a pattern. {{customMessage}}", + patternAndEverything: "* import is invalid because '{{importNames}}' from '{{importSource}}' is restricted from being used by a pattern.", + // eslint-disable-next-line eslint-plugin/report-message-format -- Custom message might not end in a period + patternAndEverythingWithCustomMessage: "* import is invalid because '{{importNames}}' from '{{importSource}}' 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 -- Custom message might not end in a period everythingWithCustomMessage: "* import is invalid because '{{importNames}}' from '{{importSource}}' is restricted. {{customMessage}}", @@ -261,7 +265,7 @@ module.exports = { * If we are not restricting to any specific import names and just the pattern itself, * report the error and move on */ - if (restrictedImportNames.length < 1 || importNames.has("*")) { + if (restrictedImportNames.length < 1) { context.report({ node, messageId: customMessage ? "patternWithCustomMessage" : "patterns", @@ -273,6 +277,23 @@ module.exports = { return; } + if (importNames.has("*")) { + const specifierData = importNames.get("*")[0]; + + context.report({ + node, + messageId: customMessage ? "patternAndEverythingWithCustomMessage" : "patternAndEverything", + loc: specifierData.loc, + data: { + importSource, + importNames: restrictedImportNames, + customMessage + } + }); + + return; + } + restrictedImportNames.forEach(importName => { if (!importNames.has(importName)) { return; diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index a26cbe6d70c..78cd9a91f0e 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -1155,8 +1155,11 @@ ruleTester.run("no-restricted-imports", rule, { }, { - // Star import should be reported for consistency with `paths` option (see: https://github.com/eslint/eslint/pull/16059#discussion_r908749964) - code: "import * as Foo from '../../my/relative-module';", + /* + * Star import should be reported for consistency with `paths` option (see: https://github.com/eslint/eslint/pull/16059#discussion_r908749964) + * For example, import * as All allows for calling/referencing the restricted import All.Foo + */ + code: "import * as All from '../../my/relative-module';", options: [{ patterns: [{ group: ["**/my/relative-module"], @@ -1164,10 +1167,11 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ + message: "* import is invalid because 'Foo' from '../../my/relative-module' is restricted from being used by a pattern.", type: "ImportDeclaration", line: 1, - column: 1, - endColumn: 49 + column: 8, + endColumn: 16 }] } ] From a748be12c07347559d4b457e926336d8e6013c75 Mon Sep 17 00:00:00 2001 From: Brandon Scott Date: Thu, 30 Jun 2022 12:55:38 -0400 Subject: [PATCH 5/6] refactor: add message assertions + additional test cases, remove early return to catch default import case --- lib/rules/no-restricted-imports.js | 2 - tests/lib/rules/no-restricted-imports.js | 66 +++++++++++++++++++++++- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index fdb9f41341b..37893aeb0fa 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -290,8 +290,6 @@ module.exports = { customMessage } }); - - return; } restrictedImportNames.forEach(importName => { diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 78cd9a91f0e..5403812de36 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -274,7 +274,7 @@ ruleTester.run("no-restricted-imports", rule, { }, { - // Default import should not be reported - just drop the importNames option for this behavior + // Default import should not be reported unless importNames includes 'default' code: "import Foo from '../../my/relative-module';", options: [{ patterns: [{ @@ -1127,7 +1127,8 @@ ruleTester.run("no-restricted-imports", rule, { type: "ImportDeclaration", line: 1, column: 10, - endColumn: 13 + endColumn: 13, + message: "'Foo' import from '../../my/relative-module' is restricted from being used by a pattern." }] }, { @@ -1173,6 +1174,67 @@ ruleTester.run("no-restricted-imports", rule, { column: 8, endColumn: 16 }] + }, + { + + /* + * Star import should be reported for consistency with `paths` option (see: https://github.com/eslint/eslint/pull/16059#discussion_r908749964) + * For example, import * as All allows for calling/referencing the restricted import All.Foo + */ + code: "import * as AllWithCustomMessage from '../../my/relative-module';", + options: [{ + patterns: [{ + group: ["**/my/relative-module"], + importNames: ["Foo"], + message: "Import from @/utils instead." + }] + }], + errors: [{ + message: "* import is invalid because 'Foo' from '../../my/relative-module' is restricted from being used by a pattern. Import from @/utils instead.", + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 33 + }] + }, + { + code: "import def, * as ns from 'mod';", + options: [{ + patterns: [{ + group: ["mod"], + importNames: ["default"] + }] + }], + errors: [{ + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 11, + message: "'default' import from 'mod' is restricted from being used by a pattern." + }, + { + type: "ImportDeclaration", + line: 1, + column: 13, + endColumn: 20, + message: "* import is invalid because 'default' from 'mod' is restricted from being used by a pattern." + }] + }, + { + code: "import Foo from 'mod';", + options: [{ + patterns: [{ + group: ["mod"], + importNames: ["default"] + }] + }], + errors: [{ + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 11, + message: "'default' import from 'mod' is restricted from being used by a pattern." + }] } ] }); From fdd0dc129eee8f0cb02328b6bafa0047a2ba00f4 Mon Sep 17 00:00:00 2001 From: Brandon Scott Date: Thu, 30 Jun 2022 12:59:01 -0400 Subject: [PATCH 6/6] refactor: require 1 unique importName in config, remove array coalesce --- lib/rules/no-restricted-imports.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index 37893aeb0fa..630c1229d94 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -62,7 +62,9 @@ const arrayOfStringsOrObjectPatterns = { type: "array", items: { type: "string" - } + }, + minItems: 1, + uniqueItems: true }, group: { type: "array", @@ -259,13 +261,13 @@ module.exports = { const importSource = node.source.value.trim(); const customMessage = group.customMessage; - const restrictedImportNames = group.importNames || []; + const restrictedImportNames = group.importNames; /* * If we are not restricting to any specific import names and just the pattern itself, * report the error and move on */ - if (restrictedImportNames.length < 1) { + if (!restrictedImportNames) { context.report({ node, messageId: customMessage ? "patternWithCustomMessage" : "patterns",