Merge pull request #179 from ghiculescu/perf-sum-empty-array

`Performance/Sum` should avoid, or warn about, empty arrays

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Changes
 
 * [#170](https://github.com/rubocop-hq/rubocop-performance/pull/170): Extend `Performance/Sum` to register an offense for `map { ... }.sum`. ([@eugeneius][])
+* [#179](https://github.com/rubocop-hq/rubocop-performance/pull/179): Change `Performance/Sum` to warn about empty arrays, and not register an offense on empty array literals. ([@ghiculescu][])
 
 ## 1.8.1 (2020-09-19)
 
@@ -174,3 +175,4 @@
 [@siegfault]: https://github.com/siegfault
 [@fatkodima]: https://github.com/fatkodima
 [@dvandersluis]: https://github.com/dvandersluis
+[@ghiculescu]: https://github.com/ghiculescu

lib/rubocop/cop/performance/sum.rb

@@ -26,6 +26,8 @@ class Sum < Base
         extend AutoCorrector
 
         MSG = 'Use `%<good_method>s` instead of `%<bad_method>s`.'
+        MSG_IF_NO_INIT_VALUE =
+          'Use `%<good_method>s` instead of `%<bad_method>s`, unless calling `%<bad_method>s` on an empty array.'
 
         def_node_matcher :sum_candidate?, <<~PATTERN
           (send _ ${:inject :reduce} $_init ? ${(sym :+) (block_pass (sym :+))})
@@ -53,15 +55,39 @@ class Sum < Base
         alias elem_plus_acc? acc_plus_elem?
 
         def on_send(node)
+          return if empty_array_literal?(node)
+
+          handle_sum_candidate(node)
+          handle_sum_map_candidate(node)
+        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(range, message: message) do |corrector|
+                autocorrect(corrector, init, range)
+              end
+            end
+          end
+        end
+
+        private
+
+        def handle_sum_candidate(node)
           sum_candidate?(node) do |method, init, operation|
             range = sum_method_range(node)
-            message = build_method_message(method, init, operation)
+            message = build_method_message(node, method, init, operation)
 
             add_offense(range, message: message) do |corrector|
               autocorrect(corrector, init, range)
             end
           end
+        end
 
+        def handle_sum_map_candidate(node)
           sum_map_candidate?(node) do |map, init|
             next if node.block_literal? || node.block_argument?
 
@@ -73,20 +99,15 @@ def on_send(node)
           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(range, message: message) do |corrector|
-                autocorrect(corrector, init, range)
-              end
-            end
-          end
+        def empty_array_literal?(node)
+          receiver = node.children.first
+          array_literal?(node) && receiver && receiver.children.empty?
         end
 
-        private
+        def array_literal?(node)
+          receiver = node.children.first
+          receiver&.literal? && receiver&.array_type?
+        end
 
         def autocorrect(corrector, init, range)
           return if init.empty?
@@ -119,10 +140,15 @@ 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, operation)
+        def build_method_message(node, method, init, operation)
           good_method = build_good_method(init)
           bad_method = build_method_bad_method(init, method, operation)
-          format(MSG, good_method: good_method, bad_method: bad_method)
+          msg = if init.empty? && !array_literal?(node)
+                  MSG_IF_NO_INIT_VALUE
+                else
+                  MSG
+                end
+          format(msg, good_method: good_method, bad_method: bad_method)
         end
 
         def build_sum_map_message(method, init)

spec/rubocop/cop/performance/sum_spec.rb

@@ -73,7 +73,7 @@
     it 'does not autocorrect `:+` when initial value is not provided' do
       expect_offense(<<~RUBY, method: method)
         array.#{method}(:+)
-              ^{method}^^^^ Use `sum` instead of `#{method}(:+)`.
+              ^{method}^^^^ Use `sum` instead of `#{method}(:+)`, unless calling `#{method}(:+)` on an empty array.
       RUBY
 
       expect_no_corrections
@@ -93,12 +93,28 @@
     it 'does not autocorrect `&:+` when initial value is not provided' do
       expect_offense(<<~RUBY, method: method)
         array.#{method}(&:+)
-              ^{method}^^^^^ Use `sum` instead of `#{method}(&:+)`.
+              ^{method}^^^^^ Use `sum` instead of `#{method}(&:+)`, unless calling `#{method}(&:+)` on an empty array.
       RUBY
 
       expect_no_corrections
     end
 
+    # ideally it would autocorrect to `[1, 2, 3].sum`
+    it 'registers an offense but does not autocorrect on array literals' do
+      expect_offense(<<~RUBY, method: method)
+        [1, 2, 3].#{method}(:+)
+                  ^{method}^^^^ Use `sum` instead of `#{method}(:+)`.
+      RUBY
+
+      expect_no_corrections
+    end
+
+    it 'does not register an offense when the array is empty' do
+      expect_no_offenses(<<~RUBY, method: method)
+        [].#{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 }