crystal-ameba / ameba

A static code analysis tool for Crystal
https://crystal-ameba.github.io
MIT License
520 stars 38 forks source link

Add rule enforcing explicit return #467

Closed nobodywasishere closed 2 weeks ago

nobodywasishere commented 2 months ago

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 commented 2 months ago

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

z64 commented 2 months ago

@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

So we are more than fine with enforcing it.

straight-shoota commented 2 months ago

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 commented 2 months ago

@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 commented 1 month ago

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