From f597ed404df797dbfcbf1015cdc9434f613c34c7 Mon Sep 17 00:00:00 2001 From: fanich37 Date: Fri, 20 Dec 2019 18:06:52 +0300 Subject: [PATCH 01/10] Process importNames --- lib/rules/no-restricted-imports.js | 54 +++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index ec0696f99a2..c8fe933693d 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -125,22 +125,49 @@ module.exports = { /** * Report a restricted path. + * @param {string} importSource path of the import + * @param {Set.} importNames Set of import names that are being imported * @param {node} node representing the restricted path reference * @returns {void} * @private */ - function reportPath(node) { - const importSource = node.source.value.trim(); - const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; - - context.report({ - node, - messageId: customMessage ? "pathWithCustomMessage" : "path", - data: { - importSource, - customMessage - } - }); + function reportPath(importSource, importNames, node) { + const isRestrictedImportNames = restrictedPathMessages[importSource] && + restrictedPathMessages[importSource].importNames && + restrictedPathMessages[importSource].importNames.length; + + if (importNames.has("*") || importNames.has("default") || !isRestrictedImportNames) { + const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; + + context.report({ + node, + messageId: customMessage ? "pathWithCustomMessage" : "path", + data: { + importSource, + customMessage + } + }); + } else { + node.specifiers.forEach(specifier => { + const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; + const specifierKey = specifier.imported ? "imported" : "local"; + + if ( + isRestrictedImportNames && + restrictedPathMessages[importSource].importNames.includes(specifier[specifierKey].name) + ) { + context.report({ + node, + messageId: customMessage ? "pathWithCustomMessage" : "path", + loc: specifier[specifierKey].loc, + data: { + importSource, + customMessage + } + }); + } + }); + } } /** @@ -266,8 +293,9 @@ module.exports = { } if (isRestrictedPath(importSource, importNames)) { - reportPath(node); + reportPath(importSource, importNames, node); } + if (isRestrictedPattern(importSource)) { reportPathForPatterns(node); } From 9a0912710da39e7e3c43d4bd9f8aea2609538435 Mon Sep 17 00:00:00 2001 From: fanich37 Date: Wed, 25 Dec 2019 13:29:54 +0300 Subject: [PATCH 02/10] Update test cases --- lib/rules/no-restricted-imports.js | 19 ++++--- tests/lib/rules/no-restricted-imports.js | 65 +++++++++++++++++++++--- 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index c8fe933693d..f760a5ba69f 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -64,7 +64,11 @@ module.exports = { everything: "* import is invalid because '{{importNames}}' from '{{importSource}}' is restricted.", // eslint-disable-next-line eslint-plugin/report-message-format - everythingWithCustomMessage: "* import is invalid because '{{importNames}}' from '{{importSource}}' is restricted. {{customMessage}}" + everythingWithCustomMessage: "* import is invalid because '{{importNames}}' from '{{importSource}}' is restricted. {{customMessage}}", + + importName: "'{{importName}}' import from '{{importSource}}' is restricted.", + // eslint-disable-next-line eslint-plugin/report-message-format + importNameWithCustomMessage: "'{{importName}}' import from '{{importSource}}' is restricted. {{customMessage}}" }, schema: { @@ -72,14 +76,14 @@ module.exports = { arrayOfStringsOrObjects, { type: "array", - items: [{ + items: { type: "object", properties: { paths: arrayOfStringsOrObjects, patterns: arrayOfStrings }, additionalProperties: false - }], + }, additionalItems: false } ] @@ -132,13 +136,12 @@ module.exports = { * @private */ function reportPath(importSource, importNames, node) { + const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; const isRestrictedImportNames = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].importNames && restrictedPathMessages[importSource].importNames.length; if (importNames.has("*") || importNames.has("default") || !isRestrictedImportNames) { - const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; - context.report({ node, messageId: customMessage ? "pathWithCustomMessage" : "path", @@ -149,7 +152,6 @@ module.exports = { }); } else { node.specifiers.forEach(specifier => { - const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; const specifierKey = specifier.imported ? "imported" : "local"; if ( @@ -158,11 +160,12 @@ module.exports = { ) { context.report({ node, - messageId: customMessage ? "pathWithCustomMessage" : "path", + messageId: customMessage ? "importNameWithCustomMessage" : "importName", loc: specifier[specifierKey].loc, data: { importSource, - customMessage + customMessage, + importName: specifier[specifierKey].name } }); } diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 15ee089b28b..53551338948 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -20,7 +20,6 @@ const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6, sourceType: ruleTester.run("no-restricted-imports", rule, { valid: [ - "import os from \"os\";", { code: "import os from \"os\";", options: ["osx"] }, { code: "import fs from \"fs\";", options: ["crypto"] }, { code: "import path from \"path\";", options: ["crypto", "stream", "os"] }, @@ -164,6 +163,10 @@ ruleTester.run("no-restricted-imports", rule, { message: "Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead." }] }] + }, + { + code: "import {\nAllowedObject,\nDisallowedObject, // eslint-disable-line\n} from \"foo\";", + options: [{ paths: [{ name: "foo", importNames: ["DisallowedObject"] }] }] } ], invalid: [{ @@ -211,7 +214,7 @@ ruleTester.run("no-restricted-imports", rule, { message: "Don't import 'foo'." }] }], - errors: [{ message: "'fs' import is restricted from being used. Don't import 'foo'.", type: "ExportNamedDeclaration" }] + errors: [{ message: "'foo' import from 'fs' is restricted. Don't import 'foo'.", type: "ExportNamedDeclaration" }] }, { code: "import withGitignores from \"foo\";", options: [{ @@ -287,7 +290,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import 'DisallowedObject' from /bar/ instead.", + message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -301,7 +304,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import 'DisallowedObject' from /bar/ instead.", + message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -315,7 +318,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import 'DisallowedObject' from /bar/ instead.", + message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -329,7 +332,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import 'DisallowedObject' from /bar/ instead.", + message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -343,7 +346,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", + message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -357,7 +360,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", + message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -416,6 +419,52 @@ ruleTester.run("no-restricted-imports", rule, { message: "* import is invalid because 'DisallowedObject,DisallowedObjectTwo' from 'foo' is restricted. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", type: "ImportDeclaration" }] + }, + { + code: "import { DisallowedObjectOne, DisallowedObjectTwo, AllowedObject } from \"foo\";", + options: [{ + paths: [{ + name: "foo", + importNames: ["DisallowedObjectOne", "DisallowedObjectTwo"] + }] + }], + errors: [{ + message: "'DisallowedObjectOne' import from 'foo' is restricted.", + type: "ImportDeclaration" + }, { + message: "'DisallowedObjectTwo' import from 'foo' is restricted.", + type: "ImportDeclaration" + }] + }, + { + code: "import { DisallowedObjectOne, DisallowedObjectTwo, AllowedObject } from \"foo\";", + options: [{ + paths: [{ + name: "foo", + importNames: ["DisallowedObjectOne", "DisallowedObjectTwo"], + message: "Please import this module from /bar/ instead." + }] + }], + errors: [{ + message: "'DisallowedObjectOne' import from 'foo' is restricted. Please import this module from /bar/ instead.", + type: "ImportDeclaration" + }, { + message: "'DisallowedObjectTwo' import from 'foo' is restricted. Please import this module from /bar/ instead.", + type: "ImportDeclaration" + }] + }, + { + code: "import { AllowedObject, DisallowedObject as Bar } from \"foo\";", + options: [{ + paths: [{ + name: "foo", + importNames: ["DisallowedObject"] + }] + }], + errors: [{ + message: "'DisallowedObject' import from 'foo' is restricted.", + type: "ImportDeclaration" + }] } ] }); From 41979a89540a17cc268bf847c02f23db625b75d6 Mon Sep 17 00:00:00 2001 From: fanich37 Date: Thu, 26 Dec 2019 10:02:39 +0300 Subject: [PATCH 03/10] Fix rebase issue --- lib/rules/no-restricted-imports.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index f760a5ba69f..63bb5152999 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -76,14 +76,14 @@ module.exports = { arrayOfStringsOrObjects, { type: "array", - items: { + items: [{ type: "object", properties: { paths: arrayOfStringsOrObjects, patterns: arrayOfStrings }, additionalProperties: false - }, + }], additionalItems: false } ] From 21fbe208e83593b4ba604c89ac7b927e2b051c7e Mon Sep 17 00:00:00 2001 From: fanich37 Date: Thu, 2 Jan 2020 14:15:36 +0300 Subject: [PATCH 04/10] Update importNames logic --- lib/rules/no-restricted-imports.js | 67 +++++++++++------------- tests/lib/rules/no-restricted-imports.js | 48 +++++++++++++++-- 2 files changed, 77 insertions(+), 38 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index 63bb5152999..531fb07db34 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -120,7 +120,7 @@ module.exports = { /** * Checks to see if "*" is being used to import everything. - * @param {Set.} importNames Set of import names that are being imported + * @param {Map} importNames Map of import names that are being imported * @returns {boolean} whether everything is imported or not */ function isEverythingImported(importNames) { @@ -130,46 +130,41 @@ module.exports = { /** * Report a restricted path. * @param {string} importSource path of the import - * @param {Set.} importNames Set of import names that are being imported + * @param {Map { - const specifierKey = specifier.imported ? "imported" : "local"; + if (restrictedImportNames) { + restrictedImportNames.forEach(importName => { + if (importNames.has(importName)) { + const data = importNames.get(importName); - if ( - isRestrictedImportNames && - restrictedPathMessages[importSource].importNames.includes(specifier[specifierKey].name) - ) { context.report({ node, messageId: customMessage ? "importNameWithCustomMessage" : "importName", - loc: specifier[specifierKey].loc, + loc: data.loc, data: { importSource, customMessage, - importName: specifier[specifierKey].name + importName } }); } }); + } else { + context.report({ + node, + messageId: customMessage ? "pathWithCustomMessage" : "path", + data: { + importSource, + customMessage + } + }); } } @@ -216,7 +211,7 @@ module.exports = { /** * Check if the given importSource is restricted because '*' is being imported. * @param {string} importSource path of the import - * @param {Set.} importNames Set of import names that are being imported + * @param {Map} importNames Set of import names that are being imported + * @param {Map} importNames Set of import names that are being imported + * @param {Map { + const importNames = node.specifiers ? node.specifiers.reduce((map, specifier) => { + const specifierKey = specifier.imported ? "imported" : "local"; + const specifierData = { loc: specifier.loc.start }; + if (specifier.type === "ImportDefaultSpecifier") { - set.add("default"); + map.set("default", specifierData); } else if (specifier.type === "ImportNamespaceSpecifier") { - set.add("*"); - } else if (specifier.imported) { - set.add(specifier.imported.name); - } else if (specifier.local) { - set.add(specifier.local.name); + map.set("*", specifierData); + } else { + map.set(specifier[specifierKey].name, specifierData); } - return set; - }, new Set()) : new Set(); + + return map; + }, new Map()) : new Map(); if (isRestrictedForEverythingImported(importSource, importNames)) { reportPathForEverythingImported(importSource, node); diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 53551338948..5dcaf2580f7 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -262,7 +262,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import the default import of 'foo' from /bar/ instead.", + message: "'default' import from 'foo' is restricted. Please import the default import of 'foo' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -374,7 +374,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import the default import of 'foo' from /bar/ instead.", + message: "'default' import from 'foo' is restricted. Please import the default import of 'foo' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -388,7 +388,7 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "'foo' import is restricted from being used. Please import 'DisallowedObject' from /bar/ instead.", + message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", type: "ImportDeclaration" }] }, @@ -465,6 +465,48 @@ ruleTester.run("no-restricted-imports", rule, { message: "'DisallowedObject' import from 'foo' is restricted.", type: "ImportDeclaration" }] + }, + { + code: "import foo, { bar } from 'mod';", + options: [{ + paths: [{ + name: "mod", + importNames: ["bar"] + }] + }], + errors: [{ + message: "'bar' import from 'mod' is restricted.", + type: "ImportDeclaration" + }] + }, + { + code: "import foo, { bar } from 'mod';", + options: [{ + paths: [{ + name: "mod", + importNames: ["default"] + }] + }], + errors: [{ + message: "'default' import from 'mod' is restricted.", + type: "ImportDeclaration" + }] + }, + { + code: "import foo, * as bar from 'mod';", + options: [{ + paths: [{ + name: "mod", + importNames: ["default"] + }] + }], + errors: [{ + message: "* import is invalid because 'default' from 'mod' is restricted.", + type: "ImportDeclaration" + }, { + message: "'default' import from 'mod' is restricted.", + type: "ImportDeclaration" + }] } ] }); From 47a9dbedb062784ab946b641a787c670f12b8d98 Mon Sep 17 00:00:00 2001 From: fanich37 Date: Sun, 5 Jan 2020 02:03:01 +0300 Subject: [PATCH 05/10] Remove useless funcs, update tests --- lib/rules/no-restricted-imports.js | 121 ++++---------- tests/lib/rules/no-restricted-imports.js | 204 +++++++++++++++++++---- 2 files changed, 196 insertions(+), 129 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index 531fb07db34..d38f2c38710 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -99,6 +99,11 @@ module.exports = { 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 }; @@ -111,22 +116,8 @@ module.exports = { return memo; }, {}); - // if no imports are restricted we don"t need to check - if (Object.keys(restrictedPaths).length === 0 && restrictedPatterns.length === 0) { - return {}; - } - const restrictedPatternsMatcher = ignore().add(restrictedPatterns); - /** - * Checks to see if "*" is being used to import everything. - * @param {Map} importNames Map of import names that are being imported - * @returns {boolean} whether everything is imported or not - */ - function isEverythingImported(importNames) { - return importNames.has("*"); - } - /** * Report a restricted path. * @param {string} importSource path of the import @@ -136,8 +127,27 @@ module.exports = { * @private */ function reportPath(importSource, importNames, node) { - const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; - const restrictedImportNames = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].importNames; + if (!restrictedPathMessages[importSource]) { + return; + } + + const customMessage = restrictedPathMessages[importSource].message; + const restrictedImportNames = restrictedPathMessages[importSource].importNames; + + if (importNames.has("*")) { + const data = importNames.get("*"); + + context.report({ + node, + messageId: customMessage ? "everythingWithCustomMessage" : "everything", + loc: data.loc, + data: { + importSource, + importNames: restrictedImportNames, + customMessage + } + }); + } if (restrictedImportNames) { restrictedImportNames.forEach(importName => { @@ -186,75 +196,6 @@ module.exports = { }); } - /** - * Report a restricted path specifically when using the '*' import. - * @param {string} importSource path of the import - * @param {node} node representing the restricted path reference - * @returns {void} - * @private - */ - function reportPathForEverythingImported(importSource, node) { - const importNames = restrictedPathMessages[importSource].importNames; - const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message; - - context.report({ - node, - messageId: customMessage ? "everythingWithCustomMessage" : "everything", - data: { - importSource, - importNames, - customMessage - } - }); - } - - /** - * Check if the given importSource is restricted because '*' is being imported. - * @param {string} importSource path of the import - * @param {Map ( - importNames.has(restrictedObjectName) - )); - } - - /** - * Check if the given importSource is a restricted path. - * @param {string} importSource path of the import - * @param {Map { const specifierKey = specifier.imported ? "imported" : "local"; - const specifierData = { loc: specifier.loc.start }; + const specifierData = { loc: specifier.loc }; if (specifier.type === "ImportDefaultSpecifier") { map.set("default", specifierData); @@ -288,13 +229,7 @@ module.exports = { return map; }, new Map()) : new Map(); - if (isRestrictedForEverythingImported(importSource, importNames)) { - reportPathForEverythingImported(importSource, node); - } - - if (isRestrictedPath(importSource, importNames)) { - reportPath(importSource, importNames, node); - } + reportPath(importSource, importNames, node); if (isRestrictedPattern(importSource)) { reportPathForPatterns(node); diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 5dcaf2580f7..7e0c288f73a 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -172,39 +172,93 @@ ruleTester.run("no-restricted-imports", rule, { invalid: [{ code: "import \"fs\"", options: ["fs"], - errors: [{ message: "'fs' import is restricted from being used.", type: "ImportDeclaration" }] + errors: [{ + message: "'fs' import is restricted from being used.", + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 12 + }] }, { code: "import os from \"os \";", options: ["fs", "crypto ", "stream", "os"], - errors: [{ message: "'os' import is restricted from being used.", type: "ImportDeclaration" }] + errors: [{ + message: "'os' import is restricted from being used.", + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 22 + }] }, { code: "import \"foo/bar\";", options: ["foo/bar"], - errors: [{ message: "'foo/bar' import is restricted from being used.", type: "ImportDeclaration" }] + errors: [{ + message: "'foo/bar' import is restricted from being used.", + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 18 + }] }, { code: "import withPaths from \"foo/bar\";", options: [{ paths: ["foo/bar"] }], - errors: [{ message: "'foo/bar' import is restricted from being used.", type: "ImportDeclaration" }] + errors: [{ + message: "'foo/bar' import is restricted from being used.", + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 33 + }] }, { code: "import withPatterns from \"foo/bar\";", options: [{ patterns: ["foo"] }], - errors: [{ message: "'foo/bar' import is restricted from being used by a pattern.", type: "ImportDeclaration" }] + errors: [{ + message: "'foo/bar' import is restricted from being used by a pattern.", + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 36 + }] }, { code: "import withPatterns from \"foo/bar\";", options: [{ patterns: ["bar"] }], - errors: [{ message: "'foo/bar' import is restricted from being used by a pattern.", type: "ImportDeclaration" }] + 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"] }], - errors: [{ message: "'foo/bar' import is restricted from being used by a pattern.", type: "ImportDeclaration" }] + errors: [{ + message: "'foo/bar' import is restricted from being used by a pattern.", + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 38 + }] }, { code: "export * from \"fs\";", options: ["fs"], - errors: [{ message: "'fs' import is restricted from being used.", type: "ExportAllDeclaration" }] + errors: [{ + message: "'fs' import is restricted from being used.", + type: "ExportAllDeclaration", + line: 1, + column: 1, + endColumn: 20 + }] }, { code: "export {a} from \"fs\";", options: ["fs"], - errors: [{ message: "'fs' import is restricted from being used.", type: "ExportNamedDeclaration" }] + errors: [{ + message: "'fs' import is restricted from being used.", + type: "ExportNamedDeclaration", + line: 1, + column: 1, + endColumn: 22 + }] }, { code: "export {foo as b} from \"fs\";", options: [{ @@ -214,7 +268,13 @@ ruleTester.run("no-restricted-imports", rule, { message: "Don't import 'foo'." }] }], - errors: [{ message: "'foo' import from 'fs' is restricted. Don't import 'foo'.", type: "ExportNamedDeclaration" }] + errors: [{ + message: "'foo' import from 'fs' is restricted. Don't import 'foo'.", + type: "ExportNamedDeclaration", + line: 1, + column: 9, + endColumn: 17 + }] }, { code: "import withGitignores from \"foo\";", options: [{ @@ -223,7 +283,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'foo' import is restricted from being used. Please import from 'bar' instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 34 }] }, { code: "import withGitignores from \"bar\";", @@ -237,7 +300,10 @@ ruleTester.run("no-restricted-imports", rule, { ], errors: [{ message: "'bar' import is restricted from being used. Please import from 'baz' instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 34 }] }, { code: "import withGitignores from \"foo\";", @@ -249,7 +315,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'foo' import is restricted from being used. Please import from 'bar' instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 34 }] }, { @@ -263,7 +332,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'default' import from 'foo' is restricted. Please import the default import of 'foo' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 24 }] }, { @@ -277,7 +349,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "* import is invalid because 'DisallowedObject' from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 16 }] }, { @@ -291,7 +366,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 10, + endColumn: 26 }] }, { @@ -305,7 +383,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 10, + endColumn: 43 }] }, { @@ -319,7 +400,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 25, + endColumn: 41 }] }, { @@ -333,7 +417,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 25, + endColumn: 61 }] }, { @@ -347,7 +434,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 25, + endColumn: 61 }] }, { @@ -361,7 +451,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 25, + endColumn: 61 }] }, { @@ -375,7 +468,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'default' import from 'foo' is restricted. Please import the default import of 'foo' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 24 }] }, { @@ -389,7 +485,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 25, + endColumn: 61 }] }, { @@ -403,7 +502,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "* import is invalid because 'DisallowedObject' from 'foo' is restricted. Please import 'DisallowedObject' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 23, + endColumn: 44 }] }, { @@ -417,7 +519,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "* import is invalid because 'DisallowedObject,DisallowedObjectTwo' from 'foo' is restricted. Please import 'DisallowedObject' and 'DisallowedObjectTwo' from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 23, + endColumn: 44 }] }, { @@ -430,10 +535,16 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObjectOne' import from 'foo' is restricted.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 10, + endColumn: 29 }, { message: "'DisallowedObjectTwo' import from 'foo' is restricted.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 31, + endColumn: 50 }] }, { @@ -447,10 +558,16 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObjectOne' import from 'foo' is restricted. Please import this module from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 10, + endColumn: 29 }, { message: "'DisallowedObjectTwo' import from 'foo' is restricted. Please import this module from /bar/ instead.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 31, + endColumn: 50 }] }, { @@ -463,7 +580,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'DisallowedObject' import from 'foo' is restricted.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 25, + endColumn: 48 }] }, { @@ -476,7 +596,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'bar' import from 'mod' is restricted.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 15, + endColumn: 18 }] }, { @@ -489,7 +612,10 @@ ruleTester.run("no-restricted-imports", rule, { }], errors: [{ message: "'default' import from 'mod' is restricted.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 11 }] }, { @@ -501,11 +627,17 @@ ruleTester.run("no-restricted-imports", rule, { }] }], errors: [{ - message: "* import is invalid because 'default' from 'mod' is restricted.", - type: "ImportDeclaration" - }, { message: "'default' import from 'mod' is restricted.", - type: "ImportDeclaration" + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 11 + }, { + message: "* import is invalid because 'default' from 'mod' is restricted.", + type: "ImportDeclaration", + line: 1, + column: 13, + endColumn: 21 }] } ] From 347d326af510085f9eb63322334f799fe5134071 Mon Sep 17 00:00:00 2001 From: fanich37 Date: Mon, 6 Jan 2020 00:25:19 +0300 Subject: [PATCH 06/10] Fix naming, fix everything imported w/o importNames --- lib/rules/no-restricted-imports.js | 38 ++++++++++++------------ tests/lib/rules/no-restricted-imports.js | 10 +++++++ 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index d38f2c38710..e28ee53ae22 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -126,7 +126,7 @@ module.exports = { * @returns {void} * @private */ - function reportPath(importSource, importNames, node) { + function checkRestrictedPathAndReport(importSource, importNames, node) { if (!restrictedPathMessages[importSource]) { return; } @@ -134,30 +134,30 @@ module.exports = { const customMessage = restrictedPathMessages[importSource].message; const restrictedImportNames = restrictedPathMessages[importSource].importNames; - if (importNames.has("*")) { - const data = importNames.get("*"); - - context.report({ - node, - messageId: customMessage ? "everythingWithCustomMessage" : "everything", - loc: data.loc, - data: { - importSource, - importNames: restrictedImportNames, - customMessage - } - }); - } - if (restrictedImportNames) { + if (importNames.has("*")) { + const specifierData = importNames.get("*"); + + context.report({ + node, + messageId: customMessage ? "everythingWithCustomMessage" : "everything", + loc: specifierData.loc, + data: { + importSource, + importNames: restrictedImportNames, + customMessage + } + }); + } + restrictedImportNames.forEach(importName => { if (importNames.has(importName)) { - const data = importNames.get(importName); + const specifierData = importNames.get(importName); context.report({ node, messageId: customMessage ? "importNameWithCustomMessage" : "importName", - loc: data.loc, + loc: specifierData.loc, data: { importSource, customMessage, @@ -229,7 +229,7 @@ module.exports = { return map; }, new Map()) : new Map(); - reportPath(importSource, importNames, node); + checkRestrictedPathAndReport(importSource, importNames, node); if (isRestrictedPattern(importSource)) { reportPathForPatterns(node); diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 7e0c288f73a..2ee9cf226c5 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -639,6 +639,16 @@ ruleTester.run("no-restricted-imports", rule, { column: 13, endColumn: 21 }] + }, { + code: "import * as bar from 'foo';", + options: ["foo"], + errors: [{ + message: "'foo' import is restricted from being used.", + type: "ImportDeclaration", + line: 1, + column: 1, + endColumn: 28 + }] } ] }); From 1b720c45830dd1d7d29671d8199be3aeb5ca8c0d Mon Sep 17 00:00:00 2001 From: fanich37 Date: Mon, 6 Jan 2020 18:32:35 +0300 Subject: [PATCH 07/10] Fix typo, fix specifier clause, fix rebase issue --- lib/rules/no-restricted-imports.js | 11 ++++++----- tests/lib/rules/no-restricted-imports.js | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index e28ee53ae22..fb6aa6894f7 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -121,13 +121,13 @@ module.exports = { /** * Report a restricted path. * @param {string} importSource path of the import - * @param {Map} importNames Map of import names that are being imported * @param {node} node representing the restricted path reference * @returns {void} * @private */ function checkRestrictedPathAndReport(importSource, importNames, node) { - if (!restrictedPathMessages[importSource]) { + if (!Object.prototype.hasOwnProperty.call(restrictedPathMessages, importSource)) { return; } @@ -215,15 +215,16 @@ module.exports = { function checkNode(node) { const importSource = node.source.value.trim(); const importNames = node.specifiers ? node.specifiers.reduce((map, specifier) => { - const specifierKey = specifier.imported ? "imported" : "local"; const specifierData = { loc: specifier.loc }; if (specifier.type === "ImportDefaultSpecifier") { map.set("default", specifierData); } else if (specifier.type === "ImportNamespaceSpecifier") { map.set("*", specifierData); - } else { - map.set(specifier[specifierKey].name, specifierData); + } else if (specifier.imported) { + map.set(specifier.imported.name, specifierData); + } else if (specifier.local) { + map.set(specifier.local.name, specifierData); } return map; diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 2ee9cf226c5..8a335b9b5f2 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -20,6 +20,7 @@ const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6, sourceType: ruleTester.run("no-restricted-imports", rule, { valid: [ + "import os from \"os\";", { code: "import os from \"os\";", options: ["osx"] }, { code: "import fs from \"fs\";", options: ["crypto"] }, { code: "import path from \"path\";", options: ["crypto", "stream", "os"] }, From 56cc6b68679a83b2f2162fb3c424e5ed3c896cee Mon Sep 17 00:00:00 2001 From: fanich37 Date: Sat, 11 Jan 2020 17:40:47 +0300 Subject: [PATCH 08/10] Process importNames with the same name --- lib/rules/no-restricted-imports.js | 47 ++++++++++++------ tests/lib/rules/no-restricted-imports.js | 63 ++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index fb6aa6894f7..133174837cf 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -152,17 +152,22 @@ module.exports = { restrictedImportNames.forEach(importName => { if (importNames.has(importName)) { - const specifierData = importNames.get(importName); - - context.report({ - node, - messageId: customMessage ? "importNameWithCustomMessage" : "importName", - loc: specifierData.loc, - data: { - importSource, - customMessage, - importName - } + const specifiers = importNames.get(importName); + + if (importSource === "moda") { + console.log("+++++", specifiers); // eslint-disable-line no-console + } + specifiers.forEach(specifier => { + context.report({ + node, + messageId: customMessage ? "importNameWithCustomMessage" : "importName", + loc: specifier.loc, + data: { + importSource, + customMessage, + importName + } + }); }); } }); @@ -218,13 +223,27 @@ module.exports = { const specifierData = { loc: specifier.loc }; if (specifier.type === "ImportDefaultSpecifier") { - map.set("default", specifierData); + map.set("default", [specifierData]); } else if (specifier.type === "ImportNamespaceSpecifier") { map.set("*", specifierData); } else if (specifier.imported) { - map.set(specifier.imported.name, specifierData); + if (map.has(specifier.imported.name)) { + const specifiers = map.get(specifier.imported.name); + + specifiers.push(specifierData); + map.set(specifier.imported.name, specifiers); + } else { + map.set(specifier.imported.name, [specifierData]); + } } else if (specifier.local) { - map.set(specifier.local.name, specifierData); + if (map.has(specifier.local.name)) { + const specifiers = map.get(specifier.local.name); + + specifiers.push(specifierData); + map.set(specifier.local.name, specifiers); + } else { + map.set(specifier.local.name, [specifierData]); + } } return map; diff --git a/tests/lib/rules/no-restricted-imports.js b/tests/lib/rules/no-restricted-imports.js index 8a335b9b5f2..958395f453a 100644 --- a/tests/lib/rules/no-restricted-imports.js +++ b/tests/lib/rules/no-restricted-imports.js @@ -650,6 +650,69 @@ ruleTester.run("no-restricted-imports", rule, { column: 1, endColumn: 28 }] + }, { + code: "import { a, a as b } from 'mod';", + options: [{ + paths: [{ + name: "mod", + importNames: ["a"] + }] + }], + errors: [{ + message: "'a' import from 'mod' is restricted.", + type: "ImportDeclaration", + line: 1, + column: 10, + endColumn: 11 + }, { + message: "'a' import from 'mod' is restricted.", + type: "ImportDeclaration", + line: 1, + column: 13, + endColumn: 19 + }] + }, { + code: "export { x as y, x as z } from 'mod';", + options: [{ + paths: [{ + name: "mod", + importNames: ["x"] + }] + }], + errors: [{ + message: "'x' import from 'mod' is restricted.", + type: "ExportNamedDeclaration", + line: 1, + column: 10, + endColumn: 16 + }, { + message: "'x' import from 'mod' is restricted.", + type: "ExportNamedDeclaration", + line: 1, + column: 18, + endColumn: 24 + }] + }, { + code: "import foo, { default as bar } from 'mod';", + options: [{ + paths: [{ + name: "mod", + importNames: ["default"] + }] + }], + errors: [{ + message: "'default' import from 'mod' is restricted.", + type: "ImportDeclaration", + line: 1, + column: 8, + endColumn: 11 + }, { + message: "'default' import from 'mod' is restricted.", + type: "ImportDeclaration", + line: 1, + column: 15, + endColumn: 29 + }] } ] }); From 822059e8f3e28be77421b5a19414ca052788e588 Mon Sep 17 00:00:00 2001 From: fanich37 Date: Sun, 12 Jan 2020 14:25:41 +0300 Subject: [PATCH 09/10] Clean code in receiving specifier data, remove debug --- lib/rules/no-restricted-imports.js | 32 ++++++++++-------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index 133174837cf..b6d865d7116 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -136,7 +136,7 @@ module.exports = { if (restrictedImportNames) { if (importNames.has("*")) { - const specifierData = importNames.get("*"); + const specifierData = importNames.get("*")[0]; context.report({ node, @@ -154,9 +154,6 @@ module.exports = { if (importNames.has(importName)) { const specifiers = importNames.get(importName); - if (importSource === "moda") { - console.log("+++++", specifiers); // eslint-disable-line no-console - } specifiers.forEach(specifier => { context.report({ node, @@ -220,30 +217,23 @@ module.exports = { function checkNode(node) { const importSource = node.source.value.trim(); const importNames = node.specifiers ? node.specifiers.reduce((map, specifier) => { + let name; const specifierData = { loc: specifier.loc }; if (specifier.type === "ImportDefaultSpecifier") { - map.set("default", [specifierData]); + name = "default"; } else if (specifier.type === "ImportNamespaceSpecifier") { - map.set("*", specifierData); + name = "*"; } else if (specifier.imported) { - if (map.has(specifier.imported.name)) { - const specifiers = map.get(specifier.imported.name); - - specifiers.push(specifierData); - map.set(specifier.imported.name, specifiers); - } else { - map.set(specifier.imported.name, [specifierData]); - } + name = specifier.imported.name; } else if (specifier.local) { - if (map.has(specifier.local.name)) { - const specifiers = map.get(specifier.local.name); + name = specifier.local.name; + } - specifiers.push(specifierData); - map.set(specifier.local.name, specifiers); - } else { - map.set(specifier.local.name, [specifierData]); - } + if (map.has(name)) { + map.set(name, map.get(name).concat(specifierData)); + } else { + map.set(name, [specifierData]); } return map; From 69c519d79b019f0e529e14b195ce817fe249b564 Mon Sep 17 00:00:00 2001 From: fanich37 Date: Mon, 13 Jan 2020 11:02:08 +0300 Subject: [PATCH 10/10] Fix type def, add empty name check, replace concat with push --- lib/rules/no-restricted-imports.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/rules/no-restricted-imports.js b/lib/rules/no-restricted-imports.js index b6d865d7116..5514b979c84 100644 --- a/lib/rules/no-restricted-imports.js +++ b/lib/rules/no-restricted-imports.js @@ -121,7 +121,7 @@ module.exports = { /** * Report a restricted path. * @param {string} importSource path of the import - * @param {Map} importNames Map of import names that are being imported + * @param {Map} importNames Map of import names that are being imported * @param {node} node representing the restricted path reference * @returns {void} * @private @@ -230,10 +230,12 @@ module.exports = { name = specifier.local.name; } - if (map.has(name)) { - map.set(name, map.get(name).concat(specifierData)); - } else { - map.set(name, [specifierData]); + if (name) { + if (map.has(name)) { + map.get(name).push(specifierData); + } else { + map.set(name, [specifierData]); + } } return map;