Tslint: Improve rule curly with one-line if without braces

[tkrotoff]

Having one-line if and else without braces is common:

// One-line without braces
if (foo) doSomething();
else doSomethingElse();

// Multi-line with braces
if (foo) {
  doSomething1();
  doSomething2();
} else {
  doSomethingElse1();
  doSomethingElse2();
}

ESLint allows this with "curly": [2, "multi-line"]. Would be nice for TSLint to support it.

[NeverwinterMoon]

This would be nice to have. We've been using one-liners without curly braces for years across multiple projects. Now, after moving to TS and enabling TSLint, this aforementioned problem arose.

[devrelm]

I'm starting to implement this right now. I'd like to clear up the terminology a little bit and suggest these options: same-line-multi-line, same-line-single-line, next-line-multi-line, and next-line-single-line. An additional all option will be an alias for all four, which should mirror existing behavior. The idea behind this is that the rule is only enforced on a single statement coming after a control-flow statement, so the rule's options are coined to describe the statement itself, where that statement might reside and how many lines the statement covers.

Note that the rule doesn't specify where it can reside or how many lines a statement is allowed use — that would be handled by another rule. Instead, it simply dictates whether or not it should have curly braces in some particular case. I got as granular as possible so that later we can add something like a same-line option that is an alias for both same-line-single-line and same-line-multi-line.

In Scope:

• { "curly": true } - existing behavior
• { "curly": [true, "all"] } - existing behavior

• { "curly": [true, "same-line-single-line"] } - only single-line if-statement expressions get brackets:

  // Good
  if (foo) { doSomething(); }
  if (foo) doSomething(
    someParam,
    someOtherParam);
  if (foo)
    doSomething();
  if (foo)
    doSomething(
        someParam,
        someOtherParam);
  
  // Bad
  if (foo) doSomething();
  if (foo) { doSomething(
    someParam,
    someOtherParam);
  }
  if (foo) {
    doSomething();
  }
  if (foo) {
    doSomething(
        someParam,
        someOtherParam);
  }
  

• { "curly": [true, "next-line-single-line"] }

  // Good
  if (foo) {
    doSomething();
  }
  if (foo)
    doSomething(
        someParam,
        someOtherParam);
  if (foo) doSomething();
  if (foo) doSomething(
    someParam,
    someOtherParam);
  
  // Bad
  if (foo)
    doSomething();
  if (foo) {
    doSomething(
        someParam,
        someOtherParam);
  }
  if (foo) { doSomething(); }
  if (foo) { doSomething(
    someParam,
    someOtherParam);
  }
  

• { "curly": [true, "same-line-multi-line"] }

  // Good
  if (foo) { doSomething(
    someParam,
    someOtherParam);
  }
  if (foo)
    doSomething(
        someParam,
        someOtherParam);
  if (foo)
    doSomething();
  if (foo) doSomething();
  
  // Bad
  if (foo) doSomething(
    someParam,
    someOtherParam);
  if (foo) {
    doSomething(
        someParam,
        someOtherParam);
  }
  if (foo) {
    doSomething();
  }
  if (foo) { doSomething(); }
  

• { "curly": [true, "next-line-multi-line"] }

  // Good
  if (foo) {
    doSomething(
        someParam,
        someOtherParam);
  }
  if (foo) doSomething(
    someParam,
    someOtherParam);
  if (foo) doSomething();
  if (foo)
    doSomething();
  
  // Bad
  if (foo)
    doSomething(
        someParam,
        someOtherParam);
  if (foo) { doSomething(
    someParam,
    someOtherParam);
  }
  if (foo) { doSomething(); }
  if (foo) {
    doSomething();
  }
  

Out of Scope

The following seem like thing that should be allowed or denied by multi, but should ultimately be put into their own rules or options:

// if/else on one line -- not related to curly braces
// this should be handled by some "no-single-line-if-else" rule or something
if (foo) doSomething(); else doSomethingElse();

// forcing single-statement if to one line -- not related to curly braces
if (foo)
    doSomething();
// vs
if (foo) doSomething();

// unsafe multi-line control-flow indentation
// this should be handled by a separate rule
if (foo) doSomething();
    doSomethingElse(); // this is dangerous and not part of the preceding if statement
if (foo)
    doSomething();
    doSomethingElse(); // this is dangerous and not part of the preceding if statement

[devrelm]

This is implemented with #1565, and is waiting for review.

[jefbarn]

I use single line 'if's everywhere and this fix is the ONLY thing keeping me from using tslint.
Can the PR be slimmed down to make progress?

[devrelm]

I'm working on a slimmed-down PR as we speak, but there were some changes made around auto-fixes that are taking time to merge in. Something's coming, though.