Skip to content

Commit

Permalink
[Fix rubocop#204] Add Performance/Sum option to ignore potential fa…
Browse files Browse the repository at this point in the history
…lse positives

Some codebases may contain non-numeric classes which implement a `:+` method.
If `a`, `b` and `c` are objects of such a class, then certain corrections
do not apply:

```ruby
[a, b].reduce(c, :+) # is equivalent to
[a, b].sum(c)

[a, b].map(&:to_i).sum # is equivalent to
[a, b].sum(&:to_i)

[a, b].reduce(:+) # works
[a, b].sum # raises TypeError
```

For users who wish to only register offenses where auto-correction
is unproblematic, we add the option `OnlySumOrWithInitialValue`
to the `Performance/Sum` cop.

The option is disabled by default, so standard behavior is preserved,
and can be activated by setting `OnlySumOrWithInitialValue: true`.
  • Loading branch information
leoarnold committed Nov 16, 2021
1 parent 51409ce commit 0bc2b43
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -312,3 +312,4 @@
[@ghiculescu]: https://github.com/ghiculescu
[@mfbmina]: https://github.com/mfbmina
[@mvz]: https://github.com/mvz
[@leoarnold]: https://github.com/leoarnold
Empty file added changelog/.gitkeep
Empty file.
1 change: 1 addition & 0 deletions changelog/new_add_performancesum_option_to_ignore.md
@@ -0,0 +1 @@
* [#204](https://github.com/rubocop/rubocop-performance/issues/204): Add `Performance/Sum` option to ignore potential false positives. ([@leoarnold][])
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -333,6 +333,7 @@ Performance/Sum:
Reference: 'https://blog.bigbinary.com/2016/11/02/ruby-2-4-introduces-enumerable-sum.html'
Enabled: 'pending'
VersionAdded: '1.8'
OnlySumOrWithInitialValue: false

Performance/TimesMap:
Description: 'Checks for .times.map calls.'
Expand Down
15 changes: 14 additions & 1 deletion lib/rubocop/cop/performance/sum.rb
Expand Up @@ -30,7 +30,7 @@ module Performance
# Please note that the auto-correction command line option will be changed from
# `rubocop -a` to `rubocop -A`, which includes unsafe auto-correction.
#
# @example
# @example OnlySumOrWithInitialValue: false (default)
# # bad
# [1, 2, 3].inject(:+) # These bad cases with no initial value are unsafe and
# [1, 2, 3].inject(&:+) # will not be auto-correced by default. If you want to
Expand All @@ -45,6 +45,17 @@ module Performance
# [1, 2, 3].sum { |elem| elem ** 2 }
# [1, 2, 3].sum(10, &:count)
#
# @example OnlySumOrWithInitialValue: true
# # bad
# [1, 2, 3].reduce(10, :+)
# [1, 2, 3].map { |elem| elem ** 2 }.sum
# [1, 2, 3].collect(&:count).sum(10)
#
# # good
# [1, 2, 3].sum(10)
# [1, 2, 3].sum { |elem| elem ** 2 }
# [1, 2, 3].sum(10, &:count)
#
class Sum < Base
include RangeHelp
extend AutoCorrector
Expand Down Expand Up @@ -103,6 +114,8 @@ def on_block(node)

def handle_sum_candidate(node)
sum_candidate?(node) do |method, init, operation|
next if cop_config['OnlySumOrWithInitialValue'] && init.empty?

range = sum_method_range(node)
message = build_method_message(node, method, init, operation)

Expand Down
193 changes: 193 additions & 0 deletions spec/rubocop/cop/performance/sum_spec.rb
Expand Up @@ -79,6 +79,128 @@
RUBY
end

context 'when `OnlySumOrWithInitialValue: true`' do
let(:cop_config) { { 'OnlySumOrWithInitialValue' => true } }

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 }
^{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 }
^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `sum(10)` instead of `#{method}(10) { |acc, elem| elem + acc }`.
RUBY

expect_correction(<<~RUBY)
array.sum(10)
RUBY
end

it "registers an offense and corrects when using `array.#{method}(0, :+)`" do
expect_offense(<<~RUBY, method: method)
array.#{method}(0, :+)
^{method}^^^^^^^ Use `sum` instead of `#{method}(0, :+)`.
RUBY

expect_correction(<<~RUBY)
array.sum
RUBY
end

it "registers an offense and corrects when using `array.#{method} 0, :+`" do
expect_offense(<<~RUBY, method: method)
array.#{method} 0, :+
^{method}^^^^^^ Use `sum` instead of `#{method}(0, :+)`.
RUBY

expect_correction(<<~RUBY)
array.sum
RUBY
end

it "registers an offense and corrects when using `array.#{method}(0.0, :+)`" do
expect_offense(<<~RUBY, method: method)
array.#{method}(0.0, :+)
^{method}^^^^^^^^^ Use `sum(0.0)` instead of `#{method}(0.0, :+)`.
RUBY

expect_correction(<<~RUBY)
array.sum(0.0)
RUBY
end

it "registers an offense and corrects when using `array.#{method}(init, :+)`" do
expect_offense(<<~RUBY, method: method)
array.#{method}(init, :+)
^{method}^^^^^^^^^^ Use `sum(init)` instead of `#{method}(init, :+)`.
RUBY

expect_correction(<<~RUBY)
array.sum(init)
RUBY
end

it "registers an offense and corrects when using `array.#{method}(0, &:+)`" do
expect_offense(<<~RUBY, method: method)
array.#{method}(0, &:+)
^{method}^^^^^^^^ Use `sum` instead of `#{method}(0, &:+)`.
RUBY

expect_correction(<<~RUBY)
array.sum
RUBY
end

it 'does not register an offense on `&:+` when initial value is not provided' do
expect_no_offenses(<<~RUBY)
array.#{method}(&:+)
RUBY
end

it 'does not register an offense on array literals' do
expect_no_offenses(<<~RUBY)
[1, 2, 3].#{method}(:+)
RUBY
end

it 'does not register an offense when the array is empty' do
expect_no_offenses(<<~RUBY)
[].#{method}(:+)
RUBY
end

it 'does not register an offense when block does not implement summation' do
expect_no_offenses(<<~RUBY)
array.#{method} { |acc, elem| elem * 2 }
RUBY
end

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

context 'when `SafeAutoCorrect: true' do
let(:cop_config) { { 'SafeAutoCorrect' => true } }

Expand Down Expand Up @@ -263,5 +385,76 @@
array.#{method}(&:count).sum(&:count)
RUBY
end

context 'when `SafeAutoCorrect: true' do
let(:cop_config) { { 'SafeAutoCorrect' => true } }

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
^{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
^{method}^^^^^^^^^^^^^ Use `sum { ... }` instead of `%{method} { ... }.sum`.
RUBY

expect_correction(<<~RUBY)
sum(&:count)
RUBY
end

it "registers an offense and corrects when using `array.#{method}(&:count).sum(10)`" do
expect_offense(<<~RUBY, method: method)
array.%{method}(&:count).sum(10)
^{method}^^^^^^^^^^^^^^^^^ Use `sum(10) { ... }` instead of `%{method} { ... }.sum(10)`.
RUBY

expect_correction(<<~RUBY)
array.sum(10, &:count)
RUBY
end

it "registers an offense and corrects when using `array.#{method} { elem ** 2 }.sum(10)`" do
expect_offense(<<~RUBY, method: method)
array.%{method} { |elem| elem ** 2 }.sum(10)
^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `sum(10) { ... }` instead of `%{method} { ... }.sum(10)`.
RUBY

expect_correction(<<~RUBY)
array.sum(10) { |elem| elem ** 2 }
RUBY
end

it "does not register an offense when using `array.#{method}(&:count).sum { |elem| elem ** 2 }`" do
expect_no_offenses(<<~RUBY)
array.#{method}(&:count).sum { |elem| elem ** 2 }
RUBY
end

it "does not register an offense when using `array.#{method}(&:count).sum(&:count)`" do
expect_no_offenses(<<~RUBY)
array.#{method}(&:count).sum(&:count)
RUBY
end
end
end
end

0 comments on commit 0bc2b43

Please sign in to comment.