Support named parameters in allowParams* config keys

[ilazaridis]

Close #140

[ilazaridis]

great, thank you.
Do you think we will get this baby soon out ?

[spaze]

It will take some time, sorry. I'd like to add a thing or two which would allow a param when used as a named param or as a positional param. Currently that's not possible and may require a different config format so I don't want to release this and then change the format.

I'm thinking about

allowParam:
    4:
        value: foo
        name: flags

Or something like that. In the future another key might be added, I'm thinking type: flag that would check whether a flag is present in FLAG1 | FLAG2 param (#142)

[ilazaridis]

It will take some time, sorry. I'd like to add a thing or two which would allow a param when used as a named param or as a positional param. Currently that's not possible and may require a different config format so I don't want to release this and then change the format.

I'm thinking about

allowParam:
    4:
        value: foo
        name: flags

Or something like that. In the future another key might be added, I'm thinking type: flag that would check whether a flag is present in FLAG1 | FLAG2 param (#142)

Sure, take your time.
Checking for one specific flag in the list of flags used makes sense, I would also go for that instead of checking the exact match.
Regarding the position 4, how do you have that in mind ? Because, I am a bit confused now. :D

[spaze]

Regarding the position 4, how do you have that in mind ?

Let's take the example you've added, it's a perfect example :-) (and let's forget about the flags param type problem mentioned earlier):

json_decode('foo', flags: JSON_THROW_ON_ERROR);

so you'd configure something like this:

allowParam:
    flags: ::JSON_THROW_ON_ERROR

but then you or someone else, would add a code like

json_decode('foo', false, 123, JSON_THROW_ON_ERROR);

note no named params. The constant usage wouldn't be detected and the call would be disallowed.

So you'd need to write a code like

json_decode('foo', false, 123, flags: JSON_THROW_ON_ERROR);

(name the param) and I don't think that's nice.

A configuration like

allowParam:
    4: ::JSON_THROW_ON_ERROR
    flags: ::JSON_THROW_ON_ERROR

doesn't work either because the list is not an OR list and for the call to be allowed, you'd need to specify both params which is sort of impossible.

So my idea is to somehow turn the list into an OR list and because I'll be adding a "type" field anyway (for the flags problem above), I think it makes sense to also add a "named" field:

allowParam:
    4:
        value: foo
        name: flags

The code would look for param on position 4 if its value is foo, OR would look for a named param flags and check the value. So both the following calls would be allowed:

json_decode('foo', flags: JSON_THROW_ON_ERROR);
json_decode('foo', false, 123, JSON_THROW_ON_ERROR);

I realized this is probably what many people would expect when addind tests here.

As params are alredy objects, adding this shouldn't be very difficult, just a little bit 😅

Hope that helps :-)

(And just for the record, I don't expect you to add those features, I'll happily add those myself, building upon your contribution, thanks for that 🙏)

[spaze]

Thanks for the initial implementation! I've added what I've described above and merged this now. I'll probably release it when I have the flags support (#142) unless you're in a hurry :-)

[spaze]

Just released in 2.11.0, let me know if it works for you. Thanks 👍