Skip to content

Commit

Permalink
Add UseZipToWrapArrayContents
Browse files Browse the repository at this point in the history
Performance Cop for the more efficient way to generate an Array of Arrays.

Performs 40-70% faster than mapping to iteratively wrap array contents.
Performs 5 - 55% faster on ranges, depending on size.
  • Loading branch information
corsonknowles committed Sep 14, 2024
1 parent 4c4cacd commit 4864c84
Show file tree
Hide file tree
Showing 5 changed files with 247 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_merge_pull_request_459_from.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#462](https://github.com/rubocop/rubocop-performance/pull/462): Add new `Performance/UseZipToWrapArrayContents` cop that checks patterns like `.map { |id| [id] }` and `.map { [_1] }`can replace with `.zip`. ([@corsonknowles][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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>>
87 changes: 87 additions & 0 deletions lib/rubocop/cop/performance/use_zip_to_wrap_array_contents.rb
Original file line number Diff line number Diff line change
@@ -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

RESTRICT_ON_SEND = %i[map].freeze
MSG = 'Use `.zip` instead of `.map { |id| [id] }`.'

# 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 node.method?(:map)

parent = node.parent
return unless parent

register_offense(parent) if parent.block_type? && map_with_array?(parent)
end

def on_numblock(node)
return unless node.method?(:map)
return unless map_with_array_numblock?(node)

register_offense(node)
end

private

def register_offense(node)
map_node = node.children.first # The send node for `.map`
add_offense(map_node.loc.selector) do |corrector|
autocorrect(node, corrector)
end
end

def autocorrect(node, corrector)
map_call = node.children.first
receiver = map_call.receiver.source
corrector.replace(node, "#{receiver}.zip")
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
153 changes: 153 additions & 0 deletions spec/rubocop/cop/performance/use_zip_to_wrap_array_contents_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
# 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 { |id| [id] }`.
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 the code' do
expect_offense(<<~RUBY)
(1..3).map { |x| [x] }
^^^ Use `.zip` instead of `.map { |id| [id] }`.
RUBY

expect_correction(<<~RUBY)
(1..3).zip
RUBY
end
end

context 'when using map in a do end block' do
it 'registers an offense' do
expect_offense(<<~RUBY)
(a..b).map do |m| [m] end
^^^ Use `.zip` instead of `.map { |id| [id] }`.
RUBY

expect_correction(<<~RUBY)
(a..b).zip
RUBY
end
end

context 'when using map in a chain' do
it 'registers an offense' do
expect_offense(<<~RUBY)
[record, tuple].map { |e| [e] }.call
^^^ Use `.zip` instead of `.map { |id| [id] }`.
RUBY

expect_correction(<<~RUBY)
[record, tuple].zip.call
RUBY
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

expect_correction(<<~RUBY)
[1, 2, 3].zip
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 collect' do
it 'does not register an offense' 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 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 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 using Array.wrap' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
[1, 2, 3].map { |id| Array.wrap(id) }
RUBY
end
end
end

0 comments on commit 4864c84

Please sign in to comment.