Skip to content

Commit

Permalink
[Fix #12424] Make Style/HashEachMethods aware of safe navigation op…
Browse files Browse the repository at this point in the history
…erator

Fixes #12424.

This PR makes `Style/HashEachMethods` aware of safe navigation operator.
  • Loading branch information
koic authored and bbatsov committed Dec 1, 2023
1 parent 3db23fa commit c8d2d77
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12424](https://github.com/rubocop/rubocop/issues/12424): Make `Style/HashEachMethods` aware of safe navigation operator. ([@koic][])
21 changes: 12 additions & 9 deletions lib/rubocop/cop/style/hash_each_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ class HashEachMethods < Base

# @!method kv_each(node)
def_node_matcher :kv_each, <<~PATTERN
({block numblock} $(send (send _ ${:keys :values}) :each) ...)
({block numblock} $(call (call _ ${:keys :values}) :each) ...)
PATTERN

# @!method each_arguments(node)
def_node_matcher :each_arguments, <<~PATTERN
(block (send _ :each)(args $_key $_value) ...)
(block (call _ :each)(args $_key $_value) ...)
PATTERN

# @!method kv_each_with_block_pass(node)
def_node_matcher :kv_each_with_block_pass, <<~PATTERN
(send $(send _ ${:keys :values}) :each (block_pass (sym _)))
(call $(call _ ${:keys :values}) :each (block_pass (sym _)))
PATTERN

# rubocop:disable Metrics/AbcSize
Expand Down Expand Up @@ -92,7 +92,9 @@ def register_kv_offense(target, method)
return unless (parent_receiver = target.receiver.receiver)
return if allowed_receiver?(parent_receiver)

add_offense(kv_range(target), message: format_message(method)) do |corrector|
current = target.receiver.loc.selector.join(target.source_range.end).source

add_offense(kv_range(target), message: format_message(method, current)) do |corrector|
correct_key_value_each(target, corrector)
end
end
Expand All @@ -118,14 +120,15 @@ def register_kv_with_block_pass_offense(node, target, method)
return unless (parent_receiver = node.parent.receiver.receiver)
return if allowed_receiver?(parent_receiver)

range = target.loc.selector.with(end_pos: node.parent.loc.selector.end_pos)
add_offense(range, message: format_message(method)) do |corrector|
range = target.loc.selector.join(node.parent.loc.selector.end)

add_offense(range, message: format_message(method, range.source)) do |corrector|
corrector.replace(range, "each_#{method[0..-2]}")
end
end

def format_message(method_name)
format(MSG, prefer: "each_#{method_name[0..-2]}", current: "#{method_name}.each")
def format_message(method_name, current)
format(MSG, prefer: "each_#{method_name[0..-2]}", current: current)
end

def check_argument(variable)
Expand All @@ -148,7 +151,7 @@ def correct_key_value_each(node, corrector)
name = "each_#{node.receiver.method_name.to_s.chop}"
return correct_implicit(node, corrector, name) unless receiver

new_source = receiver.source + ".#{name}"
new_source = receiver.source + "#{node.loc.dot.source}#{name}"
corrector.replace(node, new_source)
end

Expand Down
55 changes: 55 additions & 0 deletions spec/rubocop/cop/style/hash_each_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@
RUBY
end

it 'registers offense, autocorrects `foo&.keys&.each` to `foo&.each_key`' do
expect_offense(<<~RUBY)
foo&.keys&.each { |k| p k }
^^^^^^^^^^ Use `each_key` instead of `keys&.each`.
RUBY

expect_correction(<<~RUBY)
foo&.each_key { |k| p k }
RUBY
end

it 'registers offense, autocorrects foo#values.each to foo#each_value' do
expect_offense(<<~RUBY)
foo.values.each { |v| p v }
Expand All @@ -25,6 +36,17 @@
RUBY
end

it 'registers offense, autocorrects `foo&.values&.each` to `foo&.each_value`' do
expect_offense(<<~RUBY)
foo&.values&.each { |v| p v }
^^^^^^^^^^^^ Use `each_value` instead of `values&.each`.
RUBY

expect_correction(<<~RUBY)
foo&.each_value { |v| p v }
RUBY
end

it 'registers offense, autocorrects foo#keys.each to foo#each_key with a symbol proc argument' do
expect_offense(<<~RUBY)
foo.keys.each(&:bar)
Expand Down Expand Up @@ -70,6 +92,17 @@
RUBY
end

it 'registers an offense when the value block argument of `Enumerable#each` method with safe navigation call is unused' do
expect_offense(<<~RUBY)
foo&.each { |k, unused_value| do_something(k) }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `each_key` instead of `each` and remove the unused `unused_value` block argument.
RUBY

expect_correction(<<~RUBY)
foo&.each_key { |k| do_something(k) }
RUBY
end

it 'registers an offense when the key block argument of `Enumerable#each` method is unused' do
expect_offense(<<~RUBY)
foo.each { |unused_key, v| do_something(v) }
Expand Down Expand Up @@ -137,6 +170,17 @@
RUBY
end

it 'registers offense, autocorrects `{}&.keys&.each` to `{}&.each_key` with a symbol proc argument' do
expect_offense(<<~RUBY)
{}&.keys&.each(&:bar)
^^^^^^^^^^ Use `each_key` instead of `keys&.each`.
RUBY

expect_correction(<<~RUBY)
{}&.each_key(&:bar)
RUBY
end

it 'registers offense, autocorrects {}#values.each to {}#each_value with a symbol proc argument' do
expect_offense(<<~RUBY)
{}.values.each(&:bar)
Expand All @@ -148,6 +192,17 @@
RUBY
end

it 'registers offense, autocorrects `{}&.values.each` to `{}&.each_value` with a symbol proc argument' do
expect_offense(<<~RUBY)
{}&.values&.each(&:bar)
^^^^^^^^^^^^ Use `each_value` instead of `values&.each`.
RUBY

expect_correction(<<~RUBY)
{}&.each_value(&:bar)
RUBY
end

it 'does not register an offense for {}#each_key' do
expect_no_offenses('{}.each_key { |k| p k }')
end
Expand Down

0 comments on commit c8d2d77

Please sign in to comment.