Re: [Discussion] Expand deprecation notice scope for partially supported callables

On 26-5-2022 20:23, Dan Ackroyd wrote:

Hey Julie, On Thu, 26 May 2022 at 12:45, Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl> wrote:

I propose to open the vote on this RFC tomorrow.

Before you open the vote, please could you split the voting into two, one for the is_callable, and one for the callable type check? Rowan wrote in https://news-web.php.net/php.internals/117670:

is that passing "99 red balloons" to an "int" parameter raised an E_NOTICE in PHP 7.x, so a "callable" parameter raising an E_DEPRECATED should be fine.

There's one issue. When "99 red balloons" is coerced to an int, that is done once, and then any further int type check will pass. For callables, it's pretty common to pass them down a stack of code, e.g. similar to: function foo(callable $fn, $data) {

     $fn($data);

} function bar(callable $fn, $data) {

     return foo($fn);

} function quux(callable $fn, $data) {

     return bar($fn, $data);

} function main(array $data) {

     $fn = get_callable_from_input();
     if (is_callable($fn) === false) {

// give some error.

         return;
     }

     quux($data);

} As far as I understand it, this code would give a deprecation notice for each call level, which seems quite undesirable. Particularly if the callable is being used in a loop. Also, without a patch it's hard to guess what performance impact this would have. I doubt it would be huge, but it probably wouldn't be zero either. Performance wouldn't matter for is_callable, as that is typically only done once per callable, but callable type checks are done continuously. cheers Dan Ack

Hiya Dan, I admit, I puzzled over this for a little and wanted to take the time to respond properly before opening the vote, so I'm delaying the start of the vote until beginning of the upcoming week. In my first draft of the RFC I actually had two votes, but once it shaped up this was brought down to one vote. Reason being, that for the practical implications of fixing the deprecations, there is no difference between the two. In your example, you suggest a variable being passed between functions all using the callable type. The same can be encountered with functions without the callable type, but using manual defensive coding via is_callable() - typical example: a collection of callbacks being checked before being registered to a callback stack and being checked again before actually being called as the stack may have been manipulated from outside. Yes, having each of those instances throw a deprecation notice will be more noisy, especially if the same deprecated callback is used in a loop and yes, this will have a (small) performance impact while these callbacks aren't fixed yet, but the same *one* fix - at the point where the problematic callback is defined - will fix them all in one go, so the amount of fixes to be made does not change, but the chances of _identifying_ all the places were a fix is _needed_ is greatly improved. In that sense, it is no different from the "99 red balloons" case when those issues are solved at the _source_ of the problem. Patching the "99 red balloons" by applying (int) casts at the point the deprecation is thrown, will lead to a codebase full of type casts, while the underlying problem - the origin of the text string - is not addressed (and in the case of the callbacks, the origin is the only place they _can_ realistically be solved). Considering the above, do you still feel that splitting the vote in two is the best way to go ? Smile, Juliette