Skip to content

Commit

Permalink
[Fix #12070] Fix false positive in Style/ArgumentsForwarding
Browse files Browse the repository at this point in the history
Fixes a false positive caused by incorrectly searching in the receiver
as well as arguments.
  • Loading branch information
owst authored and bbatsov committed Jul 28, 2023
1 parent 14e81ad commit 24f6e99
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12070](https://github.com/rubocop/rubocop/pull/12070): Fix false positive in `Style/ArgumentsForwarding` when receiver forwards args/kwargs. ([@owst][])
22 changes: 13 additions & 9 deletions lib/rubocop/cop/style/arguments_forwarding.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,14 +240,14 @@ def use_anonymous_forwarding?
class SendNodeClassifier
extend NodePattern::Macros

# @!method find_forwarded_rest_arg(node, rest_name)
def_node_search :find_forwarded_rest_arg, '(splat (lvar %1))'
# @!method forwarded_rest_arg?(node, rest_name)
def_node_matcher :forwarded_rest_arg?, '(splat (lvar %1))'

# @!method find_forwarded_kwrest_arg(node, kwrest_name)
def_node_search :find_forwarded_kwrest_arg, '(kwsplat (lvar %1))'
# @!method extract_forwarded_kwrest_arg(node, kwrest_name)
def_node_matcher :extract_forwarded_kwrest_arg, '(hash <$(kwsplat (lvar %1)) ...>)'

# @!method find_forwarded_block_arg(node, block_name)
def_node_search :find_forwarded_block_arg, '(block_pass {(lvar %1) nil?})'
# @!method forwarded_block_arg?(node, block_name)
def_node_matcher :forwarded_block_arg?, '(block_pass {(lvar %1) nil?})'

def initialize(def_node, send_node, referenced_lvars, forwardable_args, **config)
@def_node = def_node
Expand All @@ -262,19 +262,19 @@ def initialize(def_node, send_node, referenced_lvars, forwardable_args, **config
def forwarded_rest_arg
return nil if referenced_rest_arg?

find_forwarded_rest_arg(@send_node, @rest_arg_name).first
arguments.find { |arg| forwarded_rest_arg?(arg, @rest_arg_name) }
end

def forwarded_kwrest_arg
return nil if referenced_kwrest_arg?

find_forwarded_kwrest_arg(@send_node, @kwrest_arg_name).first
arguments.filter_map { |arg| extract_forwarded_kwrest_arg(arg, @kwrest_arg_name) }.first
end

def forwarded_block_arg
return nil if referenced_block_arg?

find_forwarded_block_arg(@send_node, @block_arg_name).first
arguments.find { |arg| forwarded_block_arg?(arg, @block_arg_name) }
end

def classification
Expand All @@ -289,6 +289,10 @@ def classification

private

def arguments
@send_node.arguments
end

def referenced_rest_arg?
@referenced_lvars.include?(@rest_arg_name)
end
Expand Down
76 changes: 76 additions & 0 deletions spec/rubocop/cop/style/arguments_forwarding_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,30 @@ def foo(*args, arg: 1)
RUBY
end
end

it 'does not register an offense for restarg when passing block to separate call' do
expect_no_offenses(<<~RUBY)
def foo(*args, &block)
bar(*args).baz(&block)
end
RUBY
end

it 'does not register an offense for restarg and kwrestarg when passing block to separate call' do
expect_no_offenses(<<~RUBY)
def foo(*args, **kwargs, &block)
bar(*args, **kwargs).baz(&block)
end
RUBY
end

it 'does not register an offense for restarg/kwrestarg/block passed to separate methods' do
expect_no_offenses(<<~RUBY)
def foo(*args, **kwargs, &block)
bar(first(*args), second(**kwargs), third(&block))
end
RUBY
end
end

context 'TargetRubyVersion >= 3.1', :ruby31 do
Expand Down Expand Up @@ -591,6 +615,58 @@ def foo(*, **, &block)
RUBY
end

it 'registers an offense for restarg when passing block to separate call' do
expect_offense(<<~RUBY)
def foo(*args, &block)
^^^^^ Use anonymous positional arguments forwarding (`*`).
bar(*args).baz(&block)
^^^^^ Use anonymous positional arguments forwarding (`*`).
end
RUBY

expect_correction(<<~RUBY)
def foo(*, &block)
bar(*).baz(&block)
end
RUBY
end

it 'registers an offense for restarg and kwrestarg when passing block to separate call' do
expect_offense(<<~RUBY)
def foo(*args, **kwargs, &block)
^^^^^ Use anonymous positional arguments forwarding (`*`).
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
bar(*args, **kwargs).baz(&block)
^^^^^ Use anonymous positional arguments forwarding (`*`).
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
RUBY

expect_correction(<<~RUBY)
def foo(*, **, &block)
bar(*, **).baz(&block)
end
RUBY
end

it 'registers an offense for restarg and kwrestarg when passing to separate calls' do
expect_offense(<<~RUBY)
def foo(*args, **kwargs, &block)
^^^^^ Use anonymous positional arguments forwarding (`*`).
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
bar(first(*args), second(**kwargs), third(&block))
^^^^^ Use anonymous positional arguments forwarding (`*`).
^^^^^^^^ Use anonymous keyword arguments forwarding (`**`).
end
RUBY

expect_correction(<<~RUBY)
def foo(*, **, &block)
bar(first(*), second(**), third(&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 24f6e99

Please sign in to comment.