rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
822 stars 263 forks source link

Cop idea: Disallow `params.require.permit` and `params.require` in favor of `params.expect` for rails 8.0 #1358

Open Earlopain opened 2 months ago

Earlopain commented 2 months ago

Is your feature request related to a problem? Please describe.

Rails added params.expect in https://github.com/rails/rails/pull/51674. There are some problems with require that are nicely explained in that PR (and linked issues) but it basically boils down to this:

params.require(:user).permit(:name) raises if params looks like user=123 (not the expected shape). The newly added expect avoids this problem by making the shape part of the contract.

Describe the solution you'd like

Method docs: https://edgeapi.rubyonrails.org/classes/ActionController/Parameters.html#method-i-expect

params.require(:user).permit(:name)
# => 
params.expect(user: %i[name])

params.require(:user).permit(*permitted_params, some_ids: [])
# => 
params.permit(user: [*permitted_params, [:some_ids]])

params.permit(:name)
# =>
params.expect(:name)

These two method calls will likely be close together, so I believe it would be best for the cop to simply catch these simple cases first.

Additional context

If plain params.require is added as an offense, it must be unsafe. Consider the following case:

# Allows an array/hash
User.find(params.require(:id))
# This does not
User.find(params.expect([:id]))
# If arrays are expected:
User.find(params.expect([[:id]])
# expect can't allow both an array and plain type

There is no replacement for the following (yet?):

params.fetch(:optional, {}).permit(:some_arg)