Add UseZipToWrapArrayContents

Performs 40% faster than mapping to iteratively wrap array contents.

This optimization can be significant in large scale applications that leverage bulk enqueuing in Sidekiq, or for any other usage at sufficient scale.

changelog/new_merge_pull_request_459_from.md

@@ -0,0 +1 @@
+* [#462](https://github.com/rubocop/rubocop-performance/pull/462): Merge pull request #462 from corsonknowles:add_performance_use_zip_to_wrap_arrays. ([@corsonknowles][])

config/default.yml

@@ -372,3 +372,9 @@ Performance/UriDefaultParser:
   Description: 'Use `URI::DEFAULT_PARSER` instead of `URI::Parser.new`.'
   Enabled: true
   VersionAdded: '0.50'
+
+Performance/UseZipToWrapArrayContents:
+  Description: 'Checks for `.map { |id| [id] }` and suggests replacing it with `.zip`.'
+  Enabled: true
+  SafeAutoCorrect: true
+  VersionAdded: <<next>>

lib/rubocop/cop/performance/use_zip_to_wrap_array_contents.rb

@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      # Checks for `.map { |id| [id] }` and suggests replacing it with `.zip`.
+      #
+      # @example
+      #   # bad
+      #   [1, 2, 3].map { |id| [id] }
+      #
+      #   # good
+      #   [1, 2, 3].zip
+      #
+      # @example
+      #   # bad
+      #   [1, 2, 3].map { [_1] }
+      #
+      #   # good
+      #   [1, 2, 3].zip
+      #
+      # @example
+      #   # good (no offense)
+      #   [1, 2, 3].map { |id| id }
+      #
+      # @example
+      #   # good (no offense)
+      #   [1, 2, 3].map { |id| [id, id] }
+      class UseZipToWrapArrayContents < Base
+        extend AutoCorrector
+
+        MSG = 'Use `.zip` instead of `.map { |id| [id] }`.'
+
+        # @!method map_with_array?(node)
+        def_node_matcher :map_with_array?, <<~PATTERN
+          (block
+            (send _ :map)
+            (args (arg _id))
+            (array (lvar _id))
+          )
+        PATTERN
+
+        # Matcher for numblock (shorthand block with _1)
+        # @!method map_with_array_numblock?(node)
+        def_node_matcher :map_with_array_numblock?, <<~PATTERN
+          (numblock
+            (send _ :map)
+            1
+            (array (lvar _))
+          )
+        PATTERN
+
+        def on_block(node)
+          return unless node.method?(:map)
+          return unless map_with_array?(node)
+
+          add_offense(node) do |corrector|
+            autocorrect(node).call(corrector)
+          end
+        end
+
+        def on_numblock(node)
+          return unless node.method?(:map)
+          return unless map_with_array_numblock?(node)
+
+          add_offense(node) do |corrector|
+            autocorrect(node).call(corrector)
+          end
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            corrector.replace(node, correct_replacement(node))
+          end
+        end
+
+        private
+
+        def correct_replacement(node)
+          map_call = node.children.first
+          receiver = map_call.receiver.source
+          "#{receiver}.zip"
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/performance_cops.rb

@@ -53,3 +53,4 @@
 require_relative 'performance/unfreeze_string'
 require_relative 'performance/uri_default_parser'
 require_relative 'performance/chain_array_allocation'
+require_relative 'performance/use_zip_to_wrap_array_contents'

spec/rubocop/cop/performance/use_zip_to_wrap_array_contents_spec.rb

@@ -0,0 +1,170 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Performance::UseZipToWrapArrayContents, :config do
+  context 'when using map with array literal' do
+    it 'registers an offense' do
+      expect_offense(<<~RUBY)
+        [1, 2, 3].map { |id| [id] }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `.zip` instead of `.map { |id| [id] }`.
+      RUBY
+    end
+
+    it 'corrects the code' do
+      new_source = autocorrect_source('[1, 2, 3].map { |id| [id] }')
+
+      expect(new_source).to eq '[1, 2, 3].zip'
+    end
+  end
+
+  context 'when using map with a short iterator name' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |e| [e] }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `.zip` instead of `.map { |id| [id] }`.
+      RUBY
+    end
+
+    it 'registers an offense' do
+      expect_offense(source)
+    end
+  end
+
+  context 'when using map on a range with another iterator name' do
+    it 'registers an offense and corrects the code' do
+      expect_offense(<<~RUBY)
+        (1..3).map { |x| [x] }
+        ^^^^^^^^^^^^^^^^^^^^^^ Use `.zip` instead of `.map { |id| [id] }`.
+      RUBY
+
+      new_source = autocorrect_source('(1..3).map { |x| [x] }')
+
+      expect(new_source).to eq '(1..3).zip'
+    end
+  end
+
+  context 'when using map in a do end block' do
+    it 'registers an offense' do
+      expect_offense(<<~RUBY)
+        (1..2).map do |m| [m] end
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `.zip` instead of `.map { |id| [id] }`.
+      RUBY
+    end
+
+    it 'corrects the code' do
+      new_source = autocorrect_source('(1..2).map do |m| [m] end')
+
+      expect(new_source).to eq '(1..2).zip'
+    end
+  end
+
+  context 'when using map with a numerical argment reference' do
+    it 'registers an offense' do
+      expect_offense(<<~RUBY)
+        [1, 2, 3].map { [_1] }
+        ^^^^^^^^^^^^^^^^^^^^^^ Use `.zip` instead of `.map { |id| [id] }`.
+      RUBY
+    end
+
+    it 'corrects the code' do
+      new_source = autocorrect_source('[1, 2, 3].map { [_1] }')
+
+      expect(new_source).to eq '[1, 2, 3].zip'
+    end
+  end
+
+  context 'when the map block does not contain an array literal' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |id| id }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+
+  context 'when using select with an array literal' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].select { |id| [id] }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+
+  context 'when using map with an array literal containing multiple elements' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |id| [id, id] }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+
+  context 'when using map with doubly wrapped arrays' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |id| [[id]] }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+
+  context 'when using map with addition' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |id| id + 1 }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+
+  context 'when using map with array addition' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |id| [id] + [id] }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+
+  context 'when using map with indexing into an array' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |id| [id][id] }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+
+  context 'when using Array.wrap' do
+    let(:source) do
+      <<~RUBY
+        [1, 2, 3].map { |id| Array.wrap(id) }
+      RUBY
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses source
+    end
+  end
+end