diff --git a/changelog/fix_strong_parameters_expect_recursive_autocorrect.md b/changelog/fix_strong_parameters_expect_recursive_autocorrect.md new file mode 100644 index 0000000000..f25c57e6bd --- /dev/null +++ b/changelog/fix_strong_parameters_expect_recursive_autocorrect.md @@ -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][]) diff --git a/lib/rubocop/cop/rails/strong_parameters_expect.rb b/lib/rubocop/cop/rails/strong_parameters_expect.rb index 921c13940c..ffaeec5d8a 100644 --- a/lib/rubocop/cop/rails/strong_parameters_expect.rb +++ b/lib/rubocop/cop/rails/strong_parameters_expect.rb @@ -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 @@ -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) diff --git a/spec/rubocop/cop/rails/strong_parameters_expect_spec.rb b/spec/rubocop/cop/rails/strong_parameters_expect_spec.rb index 0a0bc30d24..76ac7c0546 100644 --- a/spec/rubocop/cop/rails/strong_parameters_expect_spec.rb +++ b/spec/rubocop/cop/rails/strong_parameters_expect_spec.rb @@ -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) @@ -92,9 +106,7 @@ expect_correction(<<~RUBY) params.expect( - user: [:name, # comment - :age] # comment - ) + user: [:name, :age]) RUBY end