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 3 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
57 changes: 44 additions & 13 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 @@ -125,22 +129,48 @@ module.exports = {

/**
* Report a restricted path.
* @param {string} importSource path of the import
* @param {Set.<string>} 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();
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 isRestrictedImportNames = restrictedPathMessages[importSource] &&
restrictedPathMessages[importSource].importNames &&
restrictedPathMessages[importSource].importNames.length;

if (importNames.has("*") || importNames.has("default") || !isRestrictedImportNames) {
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
context.report({
node,
messageId: customMessage ? "pathWithCustomMessage" : "path",
data: {
importSource,
customMessage
}
});
} else {
node.specifiers.forEach(specifier => {
const specifierKey = specifier.imported ? "imported" : "local";

if (
isRestrictedImportNames &&
restrictedPathMessages[importSource].importNames.includes(specifier[specifierKey].name)
) {
context.report({
node,
messageId: customMessage ? "importNameWithCustomMessage" : "importName",
loc: specifier[specifierKey].loc,
data: {
importSource,
customMessage,
importName: specifier[specifierKey].name
}
});
}
});
}
}

/**
Expand Down Expand Up @@ -266,8 +296,9 @@ module.exports = {
}

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

if (isRestrictedPattern(importSource)) {
reportPathForPatterns(node);
}
Expand Down
65 changes: 57 additions & 8 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 @@ -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 Down Expand Up @@ -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"
}]
}
]
});