Skip to content

proposal: no_else_after_return #59148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
5 tasks done
goderbauer opened this issue May 16, 2023 · 6 comments
Open
5 tasks done

proposal: no_else_after_return #59148

goderbauer opened this issue May 16, 2023 · 6 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@goderbauer
Copy link
Contributor

no_else_after_return

Description

Move the code of the else branch below the if block.

Details

When the if case always ends with a return, the else branch is not necessary and the code can instead be placed below the if block. This decreases code complexity and indentation and overall improves readability.

Kind

style advice, maybe?

Bad Examples

if (foo) {
  // lots of other code...
  return 'foo';
} else {
  return 'not foo';
}

Good Examples

if (foo) {
  // lots of other code...
  return 'foo';
}
return 'not foo';

Discussion

Real world example: flutter/flutter#126935 (comment)

Discussion checklist

  • List any existing rules this proposal modifies, complements, overlaps or conflicts with.
  • List any relevant issues (reported here, the SDK Tracker, or elsewhere).
  • If there's any prior art (e.g., in other linters), please add references here.
  • 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?)
  • If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.
@srawlins
Copy link
Member

I think typically I would advocate that the former (bad) example is more readable in many cases.

In cases where a return is an early return that allows 100 lines following to be not indented 2 spaces, it makes sense to not use else. But often I like to use else just to improve readability, like if the then-block is long, or if the condition is long, it ties the state of the following code back to the condition.

Also, as this is a style suggestion, I think we need such a rule to be backed by a style guide, like Effective Dart.

@bwilkerson
Copy link
Member

@munificent

@incendial
Copy link
Contributor

incendial commented May 17, 2023

jfyi, if you decide not to add this rule to the linter, it's already available in DCM https://dcm.dev/docs/rules/common/avoid-redundant-else/

@eernstg
Copy link
Member

eernstg commented May 17, 2023

Drive-by comment: It sounds like this lint could be quite useful. It also reminded me of the idea that some language constructs could announce that "if we reach this point, we're done". Cf. dart-lang/language#3080.

The point is that it is then very easy to see at a glance that a given if-statement is guaranteed to end the execution in the current function body, and this again provides a guarantee (at a glance) that the code that comes after the if-statement will be executed if and only if we would have executed an else part.

@mateusfccp
Copy link
Contributor

I'm with @srawlins. I prefer the "bad" example most of the times. Obviously, there are cases where the readability is worse, specially as we increase the nesting of the condition. However, most cases I see are just like the example:

String something(bool condition) {
  if (condition) return 'Something';

  return 'Not something';
}

Using if-else in this case is way more readable IMO, as it makes it very clear that we have two possible cases. ifs with no elses are often used in cases where we have non-returning side-effects, and this is this the first thing that comes to my mind when I see an elseless if.

@munificent
Copy link
Member

I think about this every time I have conditional logic that returns on both branches. I tend to lean towards the early return style suggested here, but I'm never fully convinced that it is actually better. I do appreciate the symmetry of having the returns in both branches at the same level.

I think this would be a reasonable lint to support but only if users have to explicitly opt in to enable it if they prefer that style and want consistency. I don't think we'd want "Effective Dart" to have an opinion one way or the other. I just don't think it matters enough either way to be worth bothering users.

@pq pq added the P3 A lower priority bug or feature request label May 22, 2023
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 27, 2024
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

9 participants