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

Fix: id-blacklist should ignore ObjectPatterns (fixes #12787) #12792

Merged
merged 6 commits into from Jan 17, 2020
Merged

Fix: id-blacklist should ignore ObjectPatterns (fixes #12787) #12792

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ec3983c
Fix: Ignore destructured object property reassignments. (fixes #12787)
jpramassini Jan 15, 2020
c25321a
Updated to ignore blacklisted id's in ObjectPattern nodes.
jpramassini Jan 16, 2020
8e340c3
Remove console.log.
jpramassini Jan 16, 2020
0e7b9a7
Added tests for nested destructuring and destructuring function param…
jpramassini Jan 16, 2020
871fc8c
Fix typo.
jpramassini Jan 16, 2020
bccd6eb
Undo accidentally auto-fixed comment indent.
jpramassini Jan 16, 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
17 changes: 11 additions & 6 deletions lib/rules/id-blacklist.js
View file
Open in desktop
Expand Up @@ -61,9 +61,12 @@ module.exports = {
* @returns {boolean} whether an error should be reported or not
*/
function shouldReport(effectiveParent, name) {
return effectiveParent.type !== "CallExpression" &&
return (
effectiveParent.type !== "CallExpression" &&
effectiveParent.type !== "NewExpression" &&
isInvalid(name);
effectiveParent.parent.type !== "ObjectPattern" &&
isInvalid(name)
);
}

/**
Expand All @@ -85,6 +88,8 @@ module.exports = {
return {

Identifier(node) {

// if (node.name === 'food') console.log(node)
kaicataldo marked this conversation as resolved.
Show resolved Hide resolved
const name = node.name,
effectiveParent = (node.parent.type === "MemberExpression") ? node.parent.parent : node.parent;

Expand All @@ -101,21 +106,21 @@ module.exports = {
// Report AssignmentExpressions only if they are the left side of the assignment
} else if (effectiveParent.type === "AssignmentExpression" &&
(effectiveParent.right.type !== "MemberExpression" ||
effectiveParent.left.type === "MemberExpression" &&
effectiveParent.left.property.name === node.name)) {
effectiveParent.left.type === "MemberExpression" &&
effectiveParent.left.property.name === node.name)) {
if (isInvalid(name)) {
report(node);
}
}

// Properties have their own rules
// Properties have their own rules
kaicataldo marked this conversation as resolved.
Show resolved Hide resolved
} else if (node.parent.type === "Property") {

if (shouldReport(effectiveParent, name)) {
report(node);
}

// Report anything that is a match and not a CallExpression
// Report anything that is a match and not a CallExpression
Copy link
Member

Choose a reason for hiding this comment

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

Mind undoing this? This is referring to the following block and I think it's clearer to have it indented as it was.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just pushed a commit that should fix this, the commit has these lined up properly but for some reason the diff doesn't seem to have updated.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I see that. Strange :/ Seems like something on GitHub's end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like it. This is the first time I've had this happen. 🤷‍♂

} else if (shouldReport(effectiveParent, name)) {
report(node);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/lib/rules/id-blacklist.js
View file
Open in desktop
Expand Up @@ -65,6 +65,11 @@ ruleTester.run("id-blacklist", rule, {
code: "var obj = { key: foo.bar };",
options: ["f", "fo", "fooo", "b", "ba", "barr", "bazz", "bingg"]
},
{
code: "const {foo: bar} = baz",
kaicataldo marked this conversation as resolved.
Show resolved Hide resolved
options: ["foo, bar"],
parserOptions: { ecmaVersion: 6 }
},
{
code: "var arr = [foo.bar];",
options: ["f", "fo", "fooo", "b", "ba", "barr", "bazz", "bingg"]
Expand Down