Skip to content

Commit 851f382

Browse files
koickoic
authored
Merge pull request #1404 from eugeneius/index_by_index_with_numblock
Update `Rails/IndexBy` and `Rails/IndexWith` to support numbered block parameters
2 parents 43fb400 + 15605e1 commit 851f382

File tree

6 files changed

+180
-25
lines changed

6 files changed

+180
-25
lines changed

‎changelog/fix_index_by_index_with_numblock.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1404](https://github.com/rubocop/rubocop-rails/pull/1404): Update `Rails/IndexBy` and `Rails/IndexWith` to support numbered block parameters. ([@eugeneius][])

‎lib/rubocop/cop/mixin/index_method.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module Cop
66
module IndexMethod # rubocop:disable Metrics/ModuleLength
77
RESTRICT_ON_SEND = %i[each_with_object to_h map collect []].freeze
88

9-
def on_block(node) # rubocop:todo InternalAffairs/NumblockHandler
9+
def on_block(node)
1010
on_bad_each_with_object(node) do |*match|
1111
handle_possible_offense(node, match, 'each_with_object')
1212
end
@@ -18,6 +18,8 @@ def on_block(node) # rubocop:todo InternalAffairs/NumblockHandler
1818
end
1919
end
2020

21+
alias on_numblock on_block
22+
2123
def on_send(node)
2224
on_bad_map_to_h(node) do |*match|
2325
handle_possible_offense(node, match, 'map { ... }.to_h')
@@ -153,6 +155,8 @@ def set_new_method_name(new_method_name, corrector)
153155
end
154156

155157
def set_new_arg_name(transformed_argname, corrector)
158+
return if block_node.numblock_type?
159+
156160
corrector.replace(block_node.arguments, "|#{transformed_argname}|")
157161
end
158162

‎lib/rubocop/cop/rails/index_by.rb‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,29 +29,45 @@ class IndexBy < Base
2929
PATTERN
3030

3131
def_node_matcher :on_bad_to_h, <<~PATTERN
32-
(block
33-
(call _ :to_h)
34-
(args (arg $_el))
35-
(array $_ (lvar _el)))
32+
{
33+
(block
34+
(call _ :to_h)
35+
(args (arg $_el))
36+
(array $_ (lvar _el)))
37+
(numblock
38+
(call _ :to_h) $1
39+
(array $_ (lvar :_1)))
40+
}
3641
PATTERN
3742

3843
def_node_matcher :on_bad_map_to_h, <<~PATTERN
3944
(call
40-
(block
41-
(call _ {:map :collect})
42-
(args (arg $_el))
43-
(array $_ (lvar _el)))
45+
{
46+
(block
47+
(call _ {:map :collect})
48+
(args (arg $_el))
49+
(array $_ (lvar _el)))
50+
(numblock
51+
(call _ {:map :collect}) $1
52+
(array $_ (lvar :_1)))
53+
}
4454
:to_h)
4555
PATTERN
4656

4757
def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
4858
(send
4959
(const {nil? cbase} :Hash)
5060
:[]
51-
(block
52-
(call _ {:map :collect})
53-
(args (arg $_el))
54-
(array $_ (lvar _el))))
61+
{
62+
(block
63+
(call _ {:map :collect})
64+
(args (arg $_el))
65+
(array $_ (lvar _el)))
66+
(numblock
67+
(call _ {:map :collect}) $1
68+
(array $_ (lvar :_1)))
69+
}
70+
)
5571
PATTERN
5672

5773
private

‎lib/rubocop/cop/rails/index_with.rb‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,29 +32,45 @@ class IndexWith < Base
3232
PATTERN
3333

3434
def_node_matcher :on_bad_to_h, <<~PATTERN
35-
(block
36-
(call _ :to_h)
37-
(args (arg $_el))
38-
(array (lvar _el) $_))
35+
{
36+
(block
37+
(call _ :to_h)
38+
(args (arg $_el))
39+
(array (lvar _el) $_))
40+
(numblock
41+
(call _ :to_h) $1
42+
(array (lvar :_1) $_))
43+
}
3944
PATTERN
4045

4146
def_node_matcher :on_bad_map_to_h, <<~PATTERN
4247
(call
43-
(block
44-
(call _ {:map :collect})
45-
(args (arg $_el))
46-
(array (lvar _el) $_))
48+
{
49+
(block
50+
(call _ {:map :collect})
51+
(args (arg $_el))
52+
(array (lvar _el) $_))
53+
(numblock
54+
(call _ {:map :collect}) $1
55+
(array (lvar :_1) $_))
56+
}
4757
:to_h)
4858
PATTERN
4959

5060
def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
5161
(send
5262
(const {nil? cbase} :Hash)
5363
:[]
54-
(block
55-
(call _ {:map :collect})
56-
(args (arg $_el))
57-
(array (lvar _el) $_)))
64+
{
65+
(block
66+
(call _ {:map :collect})
67+
(args (arg $_el))
68+
(array (lvar _el) $_))
69+
(numblock
70+
(call _ {:map :collect}) $1
71+
(array (lvar :_1) $_))
72+
}
73+
)
5874
PATTERN
5975

6076
private

‎spec/rubocop/cop/rails/index_by_spec.rb‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,63 @@
182182
RUBY
183183
end
184184
end
185+
186+
context 'numbered parameters' do
187+
it 'registers an offense for `map { ... }.to_h`' do
188+
expect_offense(<<~RUBY)
189+
x.map { [_1.to_sym, _1] }.to_h
190+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `map { ... }.to_h`.
191+
RUBY
192+
193+
expect_correction(<<~RUBY)
194+
x.index_by { _1.to_sym }
195+
RUBY
196+
end
197+
198+
context 'when values are transformed' do
199+
it 'does not register an offense for `map { ... }.to_h`' do
200+
expect_no_offenses(<<~RUBY)
201+
x.map { [_1.to_sym, foo(_1)] }.to_h
202+
RUBY
203+
end
204+
end
205+
206+
it 'registers an offense for Hash[map { ... }]' do
207+
expect_offense(<<~RUBY)
208+
Hash[x.map { [_1.to_sym, _1] }]
209+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `Hash[map { ... }]`.
210+
RUBY
211+
212+
expect_correction(<<~RUBY)
213+
x.index_by { _1.to_sym }
214+
RUBY
215+
end
216+
217+
context 'when the referenced numbered parameter is not _1' do
218+
it 'does not register an offense for Hash[map { ... }]' do
219+
expect_no_offenses(<<~RUBY)
220+
Hash[x.map { [_1.to_sym, _2] }]
221+
RUBY
222+
end
223+
end
224+
225+
it 'registers an offense for `to_h { ... }`' do
226+
expect_offense(<<~RUBY)
227+
x.to_h { [_1.to_sym, _1] }
228+
^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `to_h { ... }`.
229+
RUBY
230+
231+
expect_correction(<<~RUBY)
232+
x.index_by { _1.to_sym }
233+
RUBY
234+
end
235+
236+
context 'when a numbered parameter other than _1 is referenced in the key' do
237+
it 'does not register an offense for `to_h { ... }`' do
238+
expect_no_offenses(<<~RUBY)
239+
x.to_h { [_2.to_sym, _1] }
240+
RUBY
241+
end
242+
end
243+
end
185244
end

‎spec/rubocop/cop/rails/index_with_spec.rb‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,65 @@
155155
RUBY
156156
end
157157
end
158+
159+
context 'numbered parameters' do
160+
it 'registers an offense for `map { ... }.to_h`' do
161+
expect_offense(<<~RUBY)
162+
x.map { [_1, _1.to_sym] }.to_h
163+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `map { ... }.to_h`.
164+
RUBY
165+
166+
expect_correction(<<~RUBY)
167+
x.index_with { _1.to_sym }
168+
RUBY
169+
end
170+
171+
context 'when keys are transformed' do
172+
it 'does not register an offense for `map { ... }.to_h`' do
173+
expect_no_offenses(<<~RUBY)
174+
x.map { [foo(_1), _1.to_sym] }.to_h
175+
RUBY
176+
end
177+
end
178+
179+
it 'registers an offense for Hash[map { ... }]' do
180+
expect_offense(<<~RUBY)
181+
Hash[x.map { [_1, _1.to_sym] }]
182+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `Hash[map { ... }]`.
183+
RUBY
184+
185+
expect_correction(<<~RUBY)
186+
x.index_with { _1.to_sym }
187+
RUBY
188+
end
189+
190+
context 'when the referenced numbered parameter is not _1' do
191+
it 'does not register an offense for Hash[map { ... }]' do
192+
expect_no_offenses(<<~RUBY)
193+
Hash[x.map { [_2, _1.to_sym] }]
194+
RUBY
195+
end
196+
end
197+
198+
it 'registers an offense for `to_h { ... }`' do
199+
expect_offense(<<~RUBY)
200+
x.to_h { [_1, _1.to_sym] }
201+
^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `to_h { ... }`.
202+
RUBY
203+
204+
expect_correction(<<~RUBY)
205+
x.index_with { _1.to_sym }
206+
RUBY
207+
end
208+
209+
context 'when a numbered parameter other than _1 is referenced in the value' do
210+
it 'does not register an offense for `to_h { ... }`' do
211+
expect_no_offenses(<<~RUBY)
212+
x.to_h { [_1, _2.to_sym] }
213+
RUBY
214+
end
215+
end
216+
end
158217
end
159218

160219
context 'when using Rails 5.2 or older', :rails52 do

0 commit comments

Comments
 (0)