diff --git a/changelog/change_mark_performancesum_autocorrection_as.md b/changelog/change_mark_performancesum_autocorrection_as.md new file mode 100644 index 0000000000..ed047cf74b --- /dev/null +++ b/changelog/change_mark_performancesum_autocorrection_as.md @@ -0,0 +1 @@ +* [#270](https://github.com/rubocop/rubocop-performance/pull/270): Mark `Performance/Sum` auto-correction as unsafe and extend documentation. ([@leoarnold][]) diff --git a/config/default.yml b/config/default.yml index af19a0cf67..4cc182471f 100644 --- a/config/default.yml +++ b/config/default.yml @@ -330,9 +330,11 @@ Performance/StringReplacement: Performance/Sum: Description: 'Use `sum` instead of a custom array summation.' + SafeAutoCorrect: false Reference: 'https://blog.bigbinary.com/2016/11/02/ruby-2-4-introduces-enumerable-sum.html' Enabled: 'pending' VersionAdded: '1.8' + VersionChanged: '1.13' OnlySumOrWithInitialValue: false Performance/TimesMap: diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index 83f3b1e54a..de46ffeea0 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -2013,37 +2013,49 @@ This cop identifies places where `gsub` can be replaced by | Pending | Yes -| Yes +| Yes (Unsafe) | 1.8 -| - +| 1.13 |=== This cop identifies places where custom code finding the sum of elements in some Enumerable object can be replaced by `Enumerable#sum` method. -This cop can change auto-correction scope depending on the value of -`SafeAutoCorrect`. -Its auto-correction is marked as safe by default (`SafeAutoCorrect: true`) -to prevent `TypeError` in auto-correced code when initial value is not -specified as shown below: +=== Safety + +Auto-corrections are unproblematic wherever an initial value is provided explicitly: [source,ruby] ---- -['a', 'b'].sum # => (String can't be coerced into Integer) ----- +[1, 2, 3].reduce(4, :+) # => 10 +[1, 2, 3].sum(4) # => 10 -Therefore if initial value is not specified, unsafe auto-corrected will not occur. +[].reduce(4, :+) # => 4 +[].sum(4) # => 4 +---- -If you always want to enable auto-correction, you can set `SafeAutoCorrect: false`. +This also holds true for non-numeric types which implement a `:+` method: -[source,yaml] +[source,ruby] +---- +['l', 'o'].reduce('Hel', :+) # => "Hello" +['l', 'o'].sum('Hel') # => "Hello" ---- -Performance/Sum: - SafeAutoCorrect: false + +When no initial value is provided though, `Enumerable#reduce` will pick the first enumerated value +as initial value and successively add all following values to it, whereas +`Enumerable#sum` will set an initial value of `0` (`Integer`) which can lead to a `TypeError`: + +[source,ruby] ---- +[].reduce(:+) # => nil +[1, 2, 3].reduce(:+) # => 6 +['H', 'e', 'l', 'l', 'o'].reduce(:+) # => "Hello" -Please note that the auto-correction command line option will be changed from -`rubocop -a` to `rubocop -A`, which includes unsafe auto-correction. +[].sum # => 0 +[1, 2, 3].sum # => 6 +['H', 'e', 'l', 'l', 'o'].sum # => in `+': String can't be coerced into Integer (TypeError) +---- === Examples @@ -2052,9 +2064,9 @@ Please note that the auto-correction command line option will be changed from [source,ruby] ---- # 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 -[1, 2, 3].reduce { |acc, elem| acc + elem } # auto-corrected, you can set `SafeAutoCorrect: false`. +[1, 2, 3].inject(:+) # Auto-corrections for cases without initial value are unsafe +[1, 2, 3].inject(&:+) # and will only be performed when using the `-A` option. +[1, 2, 3].reduce { |acc, elem| acc + elem } # They can be prohibited completely using `SafeAutoCorrect: true`. [1, 2, 3].reduce(10, :+) [1, 2, 3].map { |elem| elem ** 2 }.sum [1, 2, 3].collect(&:count).sum(10) diff --git a/lib/rubocop/cop/performance/sum.rb b/lib/rubocop/cop/performance/sum.rb index cc346355ef..ebd9c08227 100644 --- a/lib/rubocop/cop/performance/sum.rb +++ b/lib/rubocop/cop/performance/sum.rb @@ -6,35 +6,46 @@ 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. # - # This cop can change auto-correction scope depending on the value of - # `SafeAutoCorrect`. - # Its auto-correction is marked as safe by default (`SafeAutoCorrect: true`) - # to prevent `TypeError` in auto-correced code when initial value is not - # specified as shown below: + # @safety + # Auto-corrections are unproblematic wherever an initial value is provided explicitly: # - # [source,ruby] - # ---- - # ['a', 'b'].sum # => (String can't be coerced into Integer) - # ---- + # [source,ruby] + # ---- + # [1, 2, 3].reduce(4, :+) # => 10 + # [1, 2, 3].sum(4) # => 10 # - # Therefore if initial value is not specified, unsafe auto-corrected will not occur. + # [].reduce(4, :+) # => 4 + # [].sum(4) # => 4 + # ---- # - # If you always want to enable auto-correction, you can set `SafeAutoCorrect: false`. + # This also holds true for non-numeric types which implement a `:+` method: # - # [source,yaml] - # ---- - # Performance/Sum: - # SafeAutoCorrect: false - # ---- + # [source,ruby] + # ---- + # ['l', 'o'].reduce('Hel', :+) # => "Hello" + # ['l', 'o'].sum('Hel') # => "Hello" + # ---- # - # Please note that the auto-correction command line option will be changed from - # `rubocop -a` to `rubocop -A`, which includes unsafe auto-correction. + # When no initial value is provided though, `Enumerable#reduce` will pick the first enumerated value + # as initial value and successively add all following values to it, whereas + # `Enumerable#sum` will set an initial value of `0` (`Integer`) which can lead to a `TypeError`: + # + # [source,ruby] + # ---- + # [].reduce(:+) # => nil + # [1, 2, 3].reduce(:+) # => 6 + # ['H', 'e', 'l', 'l', 'o'].reduce(:+) # => "Hello" + # + # [].sum # => 0 + # [1, 2, 3].sum # => 6 + # ['H', 'e', 'l', 'l', 'o'].sum # => in `+': String can't be coerced into Integer (TypeError) + # ---- # # @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 - # [1, 2, 3].reduce { |acc, elem| acc + elem } # auto-corrected, you can set `SafeAutoCorrect: false`. + # [1, 2, 3].inject(:+) # Auto-corrections for cases without initial value are unsafe + # [1, 2, 3].inject(&:+) # and will only be performed when using the `-A` option. + # [1, 2, 3].reduce { |acc, elem| acc + elem } # They can be prohibited completely using `SafeAutoCorrect: true`. # [1, 2, 3].reduce(10, :+) # [1, 2, 3].map { |elem| elem ** 2 }.sum # [1, 2, 3].collect(&:count).sum(10) diff --git a/spec/rubocop/cop/performance/sum_spec.rb b/spec/rubocop/cop/performance/sum_spec.rb index a4deb3f9b4..e1c5bd9cb7 100644 --- a/spec/rubocop/cop/performance/sum_spec.rb +++ b/spec/rubocop/cop/performance/sum_spec.rb @@ -232,43 +232,6 @@ end end - context 'when `SafeAutoCorrect: false' do - let(:cop_config) { { 'SafeAutoCorrect' => false } } - - it 'autocorrects `:+` when initial value is not provided' do - expect_offense(<<~RUBY, method: method) - array.#{method}(:+) - ^{method}^^^^ Use `sum` instead of `#{method}(:+)`, unless calling `#{method}(:+)` on an empty array. - RUBY - - expect_correction(<<~RUBY) - array.sum - RUBY - end - - it 'autocorrects `&:+` when initial value is not provided' do - expect_offense(<<~RUBY, method: method) - array.#{method}(&:+) - ^{method}^^^^^ Use `sum` instead of `#{method}(&:+)`, unless calling `#{method}(&:+)` on an empty array. - RUBY - - expect_correction(<<~RUBY) - array.sum - RUBY - end - - it 'autocorrects `:+` without brackets when initial value is not provided' do - expect_offense(<<~RUBY, method: method) - array.#{method} :+ - ^{method}^^^ Use `sum` instead of `#{method}(:+)`, unless calling `#{method}(:+)` on an empty array. - RUBY - - expect_correction(<<~RUBY) - array.sum - RUBY - end - end - it "registers an offense and corrects when using `array.#{method}(0, &:+)`" do expect_offense(<<~RUBY, method: method) array.#{method}(0, &:+) @@ -280,23 +243,48 @@ RUBY end - it 'does not autocorrect `&:+` when initial value is not provided' do + it 'autocorrects `&:+` when initial value is not provided' do expect_offense(<<~RUBY, method: method) array.#{method}(&:+) ^{method}^^^^^ Use `sum` instead of `#{method}(&:+)`, unless calling `#{method}(&:+)` on an empty array. RUBY - expect_no_corrections + expect_correction(<<~RUBY) + array.sum + RUBY + end + + it 'autocorrects `:+` when initial value is not provided' do + expect_offense(<<~RUBY, method: method) + array.#{method}(:+) + ^{method}^^^^ Use `sum` instead of `#{method}(:+)`, unless calling `#{method}(:+)` on an empty array. + RUBY + + expect_correction(<<~RUBY) + array.sum + RUBY end - # ideally it would autocorrect to `[1, 2, 3].sum` - it 'registers an offense but does not autocorrect on array literals' do + it 'autocorrects `:+` without brackets when initial value is not provided' do + expect_offense(<<~RUBY, method: method) + array.#{method} :+ + ^{method}^^^ Use `sum` instead of `#{method}(:+)`, unless calling `#{method}(:+)` on an empty array. + RUBY + + expect_correction(<<~RUBY) + array.sum + RUBY + end + + it 'autocorrects `:+` on array literals when initial value is not provided' do expect_offense(<<~RUBY, method: method) [1, 2, 3].#{method}(:+) ^{method}^^^^ Use `sum` instead of `#{method}(:+)`. RUBY - expect_no_corrections + expect_correction(<<~RUBY) + [1, 2, 3].sum + RUBY end it 'does not register an offense when the array is empty' do