Update: add fixer for prefer-arrow-callback (fixes #7002) #7004
Conversation
|
@not-an-aardvark, thanks for your PR! By analyzing the annotation information on this pull request, we identified @kaicataldo, @mysticatea and @vitorbal to be potential reviewers |
|
LGTM |
|
Marking "do not merge" since issue has not yet been accepted. |
| fix(fixer) { | ||
|
|
||
| // FIXME: Is there a better way to detect parser support for arrow functions? | ||
| if (!context.parserOptions || !context.parserOptions.ecmaVersion || context.parserOptions.ecmaVersion < 6) { |
There was a problem hiding this comment.
It's unnecessary, I think.
Developers always can disable this rule.
Other ES2015 rules (e.g. no-var, prefer-const) also don't check the language version.
There was a problem hiding this comment.
In that case, should I just use parserOptions: { ecmaVersion: 6 } for all the tests?
not-an-aardvark
force-pushed
the
issue7002
branch
from
75affc6
to
40a70b0
Compare
|
LGTM |
| * @param {ASTNode[]} paramsList The list of parameters for a function | ||
| * @returns {boolean} `true` if the list of parameters contains any duplicates | ||
| */ | ||
| function hasDuplicateParams(paramsList) { |
There was a problem hiding this comment.
This should probably be replaced with a function like the one described here.
The current implementation will work in all cases where the function body has valid syntax, but it will fail in certain cases with complex parameter lists due to the parser bug described here.
foo(function(a, a = 5) { }) // the parser incorrectly considers this to be valid.
foo((a, a = 5) => { }) // the parser correctly considers this to be invalid.There was a problem hiding this comment.
I think this implementation is no problem.
Acorn should be fixed about the duplication error of non-simple parameters.
There was a problem hiding this comment.
Fixed in Acorn: acornjs/acorn#457
not-an-aardvark
force-pushed
the
issue7002
branch
from
40a70b0
to
6151e9e
Compare
|
LGTM |
|
Issue is now accepted, so removing "do not merge". |
|
LGTM. |
|
LGTM. |
eslint-deprecated
bot
added
the
archived due to age
What issue does this pull request address?
#7002
What changes did you make? (Give an overview)
This allows errors reported by the
prefer-arrow-callbackrule to be autofixed, matching the behavior described in #7002.Is there anything you'd like reviewers to focus on?
I'm unsure about whether this check is correct. The intention is to detect whether the current parser configuration supports arrow functions, to avoid causing SyntaxErrors if ES6-mode is off. However, there might be a better/more complete way to do this.
#7002 has not been marked as accepted, so this PR should not be merged yet.(edit: #7002 is now marked as accepted.)