Add UseZipToWrapArrayContents

Performance Cop for the more efficient way to generate an Array of Arrays.

 * Performs 40-90% faster than mapping to iteratively wrap array contents.
 * Performs 5 - 55% faster on ranges, depending on size.

changelog/new_merge_pull_request_459_from.md

@@ -0,0 +1 @@
+* [#462](https://github.com/rubocop/rubocop-performance/pull/462): Add new `Performance/UseZipToWrapArrayContents` cop that checks patterns like `.map { |id| [id] }` or `.map { [_1] }` and can replace them with `.zip`. ([@corsonknowles][])

config/default.yml

@@ -372,3 +372,8 @@ 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: pending
+  VersionAdded: <<next>>

lib/rubocop/cop/performance/use_zip_to_wrap_array_contents.rb

@@ -0,0 +1,78 @@
+# 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
+      #   # good (no offense)
+      #   [1, 2, 3].map { |id| id }
+      #
+      # @example
+      #   # good (no offense)
+      #   [1, 2, 3].map { |id| [id, id] }
+      class UseZipToWrapArrayContents < Base
+        include RangeHelp
+        extend AutoCorrector
+
+        MSG = 'Use `zip` instead of `%<original_code>s`.'
+        RESTRICT_ON_SEND = %i[map].freeze
+
+        # Matches regular block form `.map { |e| [e] }`
+        # @!method map_with_array?(node)
+        def_node_matcher :map_with_array?, <<~PATTERN
+          (block
+            (send _ :map)
+            (args (arg _id))
+            (array (lvar _id)))
+        PATTERN
+
+        # Matches numblock form `.map { [_1] }`
+        # @!method map_with_array_numblock?(node)
+        def_node_matcher :map_with_array_numblock?, <<~PATTERN
+          (numblock
+            (send _ :map)
+            1
+            (array (lvar _))
+          )
+        PATTERN
+
+        def on_send(node)
+          return unless (parent = node.parent)
+          return unless map_with_array?(parent) || map_with_array_numblock?(parent)
+          return unless (receiver = node.receiver&.source)
+
+          register_offense(parent, receiver, node)
+        end
+
+        private
+
+        def register_offense(parent, receiver, node)
+          add_offense(offense_range(parent), message: message(node)) do |corrector|
+            autocorrect(parent, receiver, corrector)
+          end
+        end
+
+        def message(node)
+          format(MSG, original_code: offense_range(node).source.lines.first.chomp)
+        end
+
+        def autocorrect(parent, receiver, corrector)
+          corrector.replace(parent, "#{receiver}.zip")
+        end
+
+        def offense_range(node)
+          @offense_range ||= range_between(node.children.first.loc.selector.begin_pos, node.loc.end.end_pos)
+        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,302 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Performance::UseZipToWrapArrayContents, :config do
+  context 'when using map with array literal' do
+    it 'registers an offense and corrects to use zip with no arguments' do
+      expect_offense(<<~RUBY)
+        [1, 2, 3].map { |id| [id] }
+                  ^^^^^^^^^^^^^^^^^ Use `zip` instead of `map { |id| [id] }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2, 3].zip
+      RUBY
+    end
+  end
+
+  context 'when using map with a short iterator name' do
+    it 'registers an offense and corrects to use zip with no arguments' do
+      expect_offense(<<~RUBY)
+        [1, 2, 3].map { |e| [e] }
+                  ^^^^^^^^^^^^^^^ Use `zip` instead of `map { |e| [e] }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2, 3].zip
+      RUBY
+    end
+  end
+
+  context 'when using map on a range with another iterator name' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~RUBY)
+        (1..3).map { |x| [x] }
+               ^^^^^^^^^^^^^^^ Use `zip` instead of `map { |x| [x] }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        (1..3).zip
+      RUBY
+    end
+  end
+
+  context 'when using map in a do end block' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~RUBY)
+        (a..b).map do
+               ^^^^^^ Use `zip` instead of `map do`.
+          |m| [m]
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        (a..b).zip
+      RUBY
+    end
+  end
+
+  context 'when using map in a chain' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~RUBY)
+        [nil, tuple].flatten.map { |e| [e] }.call
+                             ^^^^^^^^^^^^^^^ Use `zip` instead of `map { |e| [e] }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [nil, tuple].flatten.zip.call
+      RUBY
+    end
+  end
+
+  context 'when the map block does not contain an array literal' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { |id| id }
+      RUBY
+    end
+  end
+
+  context 'when using map with an array literal containing multiple elements' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { |id| [id, id] }
+      RUBY
+    end
+  end
+
+  context 'when using other iterators such as' do
+    context 'when using collect' do
+      it 'does not register an offense as the use of collect is obscure and perhaps intentional' do
+        expect_no_offenses(<<~RUBY)
+          [1, 2, 3].collect { |id| [id] }
+        RUBY
+      end
+    end
+
+    context 'when using select with an array literal' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          [1, 2, 3].select { |id| [id] }
+        RUBY
+      end
+    end
+
+    context 'when calling filter_map' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          [1, 2, 3].filter_map {|id| [id]}
+        RUBY
+      end
+    end
+
+    context 'when calling flat_map' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          [1, 2, 3].flat_map {|id| [id]}
+        RUBY
+      end
+    end
+  end
+
+  context 'when using map with doubly wrapped arrays' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { |id| [[id]] }
+      RUBY
+    end
+  end
+
+  context 'when using map with addition' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { |id| id + 1 }
+      RUBY
+    end
+  end
+
+  context 'when using map with array addition' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { |id| [id] + [id] }
+      RUBY
+    end
+  end
+
+  context 'when using map with indexing into an array' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { |id| [id][id] }
+      RUBY
+    end
+  end
+
+  context 'when calling map with no arguments' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map
+      RUBY
+    end
+  end
+
+  context 'when calling map with an empty block' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map {}
+      RUBY
+    end
+  end
+
+  context 'with related array of array patterns' do
+    context 'when using [*foo] to dynamically wrap only non-arrays in the list' do
+      it 'does not register an offense since the map is doing useful work' do
+        expect_no_offenses(<<~RUBY)
+          [1, [2], 3].map { |id| [*id] }
+        RUBY
+      end
+    end
+
+    context 'when using Array.wrap the Rails extension of the [*foo] pattern that handles Hashes' do
+      it 'does not register an offense since the map is doing useful work' do
+        expect_no_offenses(<<~RUBY)
+          [1, 2, 3].map { |id| Array.wrap(id) }
+        RUBY
+      end
+    end
+
+    context 'when making an array of arrays using each_with_object' do
+      it 'does not register an offense since we have not included this pattern yet' do
+        expect_no_offenses(<<~RUBY)
+          [1,2,3].each_with_object([]) {|id, object| object << [id]}
+        RUBY
+      end
+    end
+  end
+
+  context 'when using map with a numerical argument' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~RUBY)
+        [1, 2, 3].map { [_1] }
+                  ^^^^^^^^^^^^ Use `zip` instead of `map { [_1] }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2, 3].zip
+      RUBY
+    end
+  end
+
+  context 'when using map with a numblock in a chain' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~RUBY)
+        [1, 2].sum.map { [_1] }.flatten
+                   ^^^^^^^^^^^^ Use `zip` instead of `map { [_1] }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        [1, 2].sum.zip.flatten
+      RUBY
+    end
+  end
+
+  context 'when using map on a range with a numblock' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~RUBY)
+        (1..3).map { [_1] }
+               ^^^^^^^^^^^^ Use `zip` instead of `map { [_1] }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        (1..3).zip
+      RUBY
+    end
+  end
+
+  context 'when using map in a do end block with a numblock' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~RUBY)
+        (a..b).map do [_1] end
+               ^^^^^^^^^^^^^^^ Use `zip` instead of `map do [_1] end`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        (a..b).zip
+      RUBY
+    end
+  end
+
+  context 'when calling filter_map with a numblock' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].filter_map { [_1] }
+      RUBY
+    end
+  end
+
+  context 'when calling map, adding, and wrapping, with a numblock' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { [_1 + 1] }
+      RUBY
+    end
+  end
+
+  context 'when calling double wrapping with a numblock' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { [[_1]] }
+      RUBY
+    end
+  end
+
+  context 'when calling map with an unused iterator' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { |e| [1] }
+      RUBY
+    end
+  end
+
+  context 'when calling map with a static block that always returns the same value' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { [id] }
+      RUBY
+    end
+  end
+
+  context 'when calling map with a static array' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        [1, 2, 3].map { [] }
+      RUBY
+    end
+  end
+
+  context 'when map has no receiver' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        map { |id| [id] }
+      RUBY
+    end
+  end
+end