[linter] `unnecessary_else_block` suggestion

[FMorschel]

Inspired by #60008.

If you have the following code:

void f(bool c) {
  if (c) {
    // do something
    return;
  } else {
    // do something else
  }
}

The above // do something else does not need to be inside an else { ... } block.

I'm suggesting a lint that would detect this (leaving the function on earlier branches) and suggest removing the block since that could help with indentation for big lines that come after this point. Something like unnecessary_else_block.

// CC @pq

[pq]

Cool! While I prefer this style in principle myself, I'd be curious to know if there's a general agreement (and if this style is preferred enough to recommend generally).

/cc @lrhn @munificent @natebosch @goderbauer

[lrhn]

Which style I use depends on how I think of the problem.
I use the no-else for early bailout or default, but could choose to use an else branch if the two branches are more equal alternatives.

Not something I would consistently give a recommendation for in either direction.

[FMorschel]

I'd actually be interested in knowing whether this makes any difference in performance too, or only for indentation/meaning purposes. I'm not sure how to test that, if anyone wants to do that for me I'd appreciate it!

[lrhn]

If it makes any difference for performance, you have my permission to mock the compiler writers for not doing trivial optimization.

It's very clear that the two styles have equivalent observable behavior, so if they don't generate the exact same code, it should be because neither code is inherently superior. Otherwise the inferior code can be optimized.

[munificent]

For years, I've considered whether "Effective Dart" should say anything about this. I've mostly come to the conclusion that it's best to not be opinionated and let users decide. In my own code, I find myself preferring either style depending on the context. If I have code like:

f() {
  if (c1) {
    return ...
  } else if (c2) {
    return ...
  } else if (c3) {
    return ...
  } else {
    return ...
  }
}

Then I prefer the final clause to be an else clause so that it's symmetric with the others and it's clearer that every case is being covered.

On the other hand:

f() {
  if (c1) return ...

  doSomeStuff();

  if (c2) {
    return ...
  }

  return ...
}

If there are other early returns in the function, then I'm inclined to not use an else clause at the end and instead rely on the early return of the preceding if.

So I don't think either style is clearly better in all contexts.

[natebosch]

My initial reaction is that I don't think it makes sense to mandate either style in particular because either may be more readable. for a given block of code.

I'm a little curious whether on the balance more authors would be helped by avoiding the extra indentation than the cases where authors are annoyed because they prefer the else block. I don't think I can make a strong argument that it should be in the recommended set though.

[danaoredson-wf]

Original request seems to be just to implement the rule, not necessarily recommend it. It's a style preference, and would be helpful if those who do prefer that style could have a rule to enforce it.

[FMorschel]

@danaoredson-wf I see what you are trying to say here, just note that there is still no set of criteria on what the team will implement on the SDK or what should be provided by the community (see #60461 (comment)).

So currently, everything existing has been accepted by the team because they are willing to maintain, but that, of course, has a cost.