crystal-ameba / ameba

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

Naming/BlockParameterName + stdlib methods like sort #453

Open petr-fischer opened 7 months ago

petr-fischer commented 7 months ago

Hello - today, ameba complains for usage one-letter block parameter names in the stdlib sort method usage.

src/xxx.cr:58:91
[C] Naming/BlockParameterName: Disallowed block parameter name found
> ... emails.sort { |a, b| b.time <=> a.time }

But even in the Crystal docs, there is example with exact that short block parameter names:

a = [3, 1, 2]
b = a.sort { |a, b| b <=> a }
# ...

https://crystal-lang.org/api/1.11.2/Array.html#sort%28%26block%3AT%2CT-%3EU%29%3AArray%28T%29forallU-instance-method

How would you imagine more descriptive block parameter names when using the sort method? "a" and "b" are optimal.

Thanks!

Sija commented 7 months ago

We could add those to the exclusion list.

petr-fischer commented 7 months ago

Yes please. And also: l, m, n and z :)

straight-shoota commented 7 months ago

This rule is causing confusion and I don't think it's very helpful. Can it be disabled by default? Short names are not inherently bad, particularly in short blocks. The main point against single-character names is that variable names should ideally be descriptive. This is more relevant when used in bigger scopes. But that's a bit mute when declaration and single use are just a couple of characters apart. Then it can be more like a unique identifier without much embedded contextual information.

Related: https://forum.crystal-lang.org/t/ameba-warning-i-dont-understand/6377

Sija commented 5 months ago

Disabling it by default is a viable option, let's do that.