Unused _Variables reported as such

[paulo-ferraz-oliveira]

Let's say I have this code

test(_Config) -> ok.

rebar3_hank signals the parameter as unused. But I feel that naming unused variables (instead of simply using _) helps readability (I use it extensively).

What is the rationale behind the warning? (or is there an option to turn it off I didn't notice?)

[elbrujohalcon]

The idea of the rule is to spot parameters that are unused in all clauses of the function and therefore unnecessary. The function would work as well with one less argument.

[paulo-ferraz-oliveira]

Not if it's a callback function 😄. And you will get those (a LOT) for test files, I imagine...

[elbrujohalcon]

Look no further than http_uri:parse_host_port/4.

That function has three clauses…

parse_host_port(_Scheme, DefaultPort, <<"[", HostPort/binary>>, Opts) -> %ipv6
    …
parse_host_port(_Scheme, DefaultPort, "[" ++ HostPort, Opts) -> %ipv6
    …
parse_host_port(_Scheme, DefaultPort, HostPort, _Opts) ->
    …

The first argument (_Scheme) is ignored in all of them.

Just to be sure… this function is not a behavior callback.

It's actually only used here and here.

So, since the first argument is useless, it can be removed. That would turn parse_host_port into…

parse_host_port(DefaultPort, <<"[", HostPort/binary>>, Opts) -> %ipv6
    …
parse_host_port(DefaultPort, "[" ++ HostPort, Opts) -> %ipv6
    …
parse_host_port(DefaultPort, HostPort, _Opts) ->
    …

…and it will simplify parse_uri_rest/4 just a tiny bit, by removing Scheme from the calls to parse_uri_rest/3.

Cool, isn't it? 😎

[paulo-ferraz-oliveira]

It is cool in that case, sure. But there are "behaviours" that aren't defined as such (e.g. a CT suite) - imagining you already take callbacks into account (might not be the case).

[elbrujohalcon]

We take callbacks into account, yes… and for common test suites, at least until it's defined as a proper behavior… you (the user) will have to ignore unused_ignored_function_parameters like this…

-hank [unused_ignored_function_parameters].

Even more, we're (in the current version - 0.2.1) ignoring way too much. We plan to adjust that in #40.

[paulo-ferraz-oliveira]

That's the option I wanted. As for the behaviour, it's been merged but it'll take a while before it's ubiquitous.

[pbrudnick]

This rule name is confusing, it's the proposal for the new name (by @elbrujohalcon )
#69