-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add prefer-reflect-apply
rule
#239
Add prefer-reflect-apply
rule
#239
Conversation
1466f68
to
2d7996a
Compare
👍 This looks good so far, except, if we consider discussion in #142 settled, it should also handle |
Something's wrong with p-queue in the integration test, including on master. |
d7a84ea
to
c7a062e
Compare
rules/prefer-reflect-apply.js
Outdated
if (fix) { | ||
context.report({ | ||
node, | ||
message: 'Prefer Reflect.apply over Function.prototype.apply.', |
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.
message: 'Prefer Reflect.apply over Function.prototype.apply.', | |
message: 'Prefer `Reflect.apply()` over `Function#apply()`.', |
test/prefer-reflect-apply.js
Outdated
invalid: [ | ||
{ | ||
code: 'foo.apply(null, [42]);', | ||
errors: [error], |
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.
errors
should be below output
.
test/prefer-reflect-apply.js
Outdated
invalid: [ | ||
{ | ||
code: 'foo.apply(null, [42]);', | ||
errors: [error], |
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.
errors: [error], | |
errors, |
test/prefer-reflect-apply.js
Outdated
const error = { | ||
ruleId: 'prefer-reflect-apply', | ||
message: 'Prefer Reflect.apply over Function.prototype.apply.' | ||
}; |
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.
Can just be:
const errors = [
{
ruleId: 'prefer-reflect-apply'
}
];
rules/prefer-reflect-apply.js
Outdated
const fixFunctionPrototypeCall = (node, sourceCode) => { | ||
if ( | ||
astUtils.getPropertyName(node.callee) === 'call' && | ||
sourceCode.getText(node.callee.object) === 'Function.prototype.apply' && |
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 it would be better to check the method chain in the actual AST, than comparing string representation of the resulting code.
@alexzherdev Ping :) |
5e9d1b2
to
82656a0
Compare
@sindresorhus updated! |
🙌 Thanks for working on this. |
Fixes #142