Fix incorrect replacement recommendation for Performance/ChainArrayAllocation

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Bug fixes
+
+* [#185](https://github.com/rubocop-hq/rubocop-performance/issues/185): Fix incorrect replacement recommendation for `Performance/ChainArrayAllocation`. ([@fatkodima][])
+
 ## 1.9.0 (2020-11-17)
 
 ### New features

lib/rubocop/cop/performance/chain_array_allocation.rb

@@ -29,38 +29,40 @@ class ChainArrayAllocation < Base
         #   [1,2].first    # => 1
         #   [1,2].first(1) # => [1]
         #
-        RETURN_NEW_ARRAY_WHEN_ARGS = ':first :last :pop :sample :shift '
+        RETURN_NEW_ARRAY_WHEN_ARGS = %i[first last pop sample shift].to_set.freeze
 
         # These methods return a new array only when called without a block.
-        RETURNS_NEW_ARRAY_WHEN_NO_BLOCK = ':zip :product '
+        RETURNS_NEW_ARRAY_WHEN_NO_BLOCK = %i[zip product].to_set.freeze
 
         # These methods ALWAYS return a new array
         # after they're called it's safe to mutate the the resulting array
-        ALWAYS_RETURNS_NEW_ARRAY = ':* :+ :- :collect :compact :drop '\
-                                   ':drop_while :flatten :map :reject ' \
-                                   ':reverse :rotate :select :shuffle :sort ' \
-                                   ':take :take_while :transpose :uniq ' \
-                                   ':values_at :| '
+        ALWAYS_RETURNS_NEW_ARRAY = %i[* + - collect compact drop
+                                      drop_while flatten map reject
+                                      reverse rotate select shuffle sort
+                                      take take_while transpose uniq
+                                      values_at |].to_set.freeze
 
         # These methods have a mutation alternative. For example :collect
         # can be called as :collect!
-        HAS_MUTATION_ALTERNATIVE = ':collect :compact :flatten :map :reject '\
-                                   ':reverse :rotate :select :shuffle :sort '\
-                                   ':uniq '
-        MSG = 'Use unchained `%<method>s!` and `%<second_method>s!` '\
+        HAS_MUTATION_ALTERNATIVE = %i[collect compact flatten map reject
+                                      reverse rotate select shuffle sort uniq].to_set.freeze
+
+        RETURNS_NEW_ARRAY = (ALWAYS_RETURNS_NEW_ARRAY + RETURNS_NEW_ARRAY_WHEN_NO_BLOCK).freeze
+
+        MSG = 'Use unchained `%<method>s` and `%<second_method>s!` '\
               '(followed by `return array` if required) instead of chaining '\
               '`%<method>s...%<second_method>s`.'
 
-        def_node_matcher :flat_map_candidate?, <<~PATTERN
-          {
-            (send (send _ ${#{RETURN_NEW_ARRAY_WHEN_ARGS}} {int lvar ivar cvar gvar}) ${#{HAS_MUTATION_ALTERNATIVE}} $...)
-            (send (block (send _ ${#{ALWAYS_RETURNS_NEW_ARRAY} }) ...) ${#{HAS_MUTATION_ALTERNATIVE}} $...)
-            (send (send _ ${#{ALWAYS_RETURNS_NEW_ARRAY + RETURNS_NEW_ARRAY_WHEN_NO_BLOCK}} ...) ${#{HAS_MUTATION_ALTERNATIVE}} $...)
-          }
+        def_node_matcher :chain_array_allocation?, <<~PATTERN
+          (send {
+            (send _ $%RETURN_NEW_ARRAY_WHEN_ARGS {int lvar ivar cvar gvar})
+            (block (send _ $%ALWAYS_RETURNS_NEW_ARRAY) ...)
+            (send _ $%RETURNS_NEW_ARRAY ...)
+          } $%HAS_MUTATION_ALTERNATIVE ...)
         PATTERN
 
         def on_send(node)
-          flat_map_candidate?(node) do |fm, sm, _|
+          chain_array_allocation?(node) do |fm, sm|
             range = range_between(node.loc.dot.begin_pos, node.source_range.end_pos)
 
             add_offense(range, message: format(MSG, method: fm, second_method: sm))

spec/rubocop/cop/performance/chain_array_allocation_spec.rb

@@ -1,26 +1,7 @@
 # frozen_string_literal: true
 
-RSpec.describe RuboCop::Cop::Performance::ChainArrayAllocation, :config do
-  subject(:cop) { described_class.new(config) }
-
-  def generate_message(method_one, method_two)
-    "Use unchained `#{method_one}!` and `#{method_two}!` "\
-    '(followed by `return array` if required) instead of '\
-    "chaining `#{method_one}...#{method_two}`."
-  end
-
-  shared_examples 'map_and_flat' do |method, method_two|
-    it "registers an offense when calling #{method}...#{method_two}" do
-      expect_offense(<<~RUBY, method: method, method_two: method_two)
-        [1, 2, 3, 4].#{method} { |e| [e, e] }.#{method_two}
-                     _{method}               ^^{method_two} #{generate_message(method, method_two)}
-      RUBY
-    end
-  end
-
-  describe 'configured to only warn when flattening one level' do
-    it_behaves_like('map_and_flat', 'map', 'flatten')
-  end
+RSpec.describe RuboCop::Cop::Performance::ChainArrayAllocation do
+  subject(:cop) { described_class.new }
 
   describe 'Methods that require an argument' do
     it 'does not register an offense for `first.uniq`' do
@@ -33,15 +14,15 @@ def generate_message(method_one, method_two)
     it 'registers an offense for `first(10).uniq`' do
       expect_offense(<<~RUBY)
         [1, 2, 3, 4].first(10).uniq
-                              ^^^^^ Use unchained `first!` and `uniq!` (followed by `return array` if required) instead of chaining `first...uniq`.
+                              ^^^^^ Use unchained `first` and `uniq!` (followed by `return array` if required) instead of chaining `first...uniq`.
       RUBY
     end
 
     it 'registers an offense for `first(variable).uniq`' do
       expect_offense(<<~RUBY)
         variable = 42
         [1, 2, 3, 4].first(variable).uniq
-                                    ^^^^^ Use unchained `first!` and `uniq!` (followed by `return array` if required) instead of chaining `first...uniq`.
+                                    ^^^^^ Use unchained `first` and `uniq!` (followed by `return array` if required) instead of chaining `first...uniq`.
       RUBY
     end
   end
@@ -55,7 +36,7 @@ def generate_message(method_one, method_two)
 
       expect_offense(<<~RUBY)
         [1, 2, 3, 4].zip {|f| }.zip.uniq
-                                   ^^^^^ Use unchained `zip!` and `uniq!` (followed by `return array` if required) instead of chaining `zip...uniq`.
+                                   ^^^^^ Use unchained `zip` and `uniq!` (followed by `return array` if required) instead of chaining `zip...uniq`.
       RUBY
     end
   end