Lint/UselessMethodDefinition false positive with default method arguments

[pirj]

Lint/UselessMethodDefinition detects an offence in the following code:

module ExpectOffense
  include RuboCop::RSpec::ExpectOffense

  DEFAULT_FILENAME = 'example_spec.rb'

  def expect_offense(source, filename = DEFAULT_FILENAME, *args, **kwargs)
    super
  end
end

However, we're overloading the existing definition, and set a default value for the argument.
It doesn't matter if the default value is overloaded or it's a required positional argument, this method definition is not useless.

I guess the same applies to kwargs's default values.

The build: https://app.circleci.com/pipelines/github/rubocop-hq/rubocop-rspec/437/workflows/8378280c-b36f-4b38-93f8-2dd33e09b9cb/jobs/16059

---

Expected behavior

No offence.

Actual behavior

spec/support/expect_offense.rb:12:3: W: Lint/UselessMethodDefinition: Useless method definition detected.
  def expect_offense(source, filename = DEFAULT_FILENAME, *args, **kwargs) ...
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Steps to reproduce the problem

See the description

RuboCop version

Can't reproduce locally.
Local:

$ [bundle exec] rubocop -V
0.89.1 (using Parser 2.5.1.2, running on ruby 2.5.1 x86_64-linux)

CI:

Using rubocop 0.89.1 from https://github.com/rubocop-hq/rubocop.git (at master@4d8eb11)

[pirj]

cc @fatkodima

[fatkodima]

This cop is already marked as unsafe.

Currently, it is not possible to tell if

  def expect_offense(source, filename = DEFAULT_FILENAME, *args, **kwargs)
    super
  end

changes method definition or just duplicates it, so seems like better to wrap the code with #rubocop:disable ?

If we will specially handle default arguments (ignore, for example), we can possibly miss some legitimate cases.

[pirj]

changes method definition or just duplicates it

Static analysis is unable to tell.

However, common sense suggests that it's not necessary to keep the default value if it was defined in the overridden method. We don't seem to be dealing with copypasted methods, as super clearly indicates there's an intention to override something.

This cop is already marked as unsafe.

"The definition of safety is that the cop doesn’t generate false positives". Got it.
Unfortunately, disabling the cop has a downside, if the default argument gets removed, the method really becomes useless, but the cop is not able to detect this.

If we will specially handle default arguments (ignore, for example), we can possibly miss some legitimate cases.

What are those legitimate cases?
You are trying to point out that there's nothing actionable here, but I'm not still not convinced.

[bquorning]

I agree with @pirj here. If the def node’s args contain optarg or kwoptarg nodes, then it’s probably on purpose if the method contains only a super call.

[fatkodima]

If the def node’s args contain optarg or kwoptarg nodes, then it’s probably on purpose if the method contains only a super call.

I gave this another thought - yeah, this is reasonable.