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.

Sign up for GitHub

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

Jump to bottom

Update: fix no-restricted-imports importNames reporting (fixes #12282) #12711

Merged
merged 10 commits into from Jan 16, 2020
Merged

Update: fix no-restricted-imports importNames reporting (fixes #12282) #12711

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f597ed4
Process importNames
fanich37 Dec 20, 2019
9a09127
Update test cases
fanich37 Dec 25, 2019
41979a8
Fix rebase issue
fanich37 Dec 26, 2019
21fbe20
Update importNames logic
fanich37 Jan 2, 2020
47a9dbe
Remove useless funcs, update tests
fanich37 Jan 4, 2020
347d326
Fix naming, fix everything imported w/o importNames
fanich37 Jan 5, 2020
1b720c4
Fix typo, fix specifier clause, fix rebase issue
fanich37 Jan 6, 2020
56cc6b6
Process importNames with the same name
fanich37 Jan 11, 2020
822059e
Clean code in receiving specifier data, remove debug
fanich37 Jan 12, 2020
69c519d
Fix type def, add empty name check, replace concat with push
fanich37 Jan 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
80 changes: 54 additions & 26 deletions lib/rules/no-restricted-imports.js
View file
Open in desktop
Expand Up @@ -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: {
Expand Down Expand Up @@ -116,7 +120,7 @@ module.exports = {

/**
* Checks to see if "*" is being used to import everything.
* @param {Set.<string>} importNames Set of import names that are being imported
* @param {Map<string:Object>} importNames Map of import names that are being imported
* @returns {boolean} whether everything is imported or not
*/
function isEverythingImported(importNames) {
Expand All @@ -125,22 +129,43 @@ module.exports = {

/**
* Report a restricted path.
* @param {string} importSource path of the import
* @param {Map<string:Object} importNames Map of import names that are being imported
fanich37 marked this conversation as resolved.
Show resolved Hide resolved
* @param {node} node representing the restricted path reference
* @returns {void}
* @private
*/
function reportPath(node) {
const importSource = node.source.value.trim();
function reportPath(importSource, importNames, node) {
fanich37 marked this conversation as resolved.
Show resolved Hide resolved
const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message;

context.report({
node,
messageId: customMessage ? "pathWithCustomMessage" : "path",
data: {
importSource,
customMessage
}
});
const restrictedImportNames = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].importNames;

if (restrictedImportNames) {
restrictedImportNames.forEach(importName => {
if (importNames.has(importName)) {
const data = importNames.get(importName);

context.report({
node,
messageId: customMessage ? "importNameWithCustomMessage" : "importName",
loc: data.loc,
data: {
importSource,
customMessage,
importName
}
});
}
});
} else {
context.report({
node,
messageId: customMessage ? "pathWithCustomMessage" : "path",
data: {
importSource,
customMessage
}
});
}
}

/**
Expand Down Expand Up @@ -186,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.<string>} importNames Set of import names that are being imported
* @param {Map<string:Object} importNames Map of import names that are being imported
* @returns {boolean} whether the path is restricted
* @private
*/
Expand All @@ -198,7 +223,7 @@ module.exports = {

/**
* Check if the given importNames are restricted given a list of restrictedImportNames.
* @param {Set.<string>} importNames Set of import names that are being imported
* @param {Map<string:Object} importNames Map of import names that are being imported
* @param {string[]} restrictedImportNames array of import names that are restricted for this import
* @returns {boolean} whether the objectName is restricted
* @private
Expand All @@ -212,7 +237,7 @@ module.exports = {
/**
* Check if the given importSource is a restricted path.
* @param {string} importSource path of the import
* @param {Set.<string>} importNames Set of import names that are being imported
* @param {Map<string:Object} importNames Map of import names that are being imported
* @returns {boolean} whether the variable is a restricted path or not
* @private
*/
Expand Down Expand Up @@ -248,26 +273,29 @@ module.exports = {
*/
function checkNode(node) {
const importSource = node.source.value.trim();
const importNames = node.specifiers ? node.specifiers.reduce((set, specifier) => {
const importNames = node.specifiers ? node.specifiers.reduce((map, specifier) => {
const specifierKey = specifier.imported ? "imported" : "local";
const specifierData = { loc: specifier.loc.start };
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved

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);
fanich37 marked this conversation as resolved.
Show resolved Hide resolved
}
return set;
}, new Set()) : new Set();

return map;
}, new Map()) : new Map();

if (isRestrictedForEverythingImported(importSource, importNames)) {
reportPathForEverythingImported(importSource, node);
}

if (isRestrictedPath(importSource, importNames)) {
reportPath(node);
reportPath(importSource, importNames, node);
}

if (isRestrictedPattern(importSource)) {
reportPathForPatterns(node);
}
Expand Down
113 changes: 102 additions & 11 deletions tests/lib/rules/no-restricted-imports.js
View file
Open in desktop
Expand Up @@ -20,7 +20,6 @@ const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6, sourceType:

ruleTester.run("no-restricted-imports", rule, {
valid: [
"import os from \"os\";",
fanich37 marked this conversation as resolved.
Show resolved Hide resolved
{ code: "import os from \"os\";", options: ["osx"] },
{ code: "import fs from \"fs\";", options: ["crypto"] },
{ code: "import path from \"path\";", options: ["crypto", "stream", "os"] },
Expand Down Expand Up @@ -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: [{
Expand Down Expand Up @@ -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: [{
Expand Down Expand Up @@ -259,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"
}]
},
Expand Down Expand Up @@ -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"
}]
},
Expand All @@ -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"
}]
},
Expand All @@ -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"
}]
},
Expand All @@ -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"
}]
},
Expand All @@ -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"
}]
},
Expand All @@ -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"
}]
},
Expand All @@ -371,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"
}]
},
Expand All @@ -385,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"
}]
},
Expand Down Expand Up @@ -416,6 +419,94 @@ 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"
}]
},
{
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.",
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
type: "ImportDeclaration"
}, {
message: "'default' import from 'mod' is restricted.",
type: "ImportDeclaration"
}]
}
]
});