# [Discussion] Expand deprecation notice scope for partially supported callables

- Thread: https://externals.io/message/117720
- Original archive: https://news-web.php.net/php.internals/117720
- Started by Juliette Reinders Folmer on 2022-05-12T16:54:39+00:00
- Messages: 11
- RFC: Expand deprecation notice scope for partially supported callables (Implemented (PHP 8.2)) — https://externals.io/rfc/partially-supported-callables-expand-deprecation-notices.md
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #117720 — Juliette Reinders Folmer — 2022-05-12T16:54:39+00:00

URL: https://externals.io/message/117720#117720

After the prior discussion about the same topic: 
https://externals.io/message/117342, I have created an RFC to expand the 
scope of the deprecation notices being thrown for the deprecated 
partially supported callables to include `is_callable()` and the callable 
type in PHP 8.2.

With this email I'm opening the two week discussion period for this RFC. 
All points raised in the prior discussion are already included in the RFC.

https://wiki.php.net/rfc/partially-supported-callables-expand-deprecation-notices

I look forward to your feedback.

Smile,
Juliette

---

## #117722 — Larry Garfield — 2022-05-12T21:11:07+00:00

URL: https://externals.io/message/117720#117722 · In reply to: #117720

> After the prior discussion about the same topic: 
> https://externals.io/message/117342, I have created an RFC to expand the 
> scope of the deprecation notices being thrown for the deprecated 
> partially supported callables to include `is_callable()` and the callable 
> type in PHP 8.2.
>
> With this email I'm opening the two week discussion period for this RFC. 
> All points raised in the prior discussion are already included in the RFC.
>
> https://wiki.php.net/rfc/partially-supported-callables-expand-deprecation-notices
>
> I look forward to your feedback.
>
> Smile,
> Juliette

I didn't follow the earlier discussion in much detail, but the `is_callable()` deprecation seems fine to me.

For the `callable` type declaration, I'm not opposed but is it redundant with the existing deprecation?  When would you pass a callable to something and not end up calling it anyway, which would trigger the existing deprecation?  (Meaning in practice you'd always get 2 deprecations, which are not necessarily better than one.)

--Larry Garfield

---

## #117723 — Claude Pache — 2022-05-12T21:30:26+00:00

URL: https://externals.io/message/117720#117723 · In reply to: #117722

> Le 12 mai 2022 à 23:11, Larry Garfield <larry@garfieldtech.com> a écrit :
> 
> For the `callable` type declaration, I'm not opposed but is it redundant with the existing deprecation?  When would you pass a callable to something and not end up calling it anyway, which would trigger the existing deprecation?  (Meaning in practice you'd always get 2 deprecations, which are not necessarily better than one.)


Although such a callable is probably intended to be called at some point, it is not necessarily called immediately, and it may be easier to track the source of it when you trigger the deprecation earlier. Example:

```php
public function registerErrorHandler(callable $onerror) {
    $this->onError[] = $onerror;
}
```

(Of course, this argument also holds for `is_callable()`.)

—Claude

---

## #117725 — Juliette Reinders Folmer — 2022-05-12T22:17:04+00:00

URL: https://externals.io/message/117720#117725 · In reply to: #117723

>
> > […]
> Although such a callable is probably intended to be called at some point, it is not necessarily called immediately, and it may be easier to track the source of it when you trigger the deprecation earlier. Example:
>
> ```php
> public function registerErrorHandler(callable $onerror) {
>      $this->onError[] = $onerror;
> }
> ```
>
> (Of course, this argument also holds for `is_callable()`.)
>
> —Claude

Exactly as Claude says and to quote the RFC:

 > While it will be common to use the partially supported callable in a 
callback function call within the function with the type declaration, 
this may not always be the case, especially in frameworks where 
callbacks can be registered to be executed later in a limited set of 
circumstances and those circumstances may not be met by default.

 > Without a deprecation notice, those “/limited circumstances 
callbacks/” may not be discovered as needing updating in time for PHP 9.0.

Ref: 
https://wiki.php.net/rfc/partially-supported-callables-expand-deprecation-notices#adding_a_deprecation_notice_for_callable

Smile,
Juliette

---

## #117789 — Juliette Reinders Folmer — 2022-05-26T11:45:31+00:00

URL: https://externals.io/message/117720#117789 · In reply to: #117720

> After the prior discussion about the same topic: 
> https://externals.io/message/117342, I have created an RFC to expand 
> the scope of the deprecation notices being thrown for the deprecated 
> partially supported callables to include `is_callable()` and the 
> callable type in PHP 8.2.
>
> With this email I'm opening the two week discussion period for this 
> RFC. All points raised in the prior discussion are already included in 
> the RFC.
>
> https://wiki.php.net/rfc/partially-supported-callables-expand-deprecation-notices 
>
>
> I look forward to your feedback.
>

Seeing how there isn't any new discussions on this RFC and presuming 
that means that the previous discussion touched all concerns, I propose 
to open the vote on this RFC tomorrow.

RFC: 
https://wiki.php.net/rfc/partially-supported-callables-expand-deprecation-notices

If anyone still wants to know a little more about the background, you 
may want to have a listen to Derick's podcast about this RFC: 
https://phpinternals.news/101

Smile,
Juliette

---

## #117794 — Dan Ackroyd — 2022-05-26T18:23:08+00:00

URL: https://externals.io/message/117720#117794 · In reply to: #117789

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

---

## #117814 — Juliette Reinders Folmer — 2022-05-28T08:22:20+00:00

URL: https://externals.io/message/117720#117814 · In reply to: #117794

> Hey Julie,
>
> On Thu, 26 May 2022 at 12:45, Juliette Reinders Folmer
> <php-internals_nospam@adviesenzo.nl> wrote:
> > […]
> 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:
> > […]
> 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

---

## #117824 — Dan Ackroyd — 2022-05-29T15:34:49+00:00

URL: https://externals.io/message/117720#117824 · In reply to: #117814

Hi Julie,

On Sat, 28 May 2022 at 09:22, Juliette Reinders Folmer
<php-internals_nospam@adviesenzo.nl> wrote:
>
> 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.

Cool.

Actually, I've just realised there is an error in the code in the RFC,
which might be based on a misconception caused by how terrible
callables are. In the code:

if (is_callable('static::methodName')) {
    // For valid callbacks, this call will be executed in PHP 8.x,
    // but will no longer execute in PHP 9.x.
    static::methodName();
}

The string based callable 'static::methodName' is not equivalent to
the syntax* based callable static::methodName().

Using the string version consistently, the equivalent code would be:

if (is_callable('static::methodName')) {
    call_user_func('static::methodName', []);
}

which for 8.2 gives the message 'Deprecated: Use of "static" in
callables is deprecated in %s on line %d'. btw trying to call
('static::methodName')(); gives the error message 'Uncaught Error:
Class "static" not found in %s:%d' which is part of the consistency
cleanup done by the previous RFC.

Using the syntax version, the equivalent code that would compatible
with PHP < 8.1

if (is_callable(static::class . '::methodName')) {
    static::methodName();
}

Or if support for less than PHP 8.1 can be dropped, using the first
class callable syntax
https://wiki.php.net/rfc/first_class_callable_syntax :

if (is_callable(static::methodName(...))) {
    static::methodName();
}

or

$fn = static::methodName(...);
if (is_callable($fn)) {
    $fn();
}

Passing the callable round by getting the closure from
static::methodName(...) is probably the safest way of referencing this
type of callable.

None of the syntax based ways of referring to the callable are
deprecated or going to be removed in the foreseeable future.


> for the practical implications of fixing the deprecations,
> there is no difference between the two.

People don't have to fix their code. Deprecations can sit there for as
long as the user likes. If a company decides that paying RedHat for
long term PHP 8.2 support, then they may choose to never fix these
deprecation warning and just silence them instead.

Which leads to a difference of, the deprecation notice when checking
with is_callable and using the callable can be suppressed reasonably
easily:

@is_callable('static::methodName')
@call_user_func('static::methodName', []);

And that's a reasonably sane** thing to do. But the deprecation notice
when passing callables around could happen across many pieces of code,
and there's not a good way of suppressing them, unless you just turn
off deprecation notices entirely.

With the deprecation notice where a user is checking, and a
deprecation notice where it is used, I don't see any value in extra
deprecation notices where the callable is being passed around.

> do you still feel that splitting the vote in two is the best way to go ?

Yes, due to the deprecation notices on type-checks when calling
functions have a higher pain-to-utility that in the is_callable check.

Just guessing, I think the previous RFC thought a deprecation notice
on is_callable isn't needed, as there will be a deprecation notice
when the callable is used. But that probably didn't account for people
being able to mix'n'match callable syntaxes, where is is_callable
check, and the actual use of the callable, are not the same callable.

I also like the deprecation notice on is_callable, as that notice will
be 'closer' to where the bad callable is coming from, so would be
easier to reason about. There's also a rare edge-cases where someone
has a callable that is only called in emergencies (like a disk running
out of space) and so might not have that happen for months. Having the
deprecation on is_callable would help those edge-cases a little.

cheers
Dan
Ack

* Is "syntax based callable" the right name? Better suggestions welcome.

** compared to some stuff I've seen/written.

---

## #117827 — Dan Ackroyd — 2022-05-29T17:01:22+00:00

URL: https://externals.io/message/117720#117827 · In reply to: #117824

>
> *an incorrect name*

Apologies for writing your name incorrectly. That should of course
have been addressed to Juliette.

cheers
Dan
Ack

---

## #117830 — Juliette Reinders Folmer — 2022-05-29T19:51:18+00:00

URL: https://externals.io/message/117720#117830 · In reply to: #117824

Hi Dan,


> Actually, I've just realised there is an error in the code in the RFC,
> which might be based on a misconception caused by how terrible
> callables are. In the code:
> if (is_callable('static::methodName')) {
>      // For valid callbacks, this call will be executed in PHP 8.x,
>      // but will no longer execute in PHP 9.x.
>      static::methodName();
> }
> The string based callable 'static::methodName' is not equivalent to
> the syntax* based callable static::methodName().
>
> ...

This is actually not an error in the RFC, but an anonymized code sample 
based on real-life code patterns found in popular packages.

While the syntaxes are not technically equivalent, functionally, they 
yield the same result, which is why this code pattern is not uncommon.

It is exactly this type of code pattern - and the associated 
misconception leading to this code existing in real life code bases -, 
which started the initial discussion about the lack of deprecation 
notices for `is_callable()` and led to this RFC.

> > […]
> People don't have to fix their code. Deprecations can sit there for as
> long as the user likes. If a company decides that paying RedHat for
> long term PHP 8.2 support, then they may choose to never fix these
> deprecation warning and just silence them instead.
> Which leads to a difference of, the deprecation notice when checking
> with is_callable and using the callable can be suppressed reasonably
> easily:
> @is_callable('static::methodName')
> @call_user_func('static::methodName', []);
> And that's a reasonably sane** thing to do. But the deprecation notice
> when passing callables around could happen across many pieces of code,
> and there's not a good way of suppressing them, unless you just turn
> off deprecation notices entirely.
> With the deprecation notice where a user is checking, and a
> deprecation notice where it is used, I don't see any value in extra
> deprecation notices where the callable is being passed around.

IMO that's a false argument as deprecation notices are not intended for 
people who don't intend to upgrade their PHP version.

If there is no intention to upgrade to PHP 9.0, the much simpler 
solution would be to set `error_reporting` to `E_ALL & ~E_DEPRECATED`.
Alternatively, a custom error handling could be registered which filters 
out all, or only a selection of, deprecation notices.

Deprecation notices are for those people who _do_ want to upgrade to PHP 
9.0 once it comes out and want to prepare their code base to be ready.

And for those people, the `callable` type not throwing a deprecation 
notices means that for the "registered, but rarely called" callables, 
they will go from _nothing_ to a Fatal Error once PHP 9.0 comes round, 
which is exactly what this RFC tries to address.

Either way, I do agree that this potential objection should be heard, so 
I have added an extra section to the "Discussion" section of the RFC in 
which I have summarized our discussion (so far) about this: 
https://wiki.php.net/rfc/partially-supported-callables-expand-deprecation-notices#these_additional_deprecation_notices_will_be_very_noisy

> > […]
> Yes, due to the deprecation notices on type-checks when calling
> functions have a higher pain-to-utility that in the is_callable check.
Fair enough, I will split the vote and have updated the RFC to show this.

> I also like the deprecation notice on is_callable, as that notice will
> be 'closer' to where the bad callable is coming from, so would be
> easier to reason about. There's also a rare edge-cases where someone
> has a callable that is only called in emergencies (like a disk running
> out of space) and so might not have that happen for months. Having the
> deprecation on is_callable would help those edge-cases a little.
Just pointing out, the same thing can happen with the `callable` type - 
a callable being registered for an emergency (like a disk running out of 
space), but not being called for months. This example edge case is not 
limited to `is_callable()`.

I hope the RFC update addresses your concerns sufficiently. If there are 
no further objections, I will open the vote.

Smile,
Juliette

---

## #117836 — Guilliam Xavier — 2022-05-30T14:56:44+00:00

URL: https://externals.io/message/117720#117836 · In reply to: #117824

Hi Dan, just an error in this part:

> Or if support for less than PHP 8.1 can be dropped, using the first
> class callable syntax
> https://wiki.php.net/rfc/first_class_callable_syntax :
>
> if (is_callable(static::methodName(...))) {
>     static::methodName();
> }
>
> or
>
> $fn = static::methodName(...);
> if (is_callable($fn)) {
>     $fn();
> }

This throws an Error on `static::methodName(...)` [if not callable],
before even passing it to `is_callable()`: https://3v4l.org/m29BK

And yes, code should probably better use a same variable for both the
check and the call, but that could also degrade DX (IDE
autocompletion, static analysis) especially if calling with
arguments...

(the other parts of your message have been answered by Juliette)

Regards,

-- 
Guilliam Xavier