From c8d2d776cf242063af17f3b6fbb12461a3301d7b Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 1 Dec 2023 10:54:55 +0900 Subject: [PATCH] [Fix #12424] Make `Style/HashEachMethods` aware of safe navigation operator Fixes #12424. This PR makes `Style/HashEachMethods` aware of safe navigation operator. --- ...thods_aware_of_safe_navigation_operator.md | 1 + lib/rubocop/cop/style/hash_each_methods.rb | 21 ++++--- .../cop/style/hash_each_methods_spec.rb | 55 +++++++++++++++++++ 3 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 changelog/fix_make_style_hash_each_methods_aware_of_safe_navigation_operator.md diff --git a/changelog/fix_make_style_hash_each_methods_aware_of_safe_navigation_operator.md b/changelog/fix_make_style_hash_each_methods_aware_of_safe_navigation_operator.md new file mode 100644 index 000000000000..65a9b0bd70c9 --- /dev/null +++ b/changelog/fix_make_style_hash_each_methods_aware_of_safe_navigation_operator.md @@ -0,0 +1 @@ +* [#12424](https://github.com/rubocop/rubocop/issues/12424): Make `Style/HashEachMethods` aware of safe navigation operator. ([@koic][]) diff --git a/lib/rubocop/cop/style/hash_each_methods.rb b/lib/rubocop/cop/style/hash_each_methods.rb index 427dc1017e70..c9457dfea922 100644 --- a/lib/rubocop/cop/style/hash_each_methods.rb +++ b/lib/rubocop/cop/style/hash_each_methods.rb @@ -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 @@ -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 @@ -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) @@ -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 diff --git a/spec/rubocop/cop/style/hash_each_methods_spec.rb b/spec/rubocop/cop/style/hash_each_methods_spec.rb index af51fc31fe05..85361851f2eb 100644 --- a/spec/rubocop/cop/style/hash_each_methods_spec.rb +++ b/spec/rubocop/cop/style/hash_each_methods_spec.rb @@ -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 } @@ -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) @@ -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) } @@ -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) @@ -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