Unexpected warnings for ignored files

[paulo-ferraz-oliveira]

Bug Description

This is potentially related to recent #159.

To Reproduce

rebar3 new lib
cd mylib
echo -e "\n{project_plugins, [rebar3_hank]}." >> rebar.config
echo -e "\n{hank, [{ignore, [\"src/mylib.erl\"]}]}." >> rebar.config
rebar3 kiwf

# Got
===> The following ignore specs are no longer needed and can be removed:
* src/mylib.erl: unused_record_fields
* src/mylib.erl: unused_macros
* src/mylib.erl: unused_hrls
* src/mylib.erl: unused_configuration_options
* src/mylib.erl: unused_callbacks
* src/mylib.erl: unnecessary_function_arguments
* src/mylib.erl: single_use_hrls
* src/mylib.erl: single_use_hrl_attrs

Expected Behavior

No warnings.

[elbrujohalcon]

This thing is a bit more complex than it seems.

First of all, there is another situation in which too many warnings are emitted: a file with -hank ignore.

For instance, this file…

-module(ignore_all).
-hank ignore.

…will produce these warnings:

* src/ignore_all.erl: unused_record_fields
* src/ignore_all.erl: unused_macros
* src/ignore_all.erl: unused_hrls
* src/ignore_all.erl: unused_configuration_options
* src/ignore_all.erl: unused_callbacks
* src/ignore_all.erl: unnecessary_function_arguments
* src/ignore_all.erl: single_use_hrls
* src/ignore_all.erl: single_use_hrl_attrs

But, on the other hand, this file…

-module(ignore_all).
-hank ignore.

-define(AN_UNUSED_MACRO, this_macro).

-record(a_record, {with, multiple, unused, fields}).

…will only produce these warnings:

* src/ignore_all.erl: unused_hrls
* src/ignore_all.erl: unused_configuration_options
* src/ignore_all.erl: unused_callbacks
* src/ignore_all.erl: unnecessary_function_arguments
* src/ignore_all.erl: single_use_hrls
* src/ignore_all.erl: single_use_hrl_attrs

In this context, Hank is actually giving the user some valid information:

You don't need to ignore all the rules, there are no warnings for these ones in your file. You can be more specific.

[elbrujohalcon]

Consider this scenario as well…

rebar.config

{hank, [{ignore, ["ignore*.erl"]}]}.

ignore_empty.erl

-module(ignore_empty).

ignore_not_empty.erl

-module(ignore_not_empty).

-define(AN_UNUSED_MACRO, this_macro).

-record(a_record, {with, multiple, unused, fields}).

Hank will then tell the user…

===> The following ignore specs are no longer needed and can be removed:
* src/ignore_not_empty.erl: unused_hrls
* src/ignore_not_empty.erl: unused_configuration_options
* src/ignore_not_empty.erl: unused_callbacks
* src/ignore_not_empty.erl: unnecessary_function_arguments
* src/ignore_not_empty.erl: single_use_hrls
* src/ignore_not_empty.erl: single_use_hrl_attrs
* src/ignore_empty.erl: unused_record_fields
* src/ignore_empty.erl: unused_macros
* src/ignore_empty.erl: unused_hrls
* src/ignore_empty.erl: unused_configuration_options
* src/ignore_empty.erl: unused_callbacks
* src/ignore_empty.erl: unnecessary_function_arguments
* src/ignore_empty.erl: single_use_hrls
* src/ignore_empty.erl: single_use_hrl_attrs

Again, there is some valuable information there. The users can decide to be more specific on which rules they want to enforce and which ones they one to ignore for each file, instead of ignoring all rules for all files.

[elbrujohalcon]

I'm inclined to fix this issue by just adding a command-line option --unused_ignores that will be true by default, preserving current behavior, but users can turn it off if they don't want to see that many warnings on the command output.

What do you think, @pbrudnick @diegomanuel @paulo-ferraz-oliveira ?

[paulo-ferraz-oliveira]

I'm Ok with this. As long as it's controllable (option) and as easy to use as before (which it seems to be). This actually happened on a file I don't quite control (generated by yecc), so I'm sure I'm not alone in this. Thanks for looking into it, Brujo.