Lint/UnusedMethodArgument does not correct or mark keyword args as "todo"

[laurmurclar]

When using the --safe-auto-correct --disable-uncorrectable flags, Lint/UnusedMethodArgument has some inconsistent behaviour. It auto-corrects methods with standard arguments, but not keyword arguments. It doesn't add rubocop:todo comments to methods with keyword arguments either.

---

Expected behavior

Auto-correct and disabling uncorrectable offenses works for methods with keyword arguments

Steps to reproduce the problem

example.rb

def ordinary_method(some_arg)
  puts "Ignoring args"
end

def method_with_keyword_arg(some_keyword_arg:)
  puts "Ignoring args"
end

$ bundle exec 'rubocop --safe-auto-correct --disable-uncorrectable --only Lint/UnusedMethodArgument example.rb'
Inspecting 1 file
W

Offenses:

example.rb:1:21: W: [Corrected] Lint/UnusedMethodArgument: Unused method argument - some_arg. If it's necessary, use _ or _some_arg as an argument name to indicate that it won't be used. You can also write as ordinary_method(*) if you want the method to accept any arguments but don't care about them.
def ordinary_method(some_arg)
                    ^^^^^^^^
example.rb:5:29: W: Lint/UnusedMethodArgument: Unused method argument - some_keyword_arg. You can also write as method_with_keyword_arg(*) if you want the method to accept any arguments but don't care about them.
def method_with_keyword_arg(some_keyword_arg:)
                            ^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 1 offense corrected

RuboCop version

$ bundle exec 'rubocop -V'
0.83.0 (using Parser 2.7.1.2, running on ruby 2.6.5 x86_64-darwin19)

[rrosenblum]

I've been away from RuboCop development for too long. I didn't know --disable-uncorrectable had been implemented. I'm glad to see this feature around.

Taking a quick look at this issue, I have come across a couple of things. First off, I'm not sure why Lint/UnusedMethodArgument will change an unused argument to _some_arg to show that it is unused, but it will not correct a keyword arg in the same way. My guess is that since a keyword argument with an underscore is still a required argument, then there isn't much use in adding the underscore.

This directly plays into why the comment disable isn't working though. Here is the logic for auto-correction

        def correct(node)
          reason = reason_to_not_correct(node)
          return reason if reason

          @corrected_nodes[node] = true
          if support_autocorrect?
            correction = autocorrect(node)
            return :uncorrected unless correction

            @corrections << Correction.new(correction, node, self)
            :corrected
          elsif disable_uncorrectable?
            disable_uncorrectable(node)
            :corrected_with_todo
          end
        end

With how this has been implemented, a cop can either auto-correct or it can disable, it cannot do both. Since Lint/UnusedMethodArgument can correct some offenses, but not others, it is unable to disable anything.

I think the basic fix is to modify the code inside the support_autocorrect? branch where we are currently doing return :uncorrected unless correction.

[marcandre]

I think the basic fix is to modify the code inside the support_autocorrect? branch where we are currently doing return :uncorrected unless correction.

Actually knowing if a Cop has done corrections will be automatic once #7868 merges, hopefully in the next few days.

[laurmurclar]

Thank you so much for taking this on, delighted to see the fix merged!