New rule: Add linter for single arg fat arrow functions

[KevinGrandon]

I think we should implement this rule in the following manner:

// Passes lint
var foo = bar => {
  window.alert(bar)
}
foo('hello')

// Fails lint
var foo = (bar) => {
  window.alert(bar)
}
foo('hello')

The reason being is that with only a single argument, the function is much easier to read without parens. Some folks who don't like this might argue about expansion and having a bigger diff when adding arguments down the road. Since we currently fail lint on trailing commas I think we should fail here on un-needed parens, it feels like it follows the same ideology as the trailing comma rule. (If we did enforce trailing commas, then I would probably enforce parens here as well).

I have a simple eslint plugin here that I'd be happy to include/relinquish: https://github.com/KevinGrandon/eslint-plugins/blob/master/lib/rules/arrow-function-parens.js

[xjamundx]

FYI there's http://eslint.org/docs/rules/arrow-parens

[yoshuawuyts]

Even though I also omit parens on single arguments I don't see a lot of benefit in enforcing it.

[feross]

Can you send a PR with this change? Also, please run the tests and report how many of the repos in the test suite start to fail with this change.

That should get the discussion started.

[KevinGrandon]

Even though I also omit parens on single arguments I don't see a lot of benefit in enforcing it.

I've seen a lot of bikeshedding around this issue in various projects which is why I think it would be good to enforce this one way or another.

[KevinGrandon]

Some of this is discussed over here: standard/eslint-config-standard#14

Arrow functions specifically are well enough supported/understood that I think it makes sense to move forward with this specific rule.

[KevinGrandon]

Can you send a PR with this change?

Submitted a PR in eslint-config-standard.

Also, please run the tests and report how many of the repos in the test suite start to fail with this change.

I'm happy to do this if needed, is there any documentation about how to run the tests?

[feross]

@KevinGrandon Just run npm test

[KevinGrandon]

Thanks. I assume that this should be run in eslint-config-standard? I ran with both "as-needed" (which I would like to implement), as well as "always". Results are as follows:

// When set to "as-needed"
# tests 124
# pass  124

// When set to "always" (opposite of desired implementation)
# tests 124
# pass  120
# fail  4

[KevinGrandon]

@feross - Does that test output look correct? ^

Any thoughts on implementing this rule since it seems most of the test repos follow it?

[jprichardson]

I'm ambivalent about this proposed rule. But it does feel a bit inconsistent... to codify it:

1. No parameters... parenthesis required. (syntax requirement AFAIK)
2. One parameter... no parenthesis required. (standard rule)
3. More than one parameter... parenthesis required. (syntax requirement AFAIK)

I agree that no parenthesis looks cleaner. If I'm a noob to JavaScript, the rule feels inconsistent. It almost seems as if we should require parenthesis in all cases if we're gonna make a rule.

To be clear, I support either. Maybe we defer until later? IDC.

[jprichardson]

I can say this much, I usually write without the parenthesis in the single parameter case. But scanning over code like this:

var foo = bar => {
  window.alert(bar)
}

I have to do a double take. Whereas code like this, feels easier to recognize:

var foo = (bar) => {
  window.alert(bar)
}

All very subjective though. I think @feross will just have to pick something :)

[rstacruz]

My personal preferences are in line with @jprichardson's.

[dcousens]

Agreed with @jprichardson. My habits are the same.

Can we enforce the following:

x => x
(x, y) => x + y
(x) => { return x }

That way, the braced scope enforces parenthesis, but the 1 liner can be either.

[julien-f]

I am not sure it should be enforced but if we do I think @dcousens' proposal makes sense.

[capaj]

@dcousens
I think we shouldn't oversimplify this-my personal preference would be the same as your, with one addition:

//also valid
promise.then(res => {
  debug(res)
  return res
})

[KevinGrandon]

Not sure I follow - you should pick one style with parens, or without. I've seen lots of bikeshedding about this issue in projects, and picking a style would fix that.

My preference is to go without parens for single argument functions as I believe that it follows the current style of this guide, and it's the style that the test repos use.

[capaj]

@KevinGrandon I think standard is the best readability focused guide. Let me reiterate:

• for single argument I would allow both styles unless there is another '=' sign on the same line, then parens should be included
• for more than one argument parens always

[dcousens]

@feross thoughts on:

x => x
(x) => { return x }
(x, y) => x + y
(x, y) => { return x + y }

The English of it is: no parenthesis for a 1 liner unless there is more than 1 argument. Parenthesis always if a braced expression exists.

Meaning, of all the possible combinations (not many), the only one that is rejected is:

x => {
  return x
}

[rstacruz]

Big 👍 on that. I'd prefer (x) => x but that's not a strong opinion for me.

[jprichardson]

I think that

let y = x => x + 1

vs

let y = (x) => x + 1

may be confusing.

But I commonly write

someMethod(param1, param2, x => {
  // do something
})

IDK, @feross, I think it's just one of those cases where you're gonna have to just pick something :)

[dcousens]

In light of a 'just pick something', I'd prefer @rstacruz's, that is, always (x).

[jprichardson]

In light of a 'just pick something', I'd prefer @rstacruz's, that is, always (x).

If we go this route (enforcing a rule), this would be my preference as well - for the sake of consistency.

[Flet]

It may be best to try to use built-in eslint rules if possible.

Does this rule cover @rstacruz's preference?
http://eslint.org/docs/rules/arrow-parens.html

Is there overlap with your rule @KevinGrandon?

[rstacruz]

looks like it does.

[joshmanders]

I have personally taken to always (x) even with one liners, because of consistency with syntax rules, so I agree with adding it like @dcousens says.

[feross]

Using the new improved test suite that tests the top ~400 packages that use standard, based on number of npm dependents, here are the results:

as-needed:

# tests 427
# pass  415
# fail  12

3%

always:

# tests 427
# pass  405
# fail  22

5%

[feross]

@jprichardson's point about consistency in this comment is really good. The overall sentiment of contributors is leaning towards always.

@KevinGrandon Your original argument about consistency with the dangling comma rule was good, too. I think consistency helps readability and that was also the purpose of the dangling comma rule.

I don't think this issue is super important, and consistency is preferable. Let's enable this in v6:

    "arrow-parens": [2, "always"]

[dcousens]

👍 awesome, I hate bike-shedding on this in my mind every time I write a 1-arg lambda.

[feross]

Will be released in standard 6

[machadogj]

I may be late to the discussion, but while I think that the following is somewhat inconsistent:

() => ... //parenths
a => ... //no-parenths
(a, b) => ... //parenths again

I think this could be avoided with:

_ => .... //something common in functional languages
a => ...
(a, b) => ...

In this way, 0 and 1 arg funcs could have no parentheses... forbidding () => could be a bit too much tough.

[ahmadnassri]

seeing those errors in my editor this morning in regards to a single arg arrow func ... I found my way here.

my initial reaction ... well, I didn't like it.

but then after some thought, I do agree, it will be more consistent this way.

I don't like extra typing when it can be avoided, but I also don't like one-off conventions such as the one @machadogj (not that it is invalid, I actually kinda like the idea of using _) but because those conventions are re-invented over and over again, and varying adoption spread ends up polluting the ecosystem without consistency, while defaulting to the language syntax itself always proves to be the best over the long haul.

[elyobo]

Should this be documented somewhere, e.g. in the rules?

[jescalan]

My only objection is to not being allowed to use _ => { return true }. For an anonymous function with no args, this is by far the cleanest way to write it. Is there any way to add an exception for this case, or does anyone else agree at all?

[mightyiam]

See standard/eslint-config-standard#54.

[mightyiam]

I've wrote some more thoughts regarding this in my PR, suggesting "arrow-parens": [2, "as-needed"]. If you care, please check it out.