Skip to content

Commit

Permalink
Handle recursive keys in strong parameters expect
Browse files Browse the repository at this point in the history
Fixes #1429

Use a recursive approach to autocorrect nested parameters
- Top level hash keys do not get replaced by `[]`
- Hash values that are hashes get replaced by `[]`
- Arrays get replaced by double brackets `[[]]`
  • Loading branch information
chaadow committed Jan 26, 2025
1 parent c95d720 commit 7e12bd2
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1429](https://github.com/rubocop/rubocop-rails/issues/1429): Fix a wrong autocorrect for `Rails/StrongParametersExpect` when hashes and arrays are present. ([@chaadow][])
39 changes: 34 additions & 5 deletions lib/rubocop/cop/rails/strong_parameters_expect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,13 @@ def on_send(node)
corrector.remove(require_method.receiver.source_range.end.join(require_method.source_range.end))
corrector.replace(permit_method.loc.selector, 'expect')
if replace_argument
corrector.insert_before(permit_method.first_argument, "#{require_key(require_method)}[")
corrector.insert_after(permit_method.last_argument, ']')
transformed_arguments = permit_method.arguments.map { |arg| recursive_transform(arg) }.join(', ')
transformed_string = "#{require_key(require_method)}[#{transformed_arguments}])"

args_range = permit_method.source_range.with(
begin_pos: permit_method.first_argument.source_range.begin_pos
)
corrector.replace(args_range, transformed_string)
end
end

Expand All @@ -80,11 +85,35 @@ def offense_range(method_node, node)

def expect_method(require_method, permit_method)
require_key = require_key(require_method)
permit_args = permit_method.arguments.map(&:source).join(', ')

arguments = "#{require_key}[#{permit_args}]"
permit_args = permit_method.arguments.map { |arg| recursive_transform(arg) }.join(', ')

"expect(#{require_key}[#{permit_args}])"
end

"expect(#{arguments})"
def recursive_transform(node, top_level: true)
case node.type
when :hash
elements = hash_elements_from_node(node)
if top_level
elements.join(', ')
else
elements.empty? ? '{}' : "[#{elements.join(', ')}]"
end
when :array
elements = node.children.map { |child| recursive_transform(child, top_level: false) }
elements.empty? ? elements : "[[#{elements.join(', ')}]]"
else
node.source
end
end

def hash_elements_from_node(node)
node.pairs.map do |pair|
key = pair.key.source
value = recursive_transform(pair.value, top_level: false)
"#{key}: #{value}"
end
end

def require_key(require_method)
Expand Down
18 changes: 15 additions & 3 deletions spec/rubocop/cop/rails/strong_parameters_expect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,20 @@
RUBY
end

it 'registers an offense when using
`params.require(:user).permit(:salary, :employees_count, rows_prices:
{ row_total_one_post: [:price, :cell_id], row_total_all_posts: [:price, :cell_id]
}, data: {}, another_array: [:name], empty_array: [])`' do
expect_offense(<<~RUBY)
params.require(:user).permit(:salary, :posts_count, rows_prices: { row_total_one_post: [:price, :cell_id], row_total_all_posts: [:price, :cell_id] }, data: {}, another_array: [:name], empty_array: [])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(user: [:salary, :posts_count, rows_prices: [row_total_one_post: [[:price, :cell_id]], row_total_all_posts: [[:price, :cell_id]]], data: {}, another_array: [[:name]], empty_array: []])` instead.
RUBY

expect_correction(<<~RUBY)
params.expect(user: [:salary, :posts_count, rows_prices: [row_total_one_post: [[:price, :cell_id]], row_total_all_posts: [[:price, :cell_id]]], data: {}, another_array: [[:name]], empty_array: []])
RUBY
end

it 'registers an offense when using `params&.require(:user)&.permit(:name, :age)`' do
expect_offense(<<~RUBY)
params&.require(:user)&.permit(:name, :age)
Expand Down Expand Up @@ -92,9 +106,7 @@

expect_correction(<<~RUBY)
params.expect(
user: [:name, # comment
:age] # comment
)
user: [:name, :age])
RUBY
end

Expand Down

0 comments on commit 7e12bd2

Please sign in to comment.