Skip to content

Commit

Permalink
[Fix rubocop#12087] Fix a false positive for Style/ArgumentsForwarding
Browse files Browse the repository at this point in the history
Fixes rubocop#12087, rubocop#12089, rubocop#12096, and rubocop#12100.

This PR fixes false positives for `Style/ArgumentsForwarding`
when leading argument with rest arguments.
  • Loading branch information
koic committed Aug 8, 2023
1 parent 56b4edf commit 91c35af
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12087](https://github.com/rubocop/rubocop/issues/12087): Fix false positives for `Style/ArgumentsForwarding` when leading argument with rest arguments. ([@koic][])
73 changes: 62 additions & 11 deletions lib/rubocop/cop/style/arguments_forwarding.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ class ArgumentsForwarding < Base
ARGS_MSG = 'Use anonymous positional arguments forwarding (`*`).'
KWARGS_MSG = 'Use anonymous keyword arguments forwarding (`**`).'

def on_new_investigation
@forwardable = true
end

def on_def(node)
return unless node.body

Expand Down Expand Up @@ -205,27 +209,74 @@ def register_forward_kwargs_offense(add_parens, def_arguments_or_send, kwrest_ar
end

def register_forward_all_offense_on_forwarding_method(forwarding_method)
add_offense(arguments_range(forwarding_method), message: FORWARDING_MSG) do |corrector|
begin_pos = forwarding_method.loc.selector&.end_pos || forwarding_method.loc.dot.end_pos
range = range_between(begin_pos, forwarding_method.source_range.end_pos)
return unless (begin_pos = find_begin_pos_of_forward_all_offense_range(forwarding_method))

range = range_between(begin_pos, forwarding_method.last_argument.source_range.end_pos)

corrector.replace(range, '(...)')
add_offense(range, message: FORWARDING_MSG) do |corrector|
if forwarding_method.parenthesized?
corrector.replace(range, '...')
else
space_range = range_between(forwarding_method.loc.selector.end_pos, begin_pos)

corrector.replace(space_range, '(')
corrector.replace(range, '...)')
end
end
end

def register_forward_all_offense_on_method_def(method_definition)
add_offense(arguments_range(method_definition), message: FORWARDING_MSG) do |corrector|
arguments_range = range_with_surrounding_space(
method_definition.arguments.source_range, side: :left
)
corrector.replace(arguments_range, '(...)')
return unless @forwardable

range = arguments_range(method_definition)

add_offense(range, message: FORWARDING_MSG) do |corrector|
if method_definition.arguments.parenthesized_call?
corrector.replace(range, '...')
else
space_range = range_between(
method_definition.loc.name.end_pos,
method_definition.first_argument.loc.name.begin_pos - 1
)

corrector.replace(space_range, '(')
corrector.replace(range, '...)')
end
end
end

def find_begin_pos_of_forward_all_offense_range(forwarding_method)
if (forwarding_node = forwarding_method.arguments.find { |node| forwardable_arg?(node) })
if target_ruby_version <= 2.7 && !forwardable_arg?(forwarding_method.first_argument)
@forwardable = false

return
end

forwarding_node.source_range.begin_pos
else
forwarding_method.loc.dot.end_pos
end
end

def arguments_range(node)
arguments = node.arguments

range_between(arguments.first.source_range.begin_pos, arguments.last.source_range.end_pos)
forwarding_node = if forwardable_param?(arguments.first)
arguments.first
else
arguments.find { |argument| forwardable_param?(argument) }
end

range_between(forwarding_node.source_range.begin_pos, arguments.last.source_range.end_pos)
end

def forwardable_param?(argument)
argument.restarg_type? || argument.kwrestarg_type?
end

def forwardable_arg?(argument)
argument.splat_type? || argument.hash_type?
end

def allow_only_rest_arguments?
Expand Down Expand Up @@ -322,7 +373,7 @@ def target_ruby_version

def pre_ruby_32_allow_forward_all?
target_ruby_version < 3.2 &&
@def_node.arguments.none?(&:default?) &&
@def_node.arguments.none? { |arg| arg.respond_to?(:default?) && arg.default? } &&
(@block_arg ? forwarded_block_arg : !@config.fetch(:allow_only_rest_arguments))
end
end
Expand Down
75 changes: 75 additions & 0 deletions spec/rubocop/cop/style/arguments_forwarding_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,14 @@ def foo(...)
end
RUBY
end

it 'does not register an offense when using multiple left hand side with restarg, kwargs, and anonymous block arg' do
expect_no_offenses(<<~RUBY)
def foo((bar, baz), **kwargs)
bar(bar, baz, **kwargs)
end
RUBY
end
end

context 'TargetRubyVersion >= 3.2', :ruby32 do
Expand Down Expand Up @@ -911,6 +919,73 @@ def foo(*args, &block)
RUBY
end

# NOTE: Leading arguments with argument forwarding are not supported until Ruby 2.7.3,
# so in `:ruby27`, it's always accepted.
#
# $ ruby -vce 'def x(...); y(arg, ...); end'
# ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin19]
# -e:1: syntax error, unexpected ')'
# def x(...); y(arg, ...); end
#
# $ ruby -vce 'def x(...); y(arg, ...); end'
# ruby 2.7.3p183 (2021-04-05 revision 6847ee089d) [x86_64-darwin19]
# Syntax OK
it 'does not register an offense when using a leading argument, restarg, kwrestarg and block arg', :ruby27 do
expect_no_offenses(<<~RUBY)
def foo(*args, **kwargs, &block)
bar(arg, *args, **kwargs, &block)
end
RUBY
end

it 'registers an offense when using a leading argument, restarg, kwrestarg and block arg', :ruby30 do
expect_offense(<<~RUBY)
def foo(*args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
bar(arg, *args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
end
RUBY

expect_correction(<<~RUBY)
def foo(...)
bar(arg, ...)
end
RUBY
end

it 'registers an offense when using a leading argument with restarg, kwrestarg and block arg', :ruby30 do
expect_offense(<<~RUBY)
def foo(arg, *args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
bar(arg, *args, **kwargs, &block)
^^^^^^^^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
end
RUBY

expect_correction(<<~RUBY)
def foo(arg, ...)
bar(arg, ...)
end
RUBY
end

it 'registers an offense when using a leading argument with kwrestarg and block arg', :ruby30 do
expect_offense(<<~RUBY)
def foo(arg, **kwargs, &block)
^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
bar(arg, **kwargs, &block)
^^^^^^^^^^^^^^^^ Use shorthand syntax `...` for arguments forwarding.
end
RUBY

expect_correction(<<~RUBY)
def foo(arg, ...)
bar(arg, ...)
end
RUBY
end

context 'UseAnonymousForwarding: false' do
let(:cop_config) { { 'UseAnonymousForwarding' => false } }

Expand Down

0 comments on commit 91c35af

Please sign in to comment.