Skip to content

Commit

Permalink
Add new Performance/Sum cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Jun 9, 2020
1 parent ddd44a0 commit 3c1e47a
Show file tree
Hide file tree
Showing 7 changed files with 236 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### New features

* [#137](https://github.com/rubocop-hq/rubocop-performance/pull/137): Add new `Performance/Sum` cop. ([@fatkodima][])
* [#132](https://github.com/rubocop-hq/rubocop-performance/issues/132): Add new `Performance/RedundantSortBlock` cop. ([@fatkodima][])
* [#125](https://github.com/rubocop-hq/rubocop-performance/pull/125): Support `Array()` and `Hash()` methods for `Performance/Size` cop. ([@fatkodima][])
* [#124](https://github.com/rubocop-hq/rubocop-performance/pull/124): Add new `Performance/Squeeze` cop. ([@fatkodima][])
Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ Performance/StringReplacement:
Enabled: true
VersionAdded: '0.33'

Performance/Sum:
Description: 'Use `sum` instead of a custom array summation.'
Enabled: true
VersionAdded: '1.7'

Performance/TimesMap:
Description: 'Checks for .times.map calls.'
AutoCorrect: false
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
* xref:cops_performance.adoc#performancestartwith[Performance/StartWith]
* xref:cops_performance.adoc#performancestringinclude[Performance/StringInclude]
* xref:cops_performance.adoc#performancestringreplacement[Performance/StringReplacement]
* xref:cops_performance.adoc#performancesum[Performance/Sum]
* xref:cops_performance.adoc#performancetimesmap[Performance/TimesMap]
* xref:cops_performance.adoc#performanceunfreezestring[Performance/UnfreezeString]
* xref:cops_performance.adoc#performanceuridefaultparser[Performance/UriDefaultParser]
Expand Down
30 changes: 30 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1440,6 +1440,36 @@ This cop identifies places where `gsub` can be replaced by

* https://github.com/JuanitoFatas/fast-ruby#stringgsub-vs-stringtr-code

== Performance/Sum

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Enabled
| Yes
| Yes
| 1.7
| -
|===

This cop identifies places where custom array summation
can be replaced by `sum` method.

=== Examples

[source,ruby]
----
# bad
[1, 2, 3].inject(:+)
[1, 2, 3].reduce(10, :+)
[1, 2, 3].reduce { |acc, elem| acc + elem }
# good
[1, 2, 3].sum
[1, 2, 3].sum(10)
[1, 2, 3].sum
----

== Performance/TimesMap

|===
Expand Down
135 changes: 135 additions & 0 deletions lib/rubocop/cop/performance/sum.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Performance
# This cop identifies places where custom array summation
# can be replaced by `sum` method.
#
# @example
# # bad
# [1, 2, 3].inject(:+)
# [1, 2, 3].reduce(10, :+)
# [1, 2, 3].reduce { |acc, elem| acc + elem }
#
# # good
# [1, 2, 3].sum
# [1, 2, 3].sum(10)
# [1, 2, 3].sum
#
class Sum < Cop
include RangeHelp

MSG = 'Use `%<good_method>s` instead of `%<bad_method>s`.'

def_node_matcher :sum_candidate?, <<~PATTERN
(send _ ${:inject :reduce} $_init ? (sym :+))
PATTERN

def_node_matcher :sum_with_block_candidate?, <<~PATTERN
(block
$(send _ {:inject :reduce} $_init ?)
(args (arg $_acc) (arg $_elem))
$send)
PATTERN

def_node_matcher :acc_plus_elem?, <<~PATTERN
(send (lvar %1) :+ (lvar %2))
PATTERN
alias elem_plus_acc? acc_plus_elem?

def on_send(node)
sum_candidate?(node) do |method, init|
range = sum_method_range(node)
message = build_method_message(method, init)

add_offense(node, location: range, message: message)
end
end

def on_block(node)
sum_with_block_candidate?(node) do |send, init, var_acc, var_elem, body|
if acc_plus_elem?(body, var_acc, var_elem) || elem_plus_acc?(body, var_elem, var_acc)
range = sum_block_range(send, node)
message = build_block_message(send, init, var_acc, var_elem, body)

add_offense(node, location: range, message: message)
end
end
end

def autocorrect(node)
if (matches = sum_candidate?(node))
_, init = *matches
range = sum_method_range(node)
elsif (matches = sum_with_block_candidate?(node))
send, init, = matches
range = sum_block_range(send, node)
else
return
end

unless init.empty?
lambda do |corrector|
replacement = build_good_method(init)
corrector.replace(range, replacement)
end
end
end

private

def sum_method_range(node)
range_between(node.loc.selector.begin_pos, node.loc.end.end_pos)
end

def sum_block_range(send, node)
range_between(send.loc.selector.begin_pos, node.loc.end.end_pos)
end

def build_method_message(method, init)
good_method = build_good_method(init)
bad_method = build_method_bad_method(init, method)
format(MSG, good_method: good_method, bad_method: bad_method)
end

def build_block_message(send, init, var_acc, var_elem, body)
good_method = build_good_method(init)
bad_method = build_block_bad_method(send.method_name, init, var_acc, var_elem, body)
format(MSG, good_method: good_method, bad_method: bad_method)
end

def build_good_method(init)
good_method = 'sum'

unless init.empty?
init = init.first
good_method += "(#{init.source})" if init.source.to_i != 0
end
good_method
end

def build_method_bad_method(init, method)
bad_method = "#{method}("
unless init.empty?
init = init.first
bad_method += "#{init.source}, "
end
bad_method += ':+)'
bad_method
end

def build_block_bad_method(method, init, var_acc, var_elem, body)
bad_method = method.to_s

unless init.empty?
init = init.first
bad_method += "(#{init.source})"
end
bad_method += " { |#{var_acc}, #{var_elem}| #{body.source} }"
bad_method
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
require_relative 'performance/start_with'
require_relative 'performance/string_include'
require_relative 'performance/string_replacement'
require_relative 'performance/sum'
require_relative 'performance/times_map'
require_relative 'performance/unfreeze_string'
require_relative 'performance/uri_default_parser'
Expand Down
63 changes: 63 additions & 0 deletions spec/rubocop/cop/performance/sum_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::Sum do
subject(:cop) { described_class.new }

%i[inject reduce].each do |method|
it "registers an offense and corrects when using `array.#{method}(10, :+)`" do
source = "array.#{method}(10, :+)"
inspect_source(source)

expect(cop.offenses.size).to eq(1)
expect(cop.highlights).to eq(["#{method}(10, :+)"])

new_source = autocorrect_source(source)
expect(new_source).to eq('array.sum(10)')
end

it "registers an offense and corrects when using `array.#{method}(10) { |acc, elem| acc + elem }`" do
source = "array.#{method}(10) { |acc, elem| acc + elem }"
inspect_source(source)

expect(cop.offenses.size).to eq(1)
expect(cop.highlights).to eq(["#{method}(10) { |acc, elem| acc + elem }"])

new_source = autocorrect_source(source)
expect(new_source).to eq('array.sum(10)')
end

it "registers an offense and corrects when using `array.#{method}(10) { |acc, elem| elem + acc }`" do
source = "array.#{method}(10) { |acc, elem| elem + acc }"
inspect_source(source)

expect(cop.offenses.size).to eq(1)
expect(cop.highlights).to eq(["#{method}(10) { |acc, elem| elem + acc }"])

new_source = autocorrect_source(source)
expect(new_source).to eq('array.sum(10)')
end

it 'does not autocorrect when initial value is not provided' do
source = "array.#{method}(:+)"
inspect_source(source)

expect(cop.offenses.size).to eq(1)
expect(cop.highlights).to eq(["#{method}(:+)"])

new_source = autocorrect_source(source)
expect(new_source).to eq(source)
end

it 'does not register an offense when block does not implement summation' do
source = "array.#{method} { |acc, elem| elem * 2 }"
inspect_source(source)
expect(cop.offenses.size).to eq(0)
end

it 'does not register an offense when using `sum`' do
expect_no_offenses(<<~RUBY)
array.sum
RUBY
end
end
end

0 comments on commit 3c1e47a

Please sign in to comment.