Mark Performance/Sum auto-correction as unsafe and extend documenta…

…tion

The documentaion of `Performance/Sum` talked about setting
`SafeAutoCorrect: true` for the cop, but Rubocop complained:

```
Warning: Performance/Sum does not support SafeAutoCorrect parameter.

Supported parameters are:

  - Enabled

```

so we add the `SafeAutoCorrect` option to the cop's defaults
and thereby mark it as (partially) unsafe.

This is NOT a change in behavior: The cop will still perform all
safe auto-corrections when using `-a`, and also (by default)
perform all unsafe auto-corrections when using `-A`.

Furthermore we extend the documentation in order to make it even more
clear in _which_ cases auto-correction is unsafe, and _why_.

CHANGELOG.md

@@ -312,3 +312,4 @@
 [@ghiculescu]: https://github.com/ghiculescu
 [@mfbmina]: https://github.com/mfbmina
 [@mvz]: https://github.com/mvz
+[@leoarnold]: https://github.com/leoarnold

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][])

config/default.yml

@@ -330,9 +330,10 @@ 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'
+  VersionAdded: '1.13'
 
 Performance/TimesMap:
   Description: 'Checks for .times.map calls.'

docs/modules/ROOT/pages/cops_performance.adoc

@@ -2013,46 +2013,58 @@ This cop identifies places where `gsub` can be replaced by
 
 | Pending
 | Yes
-| Yes
-| 1.8
+| Yes (Unsafe)
+| 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
 
 [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)

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
       #   # 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)

spec/rubocop/cop/performance/sum_spec.rb

@@ -110,43 +110,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, &:+)
@@ -158,23 +121,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
 
-    # ideally it would autocorrect to `[1, 2, 3].sum`
-    it 'registers an offense but does not autocorrect on array literals' 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_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
+
+    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