Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Rails/IndexBy and Rails/IndexWith to support numbered block parameters #1404

Merged
merged 1 commit into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_index_by_index_with_numblock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1404](https://github.com/rubocop/rubocop-rails/pull/1404): Update `Rails/IndexBy` and `Rails/IndexWith` to support numbered block parameters. ([@eugeneius][])
6 changes: 5 additions & 1 deletion lib/rubocop/cop/mixin/index_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Cop
module IndexMethod # rubocop:disable Metrics/ModuleLength
RESTRICT_ON_SEND = %i[each_with_object to_h map collect []].freeze

def on_block(node) # rubocop:todo InternalAffairs/NumblockHandler
def on_block(node)
on_bad_each_with_object(node) do |*match|
handle_possible_offense(node, match, 'each_with_object')
end
Expand All @@ -18,6 +18,8 @@ def on_block(node) # rubocop:todo InternalAffairs/NumblockHandler
end
end

alias on_numblock on_block

def on_send(node)
on_bad_map_to_h(node) do |*match|
handle_possible_offense(node, match, 'map { ... }.to_h')
Expand Down Expand Up @@ -153,6 +155,8 @@ def set_new_method_name(new_method_name, corrector)
end

def set_new_arg_name(transformed_argname, corrector)
return if block_node.numblock_type?

corrector.replace(block_node.arguments, "|#{transformed_argname}|")
end

Expand Down
40 changes: 28 additions & 12 deletions lib/rubocop/cop/rails/index_by.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,45 @@ class IndexBy < Base
PATTERN

def_node_matcher :on_bad_to_h, <<~PATTERN
(block
(call _ :to_h)
(args (arg $_el))
(array $_ (lvar _el)))
{
(block
(call _ :to_h)
(args (arg $_el))
(array $_ (lvar _el)))
(numblock
(call _ :to_h) $1
(array $_ (lvar :_1)))
}
PATTERN

def_node_matcher :on_bad_map_to_h, <<~PATTERN
(call
(block
(call _ {:map :collect})
(args (arg $_el))
(array $_ (lvar _el)))
{
(block
(call _ {:map :collect})
(args (arg $_el))
(array $_ (lvar _el)))
(numblock
(call _ {:map :collect}) $1
(array $_ (lvar :_1)))
}
:to_h)
PATTERN

def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
(send
(const {nil? cbase} :Hash)
:[]
(block
(call _ {:map :collect})
(args (arg $_el))
(array $_ (lvar _el))))
{
(block
(call _ {:map :collect})
(args (arg $_el))
(array $_ (lvar _el)))
(numblock
(call _ {:map :collect}) $1
(array $_ (lvar :_1)))
}
)
PATTERN

private
Expand Down
40 changes: 28 additions & 12 deletions lib/rubocop/cop/rails/index_with.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,45 @@ class IndexWith < Base
PATTERN

def_node_matcher :on_bad_to_h, <<~PATTERN
(block
(call _ :to_h)
(args (arg $_el))
(array (lvar _el) $_))
{
(block
(call _ :to_h)
(args (arg $_el))
(array (lvar _el) $_))
(numblock
(call _ :to_h) $1
(array (lvar :_1) $_))
}
PATTERN

def_node_matcher :on_bad_map_to_h, <<~PATTERN
(call
(block
(call _ {:map :collect})
(args (arg $_el))
(array (lvar _el) $_))
{
(block
(call _ {:map :collect})
(args (arg $_el))
(array (lvar _el) $_))
(numblock
(call _ {:map :collect}) $1
(array (lvar :_1) $_))
}
:to_h)
PATTERN

def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
(send
(const {nil? cbase} :Hash)
:[]
(block
(call _ {:map :collect})
(args (arg $_el))
(array (lvar _el) $_)))
{
(block
(call _ {:map :collect})
(args (arg $_el))
(array (lvar _el) $_))
(numblock
(call _ {:map :collect}) $1
(array (lvar :_1) $_))
}
)
PATTERN

private
Expand Down
59 changes: 59 additions & 0 deletions spec/rubocop/cop/rails/index_by_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,63 @@
RUBY
end
end

context 'numbered parameters' do
it 'registers an offense for `map { ... }.to_h`' do
expect_offense(<<~RUBY)
x.map { [_1.to_sym, _1] }.to_h
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `map { ... }.to_h`.
RUBY

expect_correction(<<~RUBY)
x.index_by { _1.to_sym }
RUBY
end

context 'when values are transformed' do
it 'does not register an offense for `map { ... }.to_h`' do
expect_no_offenses(<<~RUBY)
x.map { [_1.to_sym, foo(_1)] }.to_h
RUBY
end
end

it 'registers an offense for Hash[map { ... }]' do
expect_offense(<<~RUBY)
Hash[x.map { [_1.to_sym, _1] }]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `Hash[map { ... }]`.
RUBY

expect_correction(<<~RUBY)
x.index_by { _1.to_sym }
RUBY
end

context 'when the referenced numbered parameter is not _1' do
it 'does not register an offense for Hash[map { ... }]' do
expect_no_offenses(<<~RUBY)
Hash[x.map { [_1.to_sym, _2] }]
RUBY
end
end

it 'registers an offense for `to_h { ... }`' do
expect_offense(<<~RUBY)
x.to_h { [_1.to_sym, _1] }
^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `to_h { ... }`.
RUBY

expect_correction(<<~RUBY)
x.index_by { _1.to_sym }
RUBY
end

context 'when a numbered parameter other than _1 is referenced in the key' do
it 'does not register an offense for `to_h { ... }`' do
expect_no_offenses(<<~RUBY)
x.to_h { [_2.to_sym, _1] }
RUBY
end
end
end
end
59 changes: 59 additions & 0 deletions spec/rubocop/cop/rails/index_with_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,65 @@
RUBY
end
end

context 'numbered parameters' do
it 'registers an offense for `map { ... }.to_h`' do
expect_offense(<<~RUBY)
x.map { [_1, _1.to_sym] }.to_h
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `map { ... }.to_h`.
RUBY

expect_correction(<<~RUBY)
x.index_with { _1.to_sym }
RUBY
end

context 'when keys are transformed' do
it 'does not register an offense for `map { ... }.to_h`' do
expect_no_offenses(<<~RUBY)
x.map { [foo(_1), _1.to_sym] }.to_h
RUBY
end
end

it 'registers an offense for Hash[map { ... }]' do
expect_offense(<<~RUBY)
Hash[x.map { [_1, _1.to_sym] }]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `Hash[map { ... }]`.
RUBY

expect_correction(<<~RUBY)
x.index_with { _1.to_sym }
RUBY
end

context 'when the referenced numbered parameter is not _1' do
it 'does not register an offense for Hash[map { ... }]' do
expect_no_offenses(<<~RUBY)
Hash[x.map { [_2, _1.to_sym] }]
RUBY
end
end

it 'registers an offense for `to_h { ... }`' do
expect_offense(<<~RUBY)
x.to_h { [_1, _1.to_sym] }
^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `to_h { ... }`.
RUBY

expect_correction(<<~RUBY)
x.index_with { _1.to_sym }
RUBY
end

context 'when a numbered parameter other than _1 is referenced in the value' do
it 'does not register an offense for `to_h { ... }`' do
expect_no_offenses(<<~RUBY)
x.to_h { [_1, _2.to_sym] }
RUBY
end
end
end
end

context 'when using Rails 5.2 or older', :rails52 do
Expand Down
Loading