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 8 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
197 changes: 90 additions & 107 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 @@ -95,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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return {};
}

const restrictedPathMessages = restrictedPaths.reduce((memo, importSource) => {
if (typeof importSource === "string") {
memo[importSource] = { message: null };
Expand All @@ -107,40 +116,71 @@ 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 {Set.<string>} importNames Set 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
* @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();
const customMessage = restrictedPathMessages[importSource] && restrictedPathMessages[importSource].message;
function checkRestrictedPathAndReport(importSource, importNames, node) {
if (!Object.prototype.hasOwnProperty.call(restrictedPathMessages, importSource)) {
return;
}

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

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 specifiers = importNames.get(importName);

if (importSource === "moda") {
console.log("+++++", specifiers); // eslint-disable-line no-console
}
fanich37 marked this conversation as resolved.
Show resolved Hide resolved
specifiers.forEach(specifier => {
context.report({
node,
messageId: customMessage ? "importNameWithCustomMessage" : "importName",
loc: specifier.loc,
data: {
importSource,
customMessage,
importName
}
});
});
}
});
} else {
context.report({
node,
messageId: customMessage ? "pathWithCustomMessage" : "path",
data: {
importSource,
customMessage
}
});
}
}

/**
Expand All @@ -161,75 +201,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 {Set.<string>} importNames Set of import names that are being imported
* @returns {boolean} whether the path is restricted
* @private
*/
function isRestrictedForEverythingImported(importSource, importNames) {
return Object.prototype.hasOwnProperty.call(restrictedPathMessages, importSource) &&
restrictedPathMessages[importSource].importNames &&
isEverythingImported(importNames);
}

/**
* 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 {string[]} restrictedImportNames array of import names that are restricted for this import
* @returns {boolean} whether the objectName is restricted
* @private
*/
function isRestrictedObject(importNames, restrictedImportNames) {
return restrictedImportNames.some(restrictedObjectName => (
importNames.has(restrictedObjectName)
));
}

/**
* 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
* @returns {boolean} whether the variable is a restricted path or not
* @private
*/
function isRestrictedPath(importSource, importNames) {
let isRestricted = false;

if (Object.prototype.hasOwnProperty.call(restrictedPathMessages, importSource)) {
if (restrictedPathMessages[importSource].importNames) {
isRestricted = isRestrictedObject(importNames, restrictedPathMessages[importSource].importNames);
} else {
isRestricted = true;
}
}

return isRestricted;
}

/**
* Check if the given importSource is restricted by a pattern.
* @param {string} importSource path of the import
Expand All @@ -248,26 +219,38 @@ 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 specifierData = { loc: specifier.loc };

if (specifier.type === "ImportDefaultSpecifier") {
set.add("default");
map.set("default", [specifierData]);
} else if (specifier.type === "ImportNamespaceSpecifier") {
set.add("*");
map.set("*", specifierData);
} else if (specifier.imported) {
set.add(specifier.imported.name);
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) {
set.add(specifier.local.name);
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]);
}
}
fanich37 marked this conversation as resolved.
Show resolved Hide resolved
return set;
}, new Set()) : new Set();

if (isRestrictedForEverythingImported(importSource, importNames)) {
reportPathForEverythingImported(importSource, node);
}
return map;
}, new Map()) : new Map();

checkRestrictedPathAndReport(importSource, importNames, node);

if (isRestrictedPath(importSource, importNames)) {
reportPath(node);
}
if (isRestrictedPattern(importSource)) {
reportPathForPatterns(node);
}
Expand Down