Skip to content
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

False positive for `arrow-body-style` rule when used as a noop #4411

Closed
sindresorhus opened this Issue Nov 13, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@sindresorhus
Copy link
Contributor

sindresorhus commented Nov 13, 2015

~/dev
❯ eslint --version                                                                            
v1.9.0

~/dev
❯ echo 'stream.on('error', () => {});' | eslint --stdin --env=es6 --rule='arrow-body-style: [2, "as-needed"]'

<text>
  1:24  error  Unexpected empty block in arrow body  arrow-body-style

✖ 1 problem (1 error, 0 warnings)

Sometimes you have to pass a noop function. In this example the braces are required, but ESLint still complains, even though the setting is as-needed.

@alberto

This comment has been minimized.

Copy link
Member

alberto commented Nov 13, 2015

From the docs:

Additionally, this rule specifically warns against a possible developer error when the intention is to return an empty object literal but creates an empty block instead, returning undefined.

I think you'll have to use an inline comment to ignore that.

@sindresorhus

This comment has been minimized.

Copy link
Contributor Author

sindresorhus commented Nov 13, 2015

I don't understand why that's mixed into this rule. It should at least be an option.

I think empty functions are much more common than returning an empty object.

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Nov 13, 2015

I thought there are some notations for a noop arrow.

let noop = () => {};
let noop = () => undefined;
let noop = () => void 0;

And () => {} is confusable to beginners IMO.
So I'd like to warn it by default.

I vote for an option, [2, "as-needed", {"allowEmptyBlock": true}] ?

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 13, 2015

Yeah, I'm not sure why this check in this rule, either. The rule is about style, I think we should remove that extra check.

alberto added a commit that referenced this issue Nov 14, 2015

@alberto alberto closed this in 013664f Nov 14, 2015

ilyavolodin added a commit that referenced this issue Nov 14, 2015

Merge pull request #4420 from eslint/issue4411
Update: Allow empty arrow body (fixes #4411)

nzakas added a commit that referenced this issue Nov 14, 2015

nzakas added a commit that referenced this issue Nov 14, 2015

Merge pull request #4425 from eslint/revert-4420-issue4411
Revert "Update: Allow empty arrow body (fixes #4411)"

@alberto alberto reopened this Nov 14, 2015

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Nov 14, 2015

Hmm, as I mentioned above, there are several notations for a noop arrow, so I don't think the empty block in a arrow function's body is needed. So I think that to report the empty block by this rule is reasonable.

But I don't oppose strongly if separate that behavior to new rule such as no-arrow-empty-block.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 16, 2015

This rule is about style, which is just about using braces or not. For an empty body, you need the braces. We can argue that you can have other bodies for noop functions, but since this is just about style, you must have braces for a zero statement body.

alberto added a commit that referenced this issue Nov 16, 2015

@alberto alberto closed this in 269d2ed Dec 1, 2015

nzakas added a commit that referenced this issue Dec 1, 2015

Merge pull request #4453 from eslint/issue4411
Breaking: Allow empty arrow body (fixes #4411)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.