Skip to content

Commit

Permalink
[Fix rubocop#431] prefer << with push
Browse files Browse the repository at this point in the history
  • Loading branch information
amomchilov committed Jan 12, 2024
1 parent 3ba5d91 commit 8a2164d
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 0 deletions.
57 changes: 57 additions & 0 deletions lib/rubocop/cop/performance/array_push_single.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Performance
# Identifies places where pushing a single element to an array
# can be replaced by `Array#<<`.
#
# @safety
# This cop is unsafe because not all objects that respond to `#push` also respond to `#<<`
#
# @example
# # bad
# array.push(1)
#
# # good
# array << 1
#
# # good
# array.push(1, 2, 3) # `<<` only works for one element
#
class ArrayPushSingle < Base
extend AutoCorrector

MSG = 'Use `<<` instead of `%<current>s`.'

PUSH_METHODS = Set[:push, :append].freeze
RESTRICT_ON_SEND = PUSH_METHODS

def_node_matcher :push_call?, <<~PATTERN
(call $_receiver $%PUSH_METHODS $!(splat _))
PATTERN

def on_send(node)
raise
push_call?(node) do |receiver, method_name, element|
message = format(MSG, current: method_name)

add_offense(node, message: message) do |corrector|
corrector.replace(node, "#{receiver.source} << #{element.source}")
end
end
end

def on_csend(node)
push_call?(node) do |receiver, method_name, element|
message = format(MSG, current: method_name)

add_offense(node, message: message) do |corrector|
corrector.replace(node, "#{receiver.source}&.<< #{element.source}")
end
end
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 @@ -4,6 +4,7 @@
require_relative 'mixin/sort_block'

require_relative 'performance/ancestors_include'
require_relative 'performance/array_push_single'
require_relative 'performance/array_semi_infinite_range_slice'
require_relative 'performance/big_decimal_with_numeric_argument'
require_relative 'performance/bind_call'
Expand Down
56 changes: 56 additions & 0 deletions spec/rubocop/cop/performance/array_push_single_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::ArrayPushSingle, :config do
it 'registers an offense and corrects when using `push` with a single element' do
expect_offense(<<~RUBY)
array.push(element)
^^^^^^^^^^^^^^^^^^^ Use `<<` instead of `push`.
RUBY

expect_correction(<<~RUBY)
array << element
RUBY
end

it 'registers an offense and corrects when using `push` with a single element and safe navigation operator' do
expect_offense(<<~RUBY)
array&.push(element)
^^^^^^^^^^^^^^^^^^^^ Use `<<` instead of `push`.
RUBY

# gross. TODO: make a configuration option?
expect_correction(<<~RUBY)
array&.<< element
RUBY
end

it 'does not register an offense when using `push` with multiple elements' do
expect_no_offenses(<<~RUBY)
array.push(1, 2, 3)
RUBY
end

it 'does not register an offense when using `push` with splatted elements' do
expect_no_offenses(<<~RUBY)
array.push(*elements)
RUBY
end

describe "replacing `push` with `<<`" do
subject(:array) { [1, 2, 3] }

it "returns the same result" do
expect([1, 2, 3] << 4).to eq([1, 2, 3].push(4))
end

it "it has the same side-effect" do
a = [1, 2, 3]
a << 4

b = [1, 2, 3]
b.push(4)

expect(a).to eq(b)
end
end
end

0 comments on commit 8a2164d

Please sign in to comment.