From f1303764b58cc4ed8014ff4748457e9f352f3b6a Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 2 Dec 2023 02:25:50 +0900 Subject: [PATCH] [Fix #401] Make `Performance/Sum` aware of safe navigation operator Fixes #401. This PR makes `Performance/Sum` aware of safe navigation operator. --- ...e_sum_aware_of_safe_navigation_operator.md | 1 + lib/rubocop/cop/performance/sum.rb | 20 +++++---- spec/rubocop/cop/performance/sum_spec.rb | 44 +++++++++++++++++++ 3 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 changelog/fix_make_performance_sum_aware_of_safe_navigation_operator.md diff --git a/changelog/fix_make_performance_sum_aware_of_safe_navigation_operator.md b/changelog/fix_make_performance_sum_aware_of_safe_navigation_operator.md new file mode 100644 index 0000000000..fd6fe79f55 --- /dev/null +++ b/changelog/fix_make_performance_sum_aware_of_safe_navigation_operator.md @@ -0,0 +1 @@ +* [#401](https://github.com/rubocop/rubocop-performance/issues/401): Make `Performance/Sum` aware of safe navigation operator. ([@koic][]) diff --git a/lib/rubocop/cop/performance/sum.rb b/lib/rubocop/cop/performance/sum.rb index a9cc9ab17e..8662081e0d 100644 --- a/lib/rubocop/cop/performance/sum.rb +++ b/lib/rubocop/cop/performance/sum.rb @@ -80,21 +80,21 @@ class Sum < Base RESTRICT_ON_SEND = %i[inject reduce sum].freeze def_node_matcher :sum_candidate?, <<~PATTERN - (send _ ${:inject :reduce} $_init ? ${(sym :+) (block_pass (sym :+))}) + (call _ ${:inject :reduce} $_init ? ${(sym :+) (block_pass (sym :+))}) PATTERN def_node_matcher :sum_map_candidate?, <<~PATTERN - (send + (call { - (block $(send _ {:map :collect}) ...) - $(send _ {:map :collect} (block_pass _)) + (block $(call _ {:map :collect}) ...) + $(call _ {:map :collect} (block_pass _)) } :sum $_init ?) PATTERN def_node_matcher :sum_with_block_candidate?, <<~PATTERN (block - $(send _ {:inject :reduce} $_init ?) + $(call _ {:inject :reduce} $_init ?) (args (arg $_acc) (arg $_elem)) $send) PATTERN @@ -110,6 +110,7 @@ def on_send(node) handle_sum_candidate(node) handle_sum_map_candidate(node) end + alias on_csend on_send def on_block(node) sum_with_block_candidate?(node) do |send, init, var_acc, var_elem, body| @@ -143,7 +144,7 @@ def handle_sum_map_candidate(node) sum_map_candidate?(node) do |map, init| next if node.block_literal? || node.block_argument? - message = build_sum_map_message(map.method_name, init) + message = build_sum_map_message(map, init) add_offense(sum_map_range(map, node), message: message) do |corrector| autocorrect_sum_map(corrector, node, map, init) @@ -178,7 +179,7 @@ def autocorrect_sum_map(corrector, sum, map, init) corrector.remove(sum_range) - dot = '.' if map.receiver + dot = map.loc.dot&.source || '' corrector.replace(map_range, "#{dot}#{replacement}") end @@ -205,10 +206,11 @@ def build_method_message(node, method, init, operation) format(msg, good_method: good_method, bad_method: bad_method) end - def build_sum_map_message(method, init) + def build_sum_map_message(send_node, init) sum_method = build_good_method(init) good_method = "#{sum_method} { ... }" - bad_method = "#{method} { ... }.#{sum_method}" + dot = send_node.loc.dot&.source || '.' + bad_method = "#{send_node.method_name} { ... }#{dot}#{sum_method}" format(MSG, good_method: good_method, bad_method: bad_method) end diff --git a/spec/rubocop/cop/performance/sum_spec.rb b/spec/rubocop/cop/performance/sum_spec.rb index 1cc9277c12..bc6549e561 100644 --- a/spec/rubocop/cop/performance/sum_spec.rb +++ b/spec/rubocop/cop/performance/sum_spec.rb @@ -13,6 +13,17 @@ RUBY end + it "registers an offense and corrects when using `array&.#{method}(10, :+)`" do + expect_offense(<<~RUBY, method: method) + array&.#{method}(10, :+) + ^{method}^^^^^^^^ Use `sum(10)` instead of `#{method}(10, :+)`. + RUBY + + expect_correction(<<~RUBY) + array&.sum(10) + RUBY + end + it "registers an offense and corrects when using `array.#{method}(10) { |acc, elem| acc + elem }`" do expect_offense(<<~RUBY, method: method) array.#{method}(10) { |acc, elem| acc + elem } @@ -24,6 +35,17 @@ RUBY end + it "registers an offense and corrects when using `array&.#{method}(10) { |acc, elem| acc + elem }`" do + expect_offense(<<~RUBY, method: method) + array&.#{method}(10) { |acc, elem| acc + elem } + ^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `sum(10)` instead of `#{method}(10) { |acc, elem| acc + elem }`. + RUBY + + expect_correction(<<~RUBY) + array&.sum(10) + RUBY + end + it "registers an offense and corrects when using `array.#{method}(10) { |acc, elem| elem + acc }`" do expect_offense(<<~RUBY, method: method) array.#{method}(10) { |acc, elem| elem + acc } @@ -342,6 +364,17 @@ RUBY end + it "registers an offense and corrects when using `array&.#{method} { |elem| elem ** 2 }&.sum`" do + expect_offense(<<~RUBY, method: method) + array&.%{method} { |elem| elem ** 2 }&.sum + ^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `sum { ... }` instead of `%{method} { ... }&.sum`. + RUBY + + expect_correction(<<~RUBY) + array&.sum { |elem| elem ** 2 } + RUBY + end + it "registers an offense and corrects when using `array.#{method}(&:count).sum`" do expect_offense(<<~RUBY, method: method) array.%{method}(&:count).sum @@ -353,6 +386,17 @@ RUBY end + it "registers an offense and corrects when using `array&.#{method}(&:count)&.sum`" do + expect_offense(<<~RUBY, method: method) + array&.%{method}(&:count)&.sum + ^{method}^^^^^^^^^^^^^^ Use `sum { ... }` instead of `%{method} { ... }&.sum`. + RUBY + + expect_correction(<<~RUBY) + array&.sum(&:count) + RUBY + end + it "registers an offense and corrects when using `#{method}(&:count).sum`" do expect_offense(<<~RUBY, method: method) %{method}(&:count).sum