Hi
On 01.05.24 12:26, Larry Garfield wrote:
This looks good to me, with one remaining exception, which isn't specific to
this function but should still be discussed: Always passing the value and key
to the callback is unsafe, for two reasons.
1. If the callback is an internal function rather than a user-land one, and it
has only one argument, it will error confusingly. That makes the current
implementation incompatible with unary built-in functions. See, for instance,
https://www.php.net/is_string (and friends)
I think, that this problem can easily be detected with static analysers.
Currently neither PHPStan [1] nor psalm [2] does detect this issue, but
as the tools already validate the signature (e.g. str_contains is
rejected) this can probably be integrated and might even be considered a
bug.
The proper fix from PHP's side would be something like the proposal in
https://externals.io/message/122928 (RFC idea: using the void type to
control maximum arity of user-defined functions).
2. If the callback takes two arguments but the second is optional, it's highly
unlikely that the key is the value expected as the second argument. This could
lead to confusingly hilarious errors. See, for instance,
https://www.php.net/intval.
These won't come up in the typical case of passing an inline closure (either
short or long form), but are still hidden landmines for anyone using functions
not tailor made for these functions.
I see the problem, but don't think, that this is a problem, that we can
solve. As a note: Such problems exists in JavaScript, too [3] and is
handled in the same way.
I'm not sure of a good solution here, honestly, so I don't know what to
recommend. In Crell/fp, I ended up just making two different versions of the
function that pass one or two arguments. I don't think that's a good answer
for this RFC, but I'm not sure what is. At the very least, it should be
mentioned as a known-limitation that gets documented., unless we can come up
with something better.
I have added this problem in the section "Open Issues" in the RFC to
document this behavior.
[1] https://phpstan.org/r/bd0866cd-6a76-4c18-8eb3-0f3848de7f4a
[2] https://psalm.dev/r/1baa3f8e0d
[3] https://wirfs-brock.com/allen/posts/166
