Skip to content

Commit 8a2164d

Browse files
committed
[Fix #431] prefer << with push
1 parent 3ba5d91 commit 8a2164d

File tree

3 files changed

+114
-0
lines changed

3 files changed

+114
-0
lines changed
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Performance
6+
# Identifies places where pushing a single element to an array
7+
# can be replaced by `Array#<<`.
8+
#
9+
# @safety
10+
# This cop is unsafe because not all objects that respond to `#push` also respond to `#<<`
11+
#
12+
# @example
13+
# # bad
14+
# array.push(1)
15+
#
16+
# # good
17+
# array << 1
18+
#
19+
# # good
20+
# array.push(1, 2, 3) # `<<` only works for one element
21+
#
22+
class ArrayPushSingle < Base
23+
extend AutoCorrector
24+
25+
MSG = 'Use `<<` instead of `%<current>s`.'
26+
27+
PUSH_METHODS = Set[:push, :append].freeze
28+
RESTRICT_ON_SEND = PUSH_METHODS
29+
30+
def_node_matcher :push_call?, <<~PATTERN
31+
(call $_receiver $%PUSH_METHODS $!(splat _))
32+
PATTERN
33+
34+
def on_send(node)
35+
raise
36+
push_call?(node) do |receiver, method_name, element|
37+
message = format(MSG, current: method_name)
38+
39+
add_offense(node, message: message) do |corrector|
40+
corrector.replace(node, "#{receiver.source} << #{element.source}")
41+
end
42+
end
43+
end
44+
45+
def on_csend(node)
46+
push_call?(node) do |receiver, method_name, element|
47+
message = format(MSG, current: method_name)
48+
49+
add_offense(node, message: message) do |corrector|
50+
corrector.replace(node, "#{receiver.source}&.<< #{element.source}")
51+
end
52+
end
53+
end
54+
end
55+
end
56+
end
57+
end

lib/rubocop/cop/performance_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
require_relative 'mixin/sort_block'
55

66
require_relative 'performance/ancestors_include'
7+
require_relative 'performance/array_push_single'
78
require_relative 'performance/array_semi_infinite_range_slice'
89
require_relative 'performance/big_decimal_with_numeric_argument'
910
require_relative 'performance/bind_call'
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Performance::ArrayPushSingle, :config do
4+
it 'registers an offense and corrects when using `push` with a single element' do
5+
expect_offense(<<~RUBY)
6+
array.push(element)
7+
^^^^^^^^^^^^^^^^^^^ Use `<<` instead of `push`.
8+
RUBY
9+
10+
expect_correction(<<~RUBY)
11+
array << element
12+
RUBY
13+
end
14+
15+
it 'registers an offense and corrects when using `push` with a single element and safe navigation operator' do
16+
expect_offense(<<~RUBY)
17+
array&.push(element)
18+
^^^^^^^^^^^^^^^^^^^^ Use `<<` instead of `push`.
19+
RUBY
20+
21+
# gross. TODO: make a configuration option?
22+
expect_correction(<<~RUBY)
23+
array&.<< element
24+
RUBY
25+
end
26+
27+
it 'does not register an offense when using `push` with multiple elements' do
28+
expect_no_offenses(<<~RUBY)
29+
array.push(1, 2, 3)
30+
RUBY
31+
end
32+
33+
it 'does not register an offense when using `push` with splatted elements' do
34+
expect_no_offenses(<<~RUBY)
35+
array.push(*elements)
36+
RUBY
37+
end
38+
39+
describe "replacing `push` with `<<`" do
40+
subject(:array) { [1, 2, 3] }
41+
42+
it "returns the same result" do
43+
expect([1, 2, 3] << 4).to eq([1, 2, 3].push(4))
44+
end
45+
46+
it "it has the same side-effect" do
47+
a = [1, 2, 3]
48+
a << 4
49+
50+
b = [1, 2, 3]
51+
b.push(4)
52+
53+
expect(a).to eq(b)
54+
end
55+
end
56+
end

0 commit comments

Comments
 (0)