Skip to content

Commit

Permalink
Merge pull request #402 from koic/make_performance_sum_aware_of_safe_…
Browse files Browse the repository at this point in the history
…navigation_operator

[Fix #401] Make `Performance/Sum` aware of safe navigation operator
  • Loading branch information
koic authored Dec 3, 2023
2 parents efa8759 + f130376 commit 461168f
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#401](https://github.com/rubocop/rubocop-performance/issues/401): Make `Performance/Sum` aware of safe navigation operator. ([@koic][])
20 changes: 11 additions & 9 deletions lib/rubocop/cop/performance/sum.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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|
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down
44 changes: 44 additions & 0 deletions spec/rubocop/cop/performance/sum_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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 }
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 461168f

Please sign in to comment.