Update: add fix for "no-confusing-arrow" rule

[tikotzky]

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[X] 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)
Added a fix function to no-confusing-arrow which will auto wrap an arrow function body if allowParens is set to true.

Is there anything you'd like reviewers to focus on?
I'm doing an early return from the fix if allowParens not enabled, can you please advise if it acceptable to return undefined from the fix function in such a case.
(my testing seems to indicate that its fine, just wanted to double check)

[mention-bot]

@tikotzky, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vitorbal, @lukekarrys and @mysticatea to be potential reviewers.

[eslintbot]

LGTM

[jsf-clabot]

All committers have signed the CLA.

[eslintbot]

LGTM

[platinumazure]

Hi @tikotzky, thanks for your PR!

Returning undefined from a fixer function is okay.

[tikotzky]

Thanks for the info.
I changed it so that its not actually returning undefined, rather its returning a falsy value.
I can update to explicitly return undefined if that is desired.

Also, I was not able to find how/where to add tests for the fixer functions. If you give a pointer i'd be glad to add some tests.

[platinumazure]

@tikotzky We usually just use return; in fixers, but I personally don't have a problem with anything more specific.

Thanks for the note about fixer tests. Here's what you need to know:

• Unit test files for rules are found in tests/lib/rules and are named the same as the rule.
• We provide a RuleTester class for testing rules on code snippets.
• You can test the output of a fixer by adding an output property to an invalid test object.
  • I'm now seeing this isn't in our documentation. That's not good! Sorry for the missing docs; in the meanwhile, you can use this example to get started for your use case.
  • You can probably just add output to the relevant invalid test cases. But feel free to create new test cases if you think they will help!

By the way, I wanted to mention that we have a process for accepting enhancements, so definitely feel free to wait until the issue is marked "accepted" if you want (to avoid possibly wasting your time if we don't accept the issue). That said, having an implementation and tests can also help persuade the team to accept an issue, so there's an incentive to finish up as well. Go figure! 😜

@eslint/eslint-team This seems like a pretty simple autofix enhancement request (only autofixing a very simple case under specific options). Anyone want to support this? I'll champion.

[not-an-aardvark]

It is documented here, but I think the documentation for RuleTester is split between "working with rules" and "working with plugins", which is probably confusing.

[eslintbot]

LGTM

[tikotzky]

thanks @platinumazure, I am aware of the process. Adding tests looks easy (and fun 😄) so I gave it a shot.
thank you both for the pointers!
I was able to add some tests let me know if there is anything else I can add/change to this PR.

[platinumazure]

Awesome work @tikotzky. Just one thing-- as of ESLint 3.17.0, you can now use output: null to indicate that a test case should not apply fixes. Personally I find that more readable than using the same output string.

Thanks for augmenting the test cases!

[eslintbot]

LGTM

[tikotzky]

yup I agree that output: null is nicer.
updated and pushed 😄

[platinumazure]

LGTM, thanks for the PR! Sorry for not reviewing this sooner. Hopefully we can merge this as soon as the PR is accepted.

[platinumazure]

Adding "do not merge" until this accepted. PR looks good from an implementation standpoint, though!

[platinumazure]

Now we have enough support to accept this, so I've labeled accordingly.

@eslint/eslint-team I'd love a second set of eyes on this just to be sure all looks well, can someone else review?

[gyandeeps]

Please update the docs for this rule for the fix functionality.

[not-an-aardvark]

@gyandeeps It's not necessary to manually add the header line anymore because it's autogenerated from metadata.

[platinumazure]

@gyandeeps @not-an-aardvark Do we still want a note about the limited nature of the auto-fix somewhere?

[gyandeeps]

Opps you right @not-an-aardvark . But i think its good to have a note for its limited nature.

[kaicataldo]

LGTM. Thanks for contributing to ESLint!