AdRoll / rebar3_hank

The Erlang Dead Code Cleaner
MIT License
68 stars 9 forks source link

[#68] Adding support to ignore specific items #83

Closed diegomanuel closed 3 years ago

diegomanuel commented 3 years ago

The idea is that each rule must implement the callback ignored/2 that will receive:

Added the pattern property (undefined | tuple()) to hank_rule:result/0 type and setting it as undefined in all current existing rules.


Did the first ignore implementation for unnecessary_function_arguments rule and seems to be pretty straightforward. We might need to adjust things a little bit when adding the ignore implementation for other rules, but it should be generic enough to only need minimal adjustments.


The "ignore spec" is explained in this example (including the implemented for unnecessary_function_arguments):

-hank([unused_macros,  %% This will keep ignoring the whole rule, just as before
       {unused_callbacks, cb_func_name_to_ignore}  %% When implemented, it should ignore the given cb func name
       {unnecessary_function_arguments,  %% You can give a list of multiple specs (or a single one like above)
           [{ignore_me, 2},  %% Will ignore any unused argument from `ignore_me/2`
            {ignore_me_too, 3, 2},  %% Will ignore the 2nd argument from `ignore_me_too/3`
            ignore_me_again]}]). %% Will ignore any unused argument from any `ignore_me_again/x` (no matter the arity)

IMPORTANT NOTE: It only support to give specific items to be ignored at -hank module attribute. From config, it stays the same as before, the user can ignore entire rules from specific files. It doesn't make much sense to me to have the same level of "ignore detail" at config because you would be adding specific functions and/or any other module-specific code definition at config level (in where one expects to be "abstracted" from such a detail level). It think it's a best practice to "force" the developer to define specific ignore rules within the same module that you want to ignore specific items and that's it. If you think folks that we should also add this support at config level, just LMK!

elbrujohalcon commented 3 years ago

I approved this but I forgot to check if you added proper documentation about the new ignore mechanism and the patterns for the unnecessary arguments rule. Please don't forget to document the new stuff, @diegomanuel.

diegomanuel commented 3 years ago

Done, documentation included! Plz take a final look and will merge once both of you folks approve it.