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

Proposal: No empty blocks #796

Open
tunnckoCore opened this issue Feb 21, 2017 · 9 comments

Comments

@tunnckoCore
Copy link

commented Feb 21, 2017

Digging more and more in ESLint rules, i found some good ones which enforces good style and more readable code. This issue is for no-empty rule - not fixable.

Suggested configuration.

{
  "no-empty": ["error", { "allowEmptyCatch": true }]
}

Never seen a point for doing such things. It is just waste of code and is messy, adding needless branches.

For what god reason this make sense

var foo = true // or false, no matter
if (foo) {}

I've seen such code here and there over the years, but mostly if i remember correctly it was because if-elseif combo like

let foo = true
if (foo) {
} else if (foo === 123) {
  // ok
}

But again, it not make any sense.

@feross feross added the enhancement label Feb 21, 2017

@feross

This comment has been minimized.

Copy link
Member

commented Feb 21, 2017

Thanks for the request. We need to evaluate the ecosystem impact of this.

@tunnckoCore

This comment has been minimized.

Copy link
Author

commented Feb 21, 2017

Yea, i'm curious too. But don't think it will be big, huh.. i hope, it would be bad signal for the js community.

@ghost

This comment has been minimized.

Copy link

commented Mar 22, 2017

I don't feel the value this adds outweighs the complexity it adds in coding standard. Let's please KISS this library and let ESLint serve the persnickety.

@feross

This comment has been minimized.

Copy link
Member

commented Apr 4, 2017

I really wanted to like this rule, and the ecosystem impact is only 3 repos! But all 3 repos are mine! These are all pretty low-level modules, so these snippets aren't necessarily the clearest. But it's not obvious how I would rewrite these snippets to remove the empty block.

webtorrent:

while (self._request(wire, piece, self._critical[piece] || hotswap)) {}

bittorrent-protocol:

while (this.read()) {} // consume and discard the rest of the stream data

ieee754:

for (; nBits > 0; e = (e * 256) + buffer[offset + i], i += d, nBits -= 8) {}

For that reason, I'm -1 on this. Unless other collaborators have thoughts they'd like to chime in...

@feross feross closed this Apr 4, 2017

@dcousens

This comment has been minimized.

Copy link
Member

commented Apr 5, 2017

If we could limit it to that idiom, that'd be fine.

Your for loop could easily be re-written [more clearly too]:

while (nBits > 0) {
  e = (e * 256) + buffer[offset + i]
  i += d
  nBits -= 8
}
@dcousens

This comment has been minimized.

Copy link
Member

commented Apr 5, 2017

Perhaps ask the eslint team for a special case to allow empty while blocks?

@feross

This comment has been minimized.

Copy link
Member

commented Apr 5, 2017

@dcousens Fair point about the for-loop!

Does someone want to volunteer to request this option from the ESLint team? If you do it, please post the link here so I can subscribe to it :)

@dcousens

This comment has been minimized.

Copy link
Member

commented Apr 5, 2017

@tunnckoCore

This comment has been minimized.

Copy link
Author

commented Apr 5, 2017

I don't have the time currently.

As we seen, from whole community only @feross is using such things hehe. As seen, there's always better approach instead of some tricky bytes - that's javascript: tons of available ways, one best! haha

@dcousens dcousens added the help wanted label Apr 5, 2017

@dcousens dcousens reopened this Apr 5, 2017

@dcousens dcousens added the blocked label Apr 5, 2017

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