proposal: `no-dupe-else-if`

[mr-pant]

Description

Disallow duplicate conditions in if-else-if chains

Details

Give a detailed description that should provide context and motivation. Ideally this could be used directly in the rule's documentation.

Kind

Guard against dead code.

Bad Examples

*if (isSomething(x)) { foo(); } else if (isSomething(x)) { bar(); }

if (a) { foo(); } else if (b) { bar(); } else if (c && d) { baz(); } else if (c && d) { quux(); } else { quuux(); }

if (n === 1) { foo(); } else if (n === 2) { bar(); } else if (n === 3) { baz(); } else if (n === 2) { quux(); } else if (n === 5) { quuux(); }*

Good Examples

*if (isSomething(x)) { foo(); } else if (isSomethingElse(x)) { bar(); }

if (a) { foo(); } else if (b) { bar(); } else if (c && d) { baz(); } else if (c && e) { quux(); } else { quuux(); }

if (n === 1) { foo(); } else if (n === 2) { bar(); } else if (n === 3) { baz(); } else if (n === 4) { quux(); } else if (n === 5) { quuux(); }*

Discussion

Two identical test conditions in the same chain are almost always a mistake in the code. Unless there are side effects in the expressions, a duplicate will evaluate to the same true or false value as the identical expression earlier in the chain, meaning that its branch can never execute. Reference - https://eslint.org/docs/latest/rules/no-dupe-else-if

Discussion checklist

• [x] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
• [x] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
• [x] If there's any prior art (e.g., in other linters), please add references here.
• [x] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
• [x] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

[pq]

This almost seems like an addition to dead-code analysis though I guess maybe not since technically the operators could be side-effecting?

@bwilkerson @scheglov

[scheglov]

This seems to be related to exhaustiveness analysis we have now for switch. Here the second case (true, true) is marked as already covered.

void f(bool a, bool b) {
  switch ((a, b)) {
    case (true, true):
      0;
    case (true, true):
      0;
    case _:
      -1;
  }
}

Of course in general case it is much harder. Probably could be done with "symbolic execution analysis" that Paul mentioned a couple weeks ago.

But it could be implemented relatively easily for the simplest case of identical (by tokens) conditions, when all reads are from local variables (or formal parameters).

It might be useful even with getters, but it would be up to the user to accept that the expectation of this lint rule is that all getters are "stable".

[bwilkerson]

While I agree that it would be nice to be able to catch bugs of this form, I have concerns with the use of heuristics for a lint like this. It feels very much like invariant_booleans in terms of the complexity required to get it right.

As Konstantin noted, we could potentially check if-elseif structures if the conditions are constants expressions when assuming that all local variables are constants (without requiring that they be const). But it isn't clear to me that this would catch enough cases to be useful.