Skip to content

Commit

Permalink
[Fix #12141] Fix false positive for Style/ArgumentsForwarding
Browse files Browse the repository at this point in the history
If additional kwargs are supplied then forward-all (`...`) cannot be
used.
  • Loading branch information
owst committed Aug 21, 2023
1 parent 51ed9ab commit 81ce84f
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12141](https://github.com/rubocop/rubocop/issues/12141): Fix false positive for `Style/ArgumentsForwarding` when method def includes additional kwargs. ([@owst][])
10 changes: 9 additions & 1 deletion lib/rubocop/cop/style/arguments_forwarding.rb
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ def can_forward_all?
return false if any_arg_referenced?
return false if ruby_32_missing_rest_or_kwest?
return false unless offensive_block_forwarding?
return false if forward_additional_kwargs?
return false if additional_kwargs_or_forwarded_kwargs?

no_additional_args? || (target_ruby_version >= 3.0 && no_post_splat_args?)
end
Expand Down Expand Up @@ -339,6 +339,14 @@ def no_post_splat_args?
[nil, :hash, :block_pass].include?(arg_after_splat&.type)
end

def additional_kwargs_or_forwarded_kwargs?
additional_kwargs? || forward_additional_kwargs?
end

def additional_kwargs?
@def_node.arguments.any? { |a| a.kwarg_type? || a.kwoptarg_type? }
end

def forward_additional_kwargs?
return false unless forwarded_kwrest_arg

Expand Down
122 changes: 122 additions & 0 deletions spec/rubocop/cop/style/arguments_forwarding_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,14 @@ def foo((bar, baz), **kwargs)
RUBY
end

it 'does not register an offense with an additional kwarg' do
expect_no_offenses(<<~RUBY)
def foo(first:, **kwargs, &block)
forwarded(**kwargs, &block)
end
RUBY
end

context 'AllowOnlyRestArgument: true' do
let(:cop_config) { { 'AllowOnlyRestArgument' => true } }

Expand Down Expand Up @@ -444,6 +452,38 @@ def foo(m, *args, &block)
RUBY
end

it 'does not register an offense with an additional required kwarg that is not forwarded' do
expect_no_offenses(<<~RUBY)
def foo(first:, **kwargs, &block)
forwarded(**kwargs, &block)
end
RUBY
end

it 'does not register an offense with an additional required kwarg that is forwarded' do
expect_no_offenses(<<~RUBY)
def foo(first:, **kwargs, &block)
forwarded(first: first, **kwargs, &block)
end
RUBY
end

it 'does not register an offense with an additional optional kwarg that is not forwarded' do
expect_no_offenses(<<~RUBY)
def foo(first: nil, **kwargs, &block)
forwarded(**kwargs, &block)
end
RUBY
end

it 'does not register an offense with an additional optional kwarg that is forwarded' do
expect_no_offenses(<<~RUBY)
def foo(first: nil, **kwargs, &block)
forwarded(first: first, **kwargs, &block)
end
RUBY
end

it 'does not register an offense if args/kwargs are forwarded with a positional parameter last' do
expect_no_offenses(<<~RUBY)
def foo(m, *args, **kwargs, &block)
Expand Down Expand Up @@ -1098,6 +1138,88 @@ def foo(*, **, &block)
RUBY
end

it 'registers an offense with an additional required kwarg that is not forwarded' do
expect_offense(<<~RUBY)
def foo(first:, **kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
forwarded(**kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
RUBY

expect_correction(<<~RUBY)
def foo(first:, **, &block)
forwarded(**, &block)
end
RUBY
end

it 'registers an offense with an additional required kwarg that is forwarded' do
expect_offense(<<~RUBY)
def foo(first:, **kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
forwarded(first: first, **kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
RUBY

expect_correction(<<~RUBY)
def foo(first:, **, &block)
forwarded(first: first, **, &block)
end
RUBY
end

it 'registers an offense with an additional optional kwarg that is not forwarded' do
expect_offense(<<~RUBY)
def foo(first: nil, **kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
forwarded(first:, **kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
RUBY

expect_correction(<<~RUBY)
def foo(first: nil, **, &block)
forwarded(first:, **, &block)
end
RUBY
end

it 'registers an offense with an additional optional kwarg that is forwarded' do
expect_offense(<<~RUBY)
def foo(first: nil, **kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
forwarded(first: first, **kwargs, &block)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
RUBY

expect_correction(<<~RUBY)
def foo(first: nil, **, &block)
forwarded(first: first, **, &block)
end
RUBY
end

it 'registers an offense with an arg and additional optional kwarg that is forwarded' do
expect_offense(<<~RUBY)
def foo(*args, first: nil, **kwargs, &block)
^^^^^ Use anonymous positional arguments forwarding (`*`).
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
forwarded(*args, first: first, **kwargs, &block)
^^^^^ Use anonymous positional arguments forwarding (`*`).
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
RUBY

expect_correction(<<~RUBY)
def foo(*, first: nil, **, &block)
forwarded(*, first: first, **, &block)
end
RUBY
end

it 'registers an offense when using restarg and block arg' do
expect_offense(<<~RUBY)
def foo(*args, &block)
Expand Down

0 comments on commit 81ce84f

Please sign in to comment.