Skip to content

Commit

Permalink
Merge pull request rubocop#12169 from owst/fix_bug_in_style_arguments…
Browse files Browse the repository at this point in the history
…_forwarding_with_duplicate_sends

[Fix rubocop#12168]: Fix bug in `Style/ArgumentsForwarding`
  • Loading branch information
koic authored Aug 31, 2023
2 parents 87587b8 + 6950300 commit ab65d18
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12168](https://github.com/rubocop/rubocop/issues/12168): Fix bug in `Style/ArgumentsForwarding` when there are repeated send nodes. ([@owst][])
14 changes: 8 additions & 6 deletions lib/rubocop/cop/style/arguments_forwarding.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,11 @@ def extract_forwardable_args(args)
end

def only_forwards_all?(send_classifications)
send_classifications.each_value.all? { |c, _, _| c == :all }
send_classifications.all? { |_, c, _, _| c == :all }
end

def add_forward_all_offenses(node, send_classifications, forwardable_args)
send_classifications.each do |send_node, (_c, forward_rest, _forward_kwrest)|
send_classifications.each do |send_node, _c, forward_rest, _forward_kwrest|
register_forward_all_offense(send_node, send_node, forward_rest)
end

Expand All @@ -133,7 +133,7 @@ def add_post_ruby_32_offenses(def_node, send_classifications, forwardable_args)

rest_arg, kwrest_arg, _block_arg = *forwardable_args

send_classifications.each do |send_node, (_c, forward_rest, forward_kwrest)|
send_classifications.each do |send_node, _c, forward_rest, forward_kwrest|
if forward_rest
register_forward_args_offense(def_node.arguments, rest_arg)
register_forward_args_offense(send_node, forward_rest)
Expand All @@ -157,16 +157,18 @@ def non_splat_or_block_pass_lvar_references(body)
end

def classify_send_nodes(def_node, send_nodes, referenced_lvars, forwardable_args)
send_nodes.to_h do |send_node|
send_nodes.filter_map do |send_node|
classification_and_forwards = classification_and_forwards(
def_node,
send_node,
referenced_lvars,
forwardable_args
)

[send_node, classification_and_forwards]
end.compact
next unless classification_and_forwards

[send_node, *classification_and_forwards]
end
end

def classification_and_forwards(def_node, send_node, referenced_lvars, forwardable_args)
Expand Down
100 changes: 100 additions & 0 deletions spec/rubocop/cop/style/arguments_forwarding_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,31 @@ def post(*args, &block)
end
RUBY
end

it 'registers an offense when args are forwarded at several call sites' do
expect_offense(<<~RUBY)
def foo(*args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
baz(*args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
if something?
baz(*args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
end
end
RUBY

expect_correction(<<~RUBY)
def foo(...)
baz(...)
if something?
baz(...)
end
end
RUBY
end
end

context 'TargetRubyVersion >= 3.0', :ruby30 do
Expand Down Expand Up @@ -508,6 +533,31 @@ def foo(m, ...)
RUBY
end

it 'registers an offense when args are forwarded at several call sites' do
expect_offense(<<~RUBY)
def foo(m, *args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
baz(m, *args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
if something?
baz(m, *args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
end
end
RUBY

expect_correction(<<~RUBY)
def foo(m, ...)
baz(m, ...)
if something?
baz(m, ...)
end
end
RUBY
end

it 'does not register an offense if args/kwargs are forwarded with additional pre-kwarg' do
expect_no_offenses(<<~RUBY)
def foo(m, *args, **kwargs, &block)
Expand Down Expand Up @@ -1086,6 +1136,56 @@ def foo(*, **, &block)
RUBY
end

it 'registers an offense when args are forwarded at several call sites' do
expect_offense(<<~RUBY)
def foo(bar, *args)
^^^^^ Use anonymous positional arguments forwarding (`*`).
baz(*args)
^^^^^ Use anonymous positional arguments forwarding (`*`).
if something?
baz(*args)
^^^^^ Use anonymous positional arguments forwarding (`*`).
end
end
RUBY

expect_correction(<<~RUBY)
def foo(bar, *)
baz(*)
if something?
baz(*)
end
end
RUBY
end

it 'registers an offense when kwargs are forwarded at several call sites' do
expect_offense(<<~RUBY)
def foo(bar, **kwargs)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
baz(**kwargs)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
if something?
baz(**kwargs)
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
end
RUBY

expect_correction(<<~RUBY)
def foo(bar, **)
baz(**)
if something?
baz(**)
end
end
RUBY
end

it 'registers an offense for restarg when passing block to separate call' do
expect_offense(<<~RUBY)
def foo(*args, &block)
Expand Down

0 comments on commit ab65d18

Please sign in to comment.