crystal-ameba / ameba

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

exception names are too limited #433

Closed stakach closed 8 months ago

stakach commented 8 months ago

Currently the default ameba options seem too limited. %w[e ex exception] I feel like this should include version common names like error but also it should probably add something like || begins_with?("#{allowed}_") so nested exceptions can be handled like ex_parse ex_io etc

Also e is not very descriptive and breaks other rules like descriptive block naming.

Sija commented 8 months ago

I feel like this should include version common names like error

It's not that common, therefore I didn't include it. You can add it to your project's .ameba.yml file.

it should probably add something like || begins_with?("#{allowed}_") so nested exceptions can be handled like ex_parse ex_io etc

I disagree, exception variables should IMO have unified names.

Also e is not very descriptive ...

IMO it is - in the same way i (for index) is.

... and breaks other rules like descriptive block naming.

It shouldn't, since it's explicitly whitelisted:

https://github.com/crystal-ameba/ameba/blob/47088b10ca97aff96c9580ff50b17cd4e945a175/src/ameba/rule/naming/block_parameter_name.cr#L31

stakach commented 8 months ago

Seem pretty commonly used https://github.com/search?q=%28language%3ACrystal+%29+AND+%22rescue+error%22&type=code

image

stakach commented 8 months ago

more common than rescue exception

image

stakach commented 8 months ago

also missing exc which is also quite commonly used

image

Sija commented 8 months ago

Yeah, in the same time rescue ex has 6.9k occurrences (~10x more than rescue exc) - i.e. all other uses are pretty marginal. I'm gonna add error to the defaults though, since it was unaccounted for.

stakach commented 8 months ago

thanks mate!

straight-shoota commented 8 months ago

Perhaps use cases such as ex_ prefix could be enabled with regex matching support?