Skip to content
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

Add rule enforcing explicit return #467

Open
nobodywasishere opened this issue Aug 19, 2024 · 5 comments
Open

Add rule enforcing explicit return #467

nobodywasishere opened this issue Aug 19, 2024 · 5 comments

Comments

@nobodywasishere
Copy link
Contributor

While implicit return is nice, sometimes it can cause unintended effects / footguns. Would it be possible to add a (disabled by default) rule that's the exact opposite of Style/RedundantReturn, encouraging the use of explicit returns?

@straight-shoota
Copy link
Contributor

Could you show a case where implicit return would be a footgun?

@z64
Copy link

z64 commented Aug 20, 2024

@straight-shoota We had an unnoticed buggy method that was effectively

def something : Bool
  if cond
    # .. extra evaluation ..
    true
  end
  false
end

which was unlikely to ever be written in the first place if implicit returns didn't exist. You would never just write naked true that you intended to return.

The subjective benefit of implicit return's brevity is outweighed by

  • All the other languages we work with in our stack (Go, JS, Python, C/C++) all require return. From this POV Crystal appears inconsistent and confusing to onboarders.
  • Being able to grep for the return expression instead of having to eyeball it
  • Not losing to the habits of using implicit returns causing bugs like above

So we are more than fine with enforcing it.

@straight-shoota
Copy link
Contributor

I suppose this requires semantic analysis for being any good. Without a semantic pass you dont know which code paths are NoReturn. So it would require return where no value is returned at all.

So at this point ameba cannot implement this.

@straight-shoota
Copy link
Contributor

straight-shoota commented Aug 20, 2024

@z64 Regarding the consistency argument: There are far more significant differences between the semantics of different programming languages than implicit returns - which is actually a very obvious one.

Anyway I believe for that example it might be helpful to recognize a "naked true" and other unused values. This is of course limited because method calls might only exist for side effexts and legitimately not be interested in the return type.
But simple cases like this one would be something a linter could recongnize and complain about.

@Sija
Copy link
Member

Sija commented Sep 2, 2024

Without semantic analysis that's going to be pretty useless, as mentioned already by @straight-shoota.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants