CommandFactory: reject named arguments as those are not supported#1508
CommandFactory: reject named arguments as those are not supported#1508janedbal wants to merge 3 commits into
Conversation
Adjusted. The remaining failures are unrelated. |
|
This feels a bit overkill, especially since we merged #1509? Mainly because we don't advertise named parameters or include them inside of the stubs. So I assume someone just came up with |
@tillkruss Not everybody uses PHPStan, so I think your library should either:
|
I don't see this as a BC issue, was this feature listed anywhere ever or did it just happen to work? |
I believe users are expecting native PHP features to work.
If my code was working fine before upgrade, but fails hard on |
|
If you dislike this PR, I can send PR fixing the broken support of named args. |
|
My point is it's not a breaking change, because it's not an officially supported feature, nor are there tests for it. It just happened to have worked. If you want to restore named args support, please do. |
|
@vladvildanov Any thoughts on this? |
Since predis 2.3.0, named arguments are not supported (at least in
SETcommand) due to this change as thevalueargument does not meet the condition there.Thus, following code broke our production upon upgrade:
At first, I was about to fix this by supporting it inside
SET::setArguments, but as all the command calls are implemented via__call, I believe it is safer to rely on the fact that your users are not using named arguments at all.