if statements: force brace or one-liner

[dcousens]

I find I have to double take when I look at this, to make sure I'm not mis-understanding what is actually part of the expressions that get executed.

The example is the following:

if (condition)
  expression
else
  more expressions
otherstuff

I think that is confusing and unnecessary, and IMHO it should only either be:

if (condition) expression
else more expressions

or

if (condition) {
  expression
} else {
  more expressions
}

edit: Infact, I only ever use the shorthand one-liner for early-exit return statements, but YMMV.

@feross thoughts?

[mafintosh]

👍

[feross]

I'm also 👍 but theres no ESLint or JSCS rule that captures this. The closest I could find is: http://eslint.org/docs/rules/curly.html but it's not quite what we want. Take a look for yourself.

[kytwb]

👍 for braces everywhere:

if (condition) {
  expression
} else {
  more expressions
}

[feross]

There is now an eslint rule that would enforce @dcousens's original request. curly: [2, "multi-line"], see: http://eslint.org/docs/rules/curly.html

If we add this rule to standard, the following patterns will be considered errors:

if (foo)
  doSomething()
else
  doSomethingElse()

while (foo
  && bar) baz()

if (foo) foo(
  bar,
  baz)

It will not error for these patterns:

if (foo) return; else doSomething()

if (foo) return
else if (bar) baz()
else doSomething()

do something()
while (foo)

if (foo) {
  return
}

if (foo) { return }

while (true) {
  doSomething()
  doSomethingElse()
}

This is definitely a breaking change (it causes many of the modules in test.js to fail), so we'd need to bump the major version and release this as 3.0.0.

[feross]

Is this worth doing? cc @mafintosh @othiym23

[dcousens]

I'm happy with the result of that rule. ACK from me.
On 27 Feb 2015 1:43 am, "Feross Aboukhadijeh" notifications@github.com
wrote:

There is now an eslint rule that would enforce @dcousens
https://github.com/dcousens's original request. curly: [2, "multi-line"],
see: http://eslint.org/docs/rules/curly.html

If we add this rule to standard, the following patterns will be
considered errors:

if (foo)
doSomething()else
doSomethingElse()
while (foo
&& bar) baz()
if (foo) foo(
bar,
baz)

It will not error for these patterns:

if (foo) return; else doSomething()
if (foo) returnelse if (bar) baz()else doSomething()
do something()while (foo)
if (foo) {
return
}
if (foo) { return }
while (true) {
doSomething()
doSomethingElse()
}

This is definitely a breaking change (it causes many of the modules in
test.js to fail), so we'd need to bump the major version and release this
as 3.0.0.

—
Reply to this email directly or view it on GitHub
#16 (comment).

[othiym23]

@feross I'm in favor, but (to set error bars) I feel like I'm unusually particular about this rule.

[yoshuawuyts]

fwiw, I'm also in favor of this.

[mafintosh]

👍 from me

[kytwb]

Isn't it too permissive?

[dcousens]

@kytwb what would you see changed?

[kytwb]

I'd reject those as well

if (foo) return; else doSomething()

if (foo) return
else if (bar) baz()
else doSomething()

do something()
while (foo)

if (foo) { return }

And force explicit blocks and braces while accepting same line chaining of else/else if/while with previous closing brace:

if (foo) {
  return; 
} else {
  doSomething()
}

if (foo) {
  return
} else if (bar) {
  baz()
} else {
  doSomething()
}

do {
something()
} while (foo)

if (foo) { 
  return 
}

[dcousens]

@kytwb I personally use if (condition) return as an early exit everywhere in my code, so removing that would be a huge deal breaker for me.

[feross]

@kytwb Most users of standard use if (condition) return liberally. It's not practical to forbid it – we have to be pragmatic. I think the succinctness is worth the small risk of errors in this case, since handling an err argument is so common in JavaScript/node.js code: if (err) return cb(err).

[kytwb]

Fair enough.

[feross]

This rule is added in 3.0.0-alpha. Works great! Please give it a try and update your modules to always use statement braces, except for single-liners.

npm install standard@3.0.0-alpha -g