Skip to content

Commit

Permalink
[Fix #12257] Make Style/RedundantDoubleSplatHashBraces aware of `me…
Browse files Browse the repository at this point in the history
…rge`

Resolves #12257.

This PR makes `Style/RedundantDoubleSplatHashBraces` aware of `merge` methods.
Instead of introducing a new cop, this `Style/RedundantDoubleSplatHashBraces` cop can be extended.
  • Loading branch information
koic authored and bbatsov committed Oct 11, 2023
1 parent 86c0e8d commit 81ef51e
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12257](https://github.com/rubocop/rubocop/issues/12257): Make `Style/RedundantDoubleSplatHashBraces` aware of `merge` methods. ([@koic][])
73 changes: 68 additions & 5 deletions lib/rubocop/cop/style/redundant_double_splat_hash_braces.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,95 @@ module Style
# # good
# do_something(foo: bar, baz: qux)
#
# # bad
# do_something(**{foo: bar, baz: qux}.merge(options))
#
# # good
# do_something(foo: bar, baz: qux, **options)
#
class RedundantDoubleSplatHashBraces < Base
extend AutoCorrector

MSG = 'Remove the redundant double splat and braces, use keyword arguments directly.'
MERGE_METHODS = %i[merge merge!].freeze

# rubocop:disable Metrics/CyclomaticComplexity
def on_hash(node)
return if node.pairs.empty? || node.pairs.any?(&:hash_rocket?)
return unless (parent = node.parent)
return unless parent.kwsplat_type?
return unless (kwsplat = node.each_ancestor(:kwsplat).first)
return if parent.call_type? && !merge_method?(parent)

add_offense(parent) do |corrector|
corrector.remove(parent.loc.operator)
corrector.remove(opening_brace(node))
corrector.remove(closing_brace(node))
add_offense(kwsplat) do |corrector|
autocorrect(corrector, node, kwsplat)
end
end
# rubocop:enable Metrics/CyclomaticComplexity

private

def autocorrect(corrector, node, kwsplat)
corrector.remove(kwsplat.loc.operator)
corrector.remove(opening_brace(node))
corrector.remove(closing_brace(node))

merge_methods = select_merge_method_nodes(kwsplat)
return if merge_methods.empty?

autocorrect_merge_methods(corrector, merge_methods, kwsplat)
end

def select_merge_method_nodes(kwsplat)
extract_send_methods(kwsplat).select do |node|
merge_method?(node)
end
end

def opening_brace(node)
node.loc.begin.join(node.children.first.source_range.begin)
end

def closing_brace(node)
node.children.last.source_range.end.join(node.loc.end)
end

def autocorrect_merge_methods(corrector, merge_methods, kwsplat)
range = range_of_merge_methods(merge_methods)

new_kwsplat_arguments = extract_send_methods(kwsplat).map do |descendant|
convert_to_new_arguments(descendant)
end
new_source = new_kwsplat_arguments.compact.reverse.unshift('').join(', ')

corrector.replace(range, new_source)
end

def range_of_merge_methods(merge_methods)
begin_merge_method = merge_methods.last
end_merge_method = merge_methods.first

begin_merge_method.loc.dot.begin.join(end_merge_method.source_range.end)
end

def extract_send_methods(kwsplat)
@extract_send_methods ||= kwsplat.each_descendant(:send, :csend)
end

def convert_to_new_arguments(node)
return unless merge_method?(node)

node.arguments.map do |arg|
if arg.hash_type?
arg.source
else
"**#{arg.source}"
end
end
end

def merge_method?(node)
MERGE_METHODS.include?(node.method_name)
end
end
end
end
Expand Down
81 changes: 79 additions & 2 deletions spec/rubocop/cop/style/redundant_double_splat_hash_braces_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,83 @@
RUBY
end

it 'registers an offense when using double splat hash braces with `merge` method call' do
expect_offense(<<~RUBY)
do_something(**{foo: bar, baz: qux}.merge(options))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant double splat and braces, use keyword arguments directly.
RUBY

expect_correction(<<~RUBY)
do_something(foo: bar, baz: qux, **options)
RUBY
end

it 'registers an offense when using double splat hash braces with `merge!` method call' do
expect_offense(<<~RUBY)
do_something(**{foo: bar, baz: qux}.merge!(options))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant double splat and braces, use keyword arguments directly.
RUBY

expect_correction(<<~RUBY)
do_something(foo: bar, baz: qux, **options)
RUBY
end

it 'registers an offense when using double splat hash braces with `merge` pair arguments method call' do
expect_offense(<<~RUBY)
do_something(**{foo: bar, baz: qux}.merge(x: y))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant double splat and braces, use keyword arguments directly.
RUBY

expect_correction(<<~RUBY)
do_something(foo: bar, baz: qux, x: y)
RUBY
end

it 'registers an offense when using double splat hash braces with `merge` safe navigation method call' do
expect_offense(<<~RUBY)
do_something(**{foo: bar, baz: qux}&.merge(options))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant double splat and braces, use keyword arguments directly.
RUBY

expect_correction(<<~RUBY)
do_something(foo: bar, baz: qux, **options)
RUBY
end

it 'registers an offense when using double splat hash braces with `merge` multiple arguments method call' do
expect_offense(<<~RUBY)
do_something(**{foo: bar, baz: qux}.merge(options1, options2))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant double splat and braces, use keyword arguments directly.
RUBY

expect_correction(<<~RUBY)
do_something(foo: bar, baz: qux, **options1, **options2)
RUBY
end

it 'registers an offense when using double splat hash braces with `merge` method chain' do
expect_offense(<<~RUBY)
do_something(**{foo: bar, baz: qux}.merge(options1, options2).merge(options3))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant double splat and braces, use keyword arguments directly.
RUBY

expect_correction(<<~RUBY)
do_something(foo: bar, baz: qux, **options1, **options2, **options3)
RUBY
end

it 'registers an offense when using double splat hash braces with complex `merge` method chain' do
expect_offense(<<~RUBY)
do_something(**{foo: bar, baz: qux}.merge(options1, options2)&.merge!(options3))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant double splat and braces, use keyword arguments directly.
RUBY

expect_correction(<<~RUBY)
do_something(foo: bar, baz: qux, **options1, **options2, **options3)
RUBY
end

it 'does not register an offense when using keyword arguments' do
expect_no_offenses(<<~RUBY)
do_something(foo: bar, baz: qux)
Expand All @@ -59,9 +136,9 @@
RUBY
end

it 'does not register an offense when using method call for double splat hash braces arguments' do
it 'does not register an offense when using method call that is not `merge` for double splat hash braces arguments' do
expect_no_offenses(<<~RUBY)
do_something(**{foo: bar}.merge(options))
do_something(**{foo: bar}.invert)
RUBY
end

Expand Down

0 comments on commit 81ef51e

Please sign in to comment.