@martinemde please add tests and a changelog entry

I wonder if we should just handle these NoMethodErrors differently. It feels weird to return a 500 when the error is the "the user's fault". Maybe we could return a 4XX somehow?

Let's move on with this one. @dhh already reviewed the API and said it looks promising.

Can you write tests for it and prefer using it to require.permit in the guides?

Fixes #52463

I'll update the PR soon

Fixes #52463

Actually, if I'm not mistaken, fixes only half of it.
params.require(...).permit(...) will be fixed (as used all the time in StrongParameters)
params.fetch(:x, {}).permit(...) won't be fixed (first code snippet under StrongParameters#more_examples)

I'm pushing up work as I finish. Thanks for the feedback. Should have it done soon. (I'll squash when it's ready)

As I've been working on this I thought of an alternate name. What do you think?

params.expect(person: %i[name, age])

After typing mandate over and over, I'm not totally in love with it, but I don't feel strongly.

Seems good so far. I find expect sounds nicer, but less strict than mandate. When tasked with a name for this from scratch, I'd probably come up with expect.

However, seeing as mandate internally calls permit, it is safe to assume this does not play nicely with action_on_unpermitted_params...
According to (quite old) #26831, it would be fine to have this on a per-controller basis.
Or maybe, remove logging/raising from permit and instead implement permit_only_or_raise, and permit_only_or_log (quite verbose, but the idea is clear)

@dari-us maybe params.expect_only that would raise.

The existing configuration makes sense. Logging can be useful in dev but would be extra noise in most production applications. This implies to me that logging is an environment config and raising should be controlled by how the methods are called. Please correct me if you think otherwise.

If we added expect_only and permit_only (for a non-require version) that would allow the use case in #26831. These would be methods that would raise UnpermittedParameters. expect and permit would continue to log or raise depending on the global config, allowing an app that wants both to set globally, and use the _only methods selectively. I suggest doing this in a follow-up PR that I can certainly help with.

Regarding your comment @MatheusRich, I never got to respond.

I wonder if we should just handle these NoMethodErrors differently. It feels weird to return a 500 when the error is the "the user's fault". Maybe we could return a 4XX somehow?

This was what motivated me to create this. It's certainly wrong to return 500 when the client caused the problem, but once require(:key) returns the value of params[:key] we're in plain ruby land and a NoMethodError is bound to occur if you call permit next. I think we must just remove all uses or mentions of the pattern require(:key).permit(:attr) and switch to this new method. I aim to do that here to the extent possible.

permit(key: [:attr]).require(:key) is how rubygems.org now processes all params and it has removed an annoying class of 500s that used to get us paged occasionally. I will to convert to this once it's released.

I switched the name from mandate to expect. I used a single commit so we could revert if we decide not to go that route. I personally prefer it.

I'm doing one more pass on the guides to try to find non-obvious uses. Otherwise this is ready for code review (and please call out any documentation that you think should be fixed, if I missed it)

I'm working on fixing the test failures.

@martinemde Yeah, global config for logging and raising/ignoring via method call is fine by me and I think makes sense for the broad majority of apps.
What I mean in particular with "does not play nicely" is this case:

action_on_unpermitted_params = :raise
# route PATCH /users/1 => users#update
params.expect(user: [:name, :age]) # raises, b/c params contains unpermitted param :id

I think at this point, :raise basically kills your app and :log produces lots of noise. Also in a nested context like /groups/1/users, the create action also logs/raises.
To improve the situation, one would probably move the actual filtering that permit does into an internal method, say, apply_filters, and then expect could use that, so unpermitted_keys! is not called automatically.
Also, unpermitted_keys! needs to accept an argument to trigger :raise by hand and use global config as fallback.

On another note, sadly the use case how to replace params.fetch(:x, {}).permit(...) is not covered, but I guess this is out of scope for this PR. At least I don't know a good way how to incorporate it.

Also, I would love to make a followup PR to

1. Account for action_on_unpermitted_params
2. enable devs to :raise via *_only
3. enable the params.fetch(:x, {}).permit(...) pattern (maybe this could be named accept and accept_only, as in, "it is accepted, but not expected").

Hopefully I can spare some time on that soon.

@dari-us, ok, I understand now what you mean. The fact of how permit works when applied to params "root" means it would consider any other params to be unpermitted. This explains something I was noticing on rubygems.org, which is that dev was logging more unpermitted params than usual.

Possible fix, but maybe too ugly: id, nested_id, table_params = params.expect(:id, :nested_id, table: [:attrs])
I think maybe your proposed solution is the best. Let's consider this a bug with this implementation until we can fix it, since it's not the expected outcome.

Edit: thinking ahead about expect_only, the way to make attributes required and permitted, while still raising on nested params that are not permitted, you would do: expect(user: {}).expect_only(:name, :age)

As for the other methods, I think you're right. The fetch.permit pattern could be allow(user: [:name, :age]) which would return {} on missing key. Then it uses the same pattern as expect but communicates that, just like many mocking frameworks, allows might or might not happen and that's ok. (I prefer allow over accept because accept sounds a lot like expect, just transpose a syllable).

Excellent work, guys! This is a very nice upgrade.

We'll need to also upgrade the templates in the jbuilder gem, since they're used by default. Probably have to have a Rails version toggle where we use expect when 8.0+ and whatever is there before that. @martinemde want to take a stab at that too?

Nice change, silents some noise in bug notifications. Should there be a rubocop rule for it?

Nice change, silents some noise in bug notifications. Should there be a rubocop rule for it?

No. Don't need to nag people about this. They can upgrade to the new syntax as they see fit and new apps will automatically guide folks in that direction.

Nice API!
I'm wondering if expect is a little too close to except, and could cause confusion and typo's?
Not sure if the previously mentioned allow is completely out of the picture?
Or maybe with, to counter the without alias?

@p8 I hardly see that as a problem, at least I cannot remember ever seeing except used with params.
allow sounds less strict, i.e. I wouldn't expect it to raise for missing root keys.
I would love to have allow as a replacement for params.fetch(:x, {}).permit(...) sometime, as this pattern appears in the docs, I often use it for "filter forms" and is not fixed by the expect API.

I can see non-native English speakers or people with dyslexia having problems with this - copying the guide documentation and wondering why things aren't working as expected.

As this API seems to be more strict than require and permit, maybe a verb that has a more strict meaning would be clearer?

For example constrain or contraints, which has overlap with routing contraints, which already does parameter restriction.

Appreciate the feedback here, but this bikesheed has already been painted, @p8. We're moving on to other bikesheeds now 😄

I took a stab at updating the jbuilder templates here: rails/jbuilder#573.

Also fixed a minor Rubocop issue introduced by this PR here: #52928.