Add AllowUnusedKeywordArguments option for UnusedMethodArgument

[apiology]

Hi!

The UnusedMethodArgument cop is great for finding unused method arguments. One reason why people might have unused method arguments is when they are subclassing or adhering to a duck type. In those cases, RuboCop suggests adding an underscore to the method argument.

This is normally great advice for positional arguments, but in Ruby, underscores in front of keyword arguments result in run-time errors when you try to pass in those keyword arguments--the underscore doesn't have the same semantics as positional arguments.

The net result is that if you have a project with a lot of subclassing and/or duck-types and keyword arguments, UnusedMethodArgument gives you a lot of false positives.

This change provides an option called "AllowUnusedKeywordArguments" to address this by disabling the cop for unused keyword arguments.

[jonas054]

You're right, but since prepending an underscore to keyword arguments can change the semantics of calling the method, I think you should take the change even further.

Either AllowUnusedKeywordArguments should be true by default, or remove the option and never auto-correct keyword arguments.

[bbatsov]

Btw, this is also related to simply checking whether the body of a method is empty. Guess we can have an option to suppress this check for empty method bodies (as we can presume they're just abstract).

[bbatsov]

remove the option and never auto-correct keyword arguments.

Probably this would be best.

[apiology]

Keyword arguments are already ignored in auto-correct (see https://github.com/bbatsov/rubocop/blob/master/spec/rubocop/cop/lint/unused_method_argument_spec.rb#L310-L320), which is good news.

That said, I feel that this is not enough, since if you have a project with a lot of subclassing and/or duck-types and keyword arguments, UnusedMethodArgument gives you a lot of false positives.

I'd be happy to switch the default for AllowUnusedKeywordArguments over to false or leave it as true--thoughts?

[jonas054]

Ah. Then I guess you can leave it as false (you said the other way around, but it is false now), since there's already basic support for the differentiation between positional and keyword arguments. Your addition is something extra for people who prefer peace and quiet and really don't want any reports about unused keyword arguments.

[apiology]

Yes, you're right--because in many cases those reports aren't actionable.

Ready to merge, then?

[bbatsov]

I'm OK with merging it, but the PR has to be rebased first.

[apiology]

Sounds good--done!