Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Performance/Sum cop #137

Merged
merged 1 commit into from
Jul 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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][])
* [#141](https://github.com/rubocop-hq/rubocop-performance/pull/141): Add new `Performance/RedundantStringChars` cop. ([@fatkodima][])
* [#127](https://github.com/rubocop-hq/rubocop-performance/pull/127): Add new `Performance/IoReadlines` cop. ([@fatkodima][])
* [#128](https://github.com/rubocop-hq/rubocop-performance/pull/128): Add new `Performance/ReverseFirst` cop. ([@fatkodima][])
Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,12 @@ Performance/StringReplacement:
Enabled: true
VersionAdded: '0.33'

Performance/Sum:
Description: 'Use `sum` instead of a custom array summation.'
Reference: 'https://blog.bigbinary.com/2016/11/02/ruby-2-4-introduces-enumerable-sum.html'
Enabled: 'pending'
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 @@ -36,6 +36,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
34 changes: 34 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,40 @@ 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

| Pending
| Yes
| Yes
| 1.7
| -
|===

This cop identifies places where custom code finding the sum of elements
in some Enumerable object can be replaced by `Enumerable#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
----

=== References

* https://blog.bigbinary.com/2016/11/02/ruby-2-4-introduces-enumerable-sum.html

== 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 code finding the sum of elements
# in some Enumerable object can be replaced by `Enumerable#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

return if init.empty?

lambda do |corrector|
replacement = build_good_method(init)
corrector.replace(range, replacement)
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 @@ -36,6 +36,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