Skip to content

Commit 1b8d885

Browse files
committed
Handle recursive keys in strong parameters expect
Fixes rubocop#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 `[[]]`
1 parent c95d720 commit 1b8d885

File tree

3 files changed

+50
-8
lines changed

3 files changed

+50
-8
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1429](https://github.com/rubocop/rubocop-rails/issues/1429): Fix a wrong autocorrect for `Rails/StrongParametersExpect` when hashes and arrays are present. ([@chaadow][])

lib/rubocop/cop/rails/strong_parameters_expect.rb

+34-5
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,13 @@ def on_send(node)
6262
corrector.remove(require_method.receiver.source_range.end.join(require_method.source_range.end))
6363
corrector.replace(permit_method.loc.selector, 'expect')
6464
if replace_argument
65-
corrector.insert_before(permit_method.first_argument, "#{require_key(require_method)}[")
66-
corrector.insert_after(permit_method.last_argument, ']')
65+
transformed_arguments = permit_method.arguments.map { |arg| recursive_transform(arg) }.join(', ')
66+
transformed_string = "#{require_key(require_method)}[#{transformed_arguments}])"
67+
68+
args_range = permit_method.source_range.with(
69+
begin_pos: permit_method.first_argument.source_range.begin_pos
70+
)
71+
corrector.replace(args_range, transformed_string)
6772
end
6873
end
6974

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

8186
def expect_method(require_method, permit_method)
8287
require_key = require_key(require_method)
83-
permit_args = permit_method.arguments.map(&:source).join(', ')
8488

85-
arguments = "#{require_key}[#{permit_args}]"
89+
permit_args = permit_method.arguments.map { |arg| recursive_transform(arg) }.join(', ')
90+
91+
"expect(#{require_key}[#{permit_args}])"
92+
end
8693

87-
"expect(#{arguments})"
94+
def recursive_transform(node, top_level: true)
95+
case node.type
96+
when :hash
97+
elements = hash_elements_from_node(node)
98+
if top_level
99+
elements.join(', ')
100+
else
101+
elements.empty? ? '{}' : "[#{elements.join(', ')}]"
102+
end
103+
when :array
104+
elements = node.children.map { |child| recursive_transform(child, top_level: false) }
105+
elements.empty? ? elements : "[[#{elements.join(', ')}]]"
106+
else
107+
node.source
108+
end
109+
end
110+
111+
def hash_elements_from_node(node)
112+
node.pairs.map do |pair|
113+
key = pair.key.source
114+
value = recursive_transform(pair.value, top_level: false)
115+
"#{key}: #{value}"
116+
end
88117
end
89118

90119
def require_key(require_method)

spec/rubocop/cop/rails/strong_parameters_expect_spec.rb

+15-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,20 @@
1313
RUBY
1414
end
1515

16+
it 'registers an offense when using
17+
`params.require(:user).permit(:salary, :employees_count, rows_prices:
18+
{ row_total_one_post: [:price, :cell_id], row_total_all_posts: [:price, :cell_id]
19+
}, data: {}, another_array: [:name])`' do
20+
expect_offense(<<~RUBY)
21+
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])
22+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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]]])` instead.
23+
RUBY
24+
25+
expect_correction(<<~RUBY)
26+
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]]])
27+
RUBY
28+
end
29+
1630
it 'registers an offense when using `params&.require(:user)&.permit(:name, :age)`' do
1731
expect_offense(<<~RUBY)
1832
params&.require(:user)&.permit(:name, :age)
@@ -92,9 +106,7 @@
92106

93107
expect_correction(<<~RUBY)
94108
params.expect(
95-
user: [:name, # comment
96-
:age] # comment
97-
)
109+
user: [:name, :age])
98110
RUBY
99111
end
100112

0 commit comments

Comments
 (0)