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.
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
Fix: false positive new with member in no-extra-parens (fixes #12740) #13375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be a new false negative (one pair of parens is unnecessary):
new ((a()).b).c;
If it isn't easy to fix this, I'd be personally fine with some false negatives in these edge cases, those chains with new
are really complicated.
# Conflicts: # lib/rules/no-extra-parens.js # tests/lib/rules/no-extra-parens.js
lib/rules/no-extra-parens.js
Outdated
function isMemberExpInNewCallee(node) { | ||
if (node.type === "MemberExpression") { | ||
return node.parent.type === "NewExpression" && node.parent.callee === node | ||
? true | ||
: isMemberExpInNewCallee(node.parent); | ||
} | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should go up the tree only if it's object
child of another MemberExpression
, otherwise this would be a false negative:
"new a[(b()).c]"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mdjermanovic
Thanks! 👍 I added a checking about that and test case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Fixes #12740
I changed it to allow one parens if there is a "CallExpression" node in "MemberExpression" and when it used as a "NewExpression"'s callee.
Is there anything you'd like reviewers to focus on?