prefer-conditional-expression complains about using if - elseif - else

[Martin-Luft]

Bug Report

• TSLint version: 5.4.3
• TypeScript version: 2.3.4
• Running TSLint via: CLI

TypeScript code being linted

const currentSelectedTimeInMillis: number = this.truncateToTimeStepResolution(this.selectedTimeInMillis);

if (currentSelectedTimeInMillis < this.minTimeInMillis) {
  this.selectedTimeInMillis = this.minTimeInMillis;
} else if (currentSelectedTimeInMillis > this.maxTimeInMillis) {
  this.selectedTimeInMillis = this.maxTimeInMillis;
} else {
  this.selectedTimeInMillis = currentSelectedTimeInMillis;
}

with tslint.json configuration:

  "prefer-conditional-expression": true

Actual behavior

ERROR: Use a conditional expression instead of assigning to 'this.selectedTimeInMillis' in multiple places.

Expected behavior

if - else - code can be replaced by a readable conditional expression but if - elseif - ... - else code cannot be replaced by a readable conditional expression.

So the prefer-conditional-expression rule should only complain about if - else code.

[adidahiya]

yeah, after turning this on in another code base, I mostly agree; I think the rule should be more lenient and only enforce single if-else conditions by default while having an option to opt-in to more strict checking.

you can write your code like this:

this.selectedTimeInMillis = (currentSelectedTimeInMillis < this.minTimeInMillis)
  ? this.minTimeInMillis
  : (currentSelectedTimeInMillis > this.maxTimeInMillis)
    ? this.maxTimeInMillis
    : currentSelectedTimeInMillis;

but I can see how some developers might reject that style. accepting PRs

[styu]

+1 this check is pretty annoying for if/else if/else cases

[SgtPooki]

Ternary ifs are horrible and this rule should default to false. This should never be preferred. This is anecdotal but misused ternary operators have caused more bugs than the use of any other operator i've seen.

A single ternary frequently ends up being a nested ternary and nested ternary operators are the problem.

Saved_LOC > code_readability // Expression is always false.

[adidahiya]

Well, we fixed the rule to not warn on nested ternary expressions. If you want to ban all ternary expressions, you're better off creating a custom rule and disabling this one in your tslint config.

[glen-84]

I don't really think that this:

const returnPath = error.httpMethod === "GET"
    ? encodeURIComponent(window.location.pathname + window.location.search)
    : "/";

... is much better than this:

let returnPath;
if (error.httpMethod === "GET") {
    returnPath = encodeURIComponent(window.location.pathname + window.location.search);
} else {
    returnPath = "/";
}

(probably a bad example because the "/" could arguably be moved up to the let, but you get the point)

If the rule was smart enough to determine whether the ternary would fit on one line (based on max-line-length), it might be more useful.

[Mattwmaster58]

IMO it should only warn/error when there's a simple if/else, same as @styu, @adidahiya and @Martin-Wegner . Anything more complicated should not trigger the warning. Can we get this rolling?