Skip to content

Commit d130d42

Browse files
committed
Add each_with_index to Performance/ReverseEach cop
`revese.each_with_index` has the exact same behavior as `reverse.each`, therefore add it to the cop too
1 parent ae0059a commit d130d42

File tree

3 files changed

+203
-1
lines changed

3 files changed

+203
-1
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* Enable cop of `reverse.each_with_index` to be replaced as `reverse_each.with_index`. ([@Babar][])

lib/rubocop/cop/performance/reverse_each.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@ class ReverseEach < Base
2424
extend AutoCorrector
2525

2626
MSG = 'Use `reverse_each` instead of `reverse.each`.'
27-
RESTRICT_ON_SEND = %i[each].freeze
27+
RESTRICT_ON_SEND = %i{each each_with_index}.freeze
2828

2929
def_node_matcher :reverse_each?, <<~MATCHER
3030
(call (call _ :reverse) :each)
3131
MATCHER
3232

33+
def_node_matcher :reverse_each_with_index?, <<~MATCHER
34+
(call (call _ :reverse) :each_with_index)
35+
MATCHER
36+
3337
def on_send(node)
3438
return if use_return_value?(node)
3539

@@ -40,6 +44,15 @@ def on_send(node)
4044
corrector.replace(range, 'reverse_each')
4145
end
4246
end
47+
48+
reverse_each_with_index?(node) do
49+
range = offense_range(node)
50+
51+
add_offense(range) do |corrector|
52+
corrector.replace(range, 'reverse_each.with_index')
53+
end
54+
end
55+
4356
end
4457
alias on_csend on_send
4558

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Performance::ReverseEach, :config do
4+
it 'registers an offense when each_with_index is called on reverse' do
5+
expect_offense(<<~RUBY)
6+
[1, 2, 3].reverse.each_with_index { |e, _i| puts e }
7+
^^^^^^^^^^^^ Use `reverse_each` instead of `reverse.each`.
8+
RUBY
9+
end
10+
11+
it 'registers an offense when each_with_index is called on reverse with safe navigation operator' do
12+
expect_offense(<<~RUBY)
13+
array&.reverse.each_with_index { |e, _i| puts e }
14+
^^^^^^^^^^^^ Use `reverse_each` instead of `reverse.each`.
15+
RUBY
16+
end
17+
18+
it 'registers an offense when each_with_index is called on reverse with safe navigation operator chain' do
19+
expect_offense(<<~RUBY)
20+
array&.reverse&.each.with_index { |e, _i| puts e }
21+
^^^^^^^^^^^^^ Use `reverse_each` instead of `reverse.each`.
22+
RUBY
23+
end
24+
25+
it 'registers an offense when each_with_index is called on reverse on a variable' do
26+
expect_offense(<<~RUBY)
27+
arr = [1, 2, 3]
28+
arr.reverse.each_with_index { |e, _i| puts e }
29+
^^^^^^^^^^^^ Use `reverse_each` instead of `reverse.each`.
30+
RUBY
31+
end
32+
33+
it 'registers an offense when each_with_index is called on reverse on a method call' do
34+
expect_offense(<<~RUBY)
35+
def arr
36+
[1, 2, 3]
37+
end
38+
39+
arr.reverse.each_with_index { |e, _i| puts e }
40+
^^^^^^^^^^^^ Use `reverse_each` instead of `reverse.each`.
41+
RUBY
42+
end
43+
44+
it 'registers an offense for a multi-line reverse.each_with_index' do
45+
expect_offense(<<~RUBY)
46+
def arr
47+
[1, 2, 3]
48+
end
49+
50+
arr.
51+
reverse.
52+
^^^^^^^^ Use `reverse_each` instead of `reverse.each`.
53+
each.with_index { |e, _i| puts e }
54+
RUBY
55+
end
56+
57+
it 'does not register an offense when reverse is used without each' do
58+
expect_no_offenses('[1, 2, 3].reverse')
59+
end
60+
61+
it 'does not register an offense when each_with_index is used without reverse' do
62+
expect_no_offenses('[1, 2, 3].each.with_index { |e, _i| puts e }')
63+
end
64+
65+
context 'autocorrect' do
66+
it 'corrects reverse.each_with_index to reverse_each_with_index' do
67+
new_source = autocorrect_source('[1, 2].reverse.each_with_index { |e, _i| puts e }')
68+
69+
expect(new_source).to eq('[1, 2].reverse_each.with_index { |e, _i| puts e }')
70+
end
71+
72+
it 'corrects reverse.each_with_index to reverse_each_with_index on a variable' do
73+
new_source = autocorrect_source(<<~RUBY)
74+
arr = [1, 2]
75+
arr.reverse.each_with_index { |e, _i| puts e }
76+
RUBY
77+
78+
expect(new_source).to eq(<<~RUBY)
79+
arr = [1, 2]
80+
arr.reverse_each.with_index { |e, _i| puts e }
81+
RUBY
82+
end
83+
84+
it 'corrects reverse.each_with_index to reverse_each_with_index on a method call' do
85+
new_source = autocorrect_source(<<~RUBY)
86+
def arr
87+
[1, 2]
88+
end
89+
90+
arr.reverse.each_with_index { |e, _i| puts e }
91+
RUBY
92+
93+
expect(new_source).to eq(<<~RUBY)
94+
def arr
95+
[1, 2]
96+
end
97+
98+
arr.reverse_each.with_index { |e, _i| puts e }
99+
RUBY
100+
end
101+
102+
it 'registers and corrects when using multi-line `reverse.each_with_index` with trailing dot' do
103+
expect_offense(<<~RUBY)
104+
def arr
105+
[1, 2]
106+
end
107+
108+
arr.
109+
reverse.
110+
^^^^^^^^ Use `reverse_each_with_index` instead of `reverse.each_with_index`.
111+
each.with_index { |e, _i| puts e }
112+
RUBY
113+
114+
expect_correction(<<~RUBY)
115+
def arr
116+
[1, 2]
117+
end
118+
119+
arr.
120+
reverse_each.with_index { |e, _i| puts e }
121+
RUBY
122+
end
123+
124+
it 'registers and corrects when using multi-line `reverse.each_with_index` with leading dot' do
125+
expect_offense(<<~RUBY)
126+
def arr
127+
[1, 2]
128+
end
129+
130+
arr
131+
.reverse
132+
^^^^^^^ Use `reverse_each` instead of `reverse.each`.
133+
.each.with_index { |e, _i| puts e }
134+
RUBY
135+
136+
expect_correction(<<~RUBY)
137+
def arr
138+
[1, 2]
139+
end
140+
141+
arr
142+
.reverse_each.with_index { |e, _i| puts e }
143+
RUBY
144+
end
145+
end
146+
147+
it 'does not register an offense when each_with_index is called on reverse and assign the result to lvar' do
148+
expect_no_offenses(<<~RUBY)
149+
ret = [1, 2, 3].reverse.each_with_index { |e, _i| puts e }
150+
RUBY
151+
end
152+
153+
it 'does not register an offense when each_with_index is called on reverse and assign the result to ivar' do
154+
expect_no_offenses(<<~RUBY)
155+
@ret = [1, 2, 3].reverse.each_with_index { |e, _i| puts e }
156+
RUBY
157+
end
158+
159+
it 'does not register an offense when each_with_index is called on reverse and assign the result to cvar' do
160+
expect_no_offenses(<<~RUBY)
161+
@@ret = [1, 2, 3].reverse.each_with_index { |e, _i| puts e }
162+
RUBY
163+
end
164+
165+
it 'does not register an offense when each_with_index is called on reverse and assign the result to gvar' do
166+
expect_no_offenses(<<~RUBY)
167+
$ret = [1, 2, 3].reverse.each_with_index { |e, _i| puts e }
168+
RUBY
169+
end
170+
171+
it 'does not register an offense when each_with_index is called on reverse and assign the result to constant' do
172+
expect_no_offenses(<<~RUBY)
173+
RET = [1, 2, 3].reverse.each_with_index { |e, _i| puts e }
174+
RUBY
175+
end
176+
177+
it 'does not register an offense when each_with_index is called on reverse and using the result to method chain' do
178+
expect_no_offenses(<<~RUBY)
179+
[1, 2, 3].reverse.each_with_index { |e, _i| puts e }.last
180+
RUBY
181+
end
182+
183+
it 'does not register an offense when each_with_index is called on reverse and returning the result' do
184+
expect_no_offenses(<<~RUBY)
185+
return [1, 2, 3].reverse.each_with_index { |e, _i| puts e }
186+
RUBY
187+
end
188+
end

0 commit comments

Comments
 (0)