Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117374
To: "Christoph M. Becker" <cmbecker69@gmx.de>,
 PHP internals <internals@lists.php.net>
References: <62317B01.90907@adviesenzo.nl>
 <70c5d418-7472-7f87-dc2c-e1a60ee09df6@gmx.de>
Message-ID: <6234BB7F.90507@adviesenzo.nl>
Date: Fri, 18 Mar 2022 18:03:59 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.0
MIME-Version: 1.0
In-Reply-To: <70c5d418-7472-7f87-dc2c-e1a60ee09df6@gmx.de>
Content-Type: multipart/alternative;
 boundary="------------030808070708090502020305"
Subject: Re: Deprecated partially supported callables: should is_callable()
 throwa deprecation notice ?
From: php-internals_nospam@adviesenzo.nl (Juliette Reinders Folmer)

--------------030808070708090502020305
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On 18-3-2022 14:37, Christoph M. Becker wrote:
> On 16.03.2022 at 06:52, Juliette Reinders Folmer wrote:
>> I've just been looking in detail at the Partially Supported Callables
>> deprecation RFC:
>> https://wiki.php.net/rfc/deprecate_partially_supported_callables
>> The RFC explicitly excludes the `is_callable()` function and the
>> `callable` type from throwing deprecation notices.
>>> The |is_callable()| function and |callable| type remain side-effect
>>> free and do not throw a deprecation warning. They will continue to
>>> accept these callables until support is removed entirely.
>> While I can fully support this for the `callable` type, I wonder if the
>> decision to not throw a deprecation on use in `is_callable()` is the
>> right one (though I understand the desire to keep it side-effect free).
>> Consider these code samples:
>>    function foo(callable $callback) {}
>>    foo('static::method');
>> This function call not throwing a deprecation is not problematic as in
>> PHP 9.0 the function will start throwing a TypeError.
>>    if (is_callable('static::method')) {
>>        static::method();
>>    }
>> The second code sample, however, is problematic, as in PHP 9.0, the
>> behaviour of this code will be silently reversed for those callbacks
>> which would previously result in `is_callable()` returning true, which
>> makes this a potentially dangerous change without deprecation notice.
>> Would anyone care to enlighten me as to whether this was given due
>> consideration ?
> Frankly, I don't know.  Apparently, there was almost no discussion about
> that RFC.  Part of the reasoning to not raise E_DEPRECATED when calling
> is_callable() was likely the typical use case
>    $callable = …;
>    if (is_callable($callable)) {
>        call_user_func($callable);
>    }
> what would report the deprecation when actually calling the callable.
> Not sure what to do regarding your given use case(s).
> --
> Christoph M. Becker

Unfortunately, those aren't the only places I see this happening and my 
example is not a stand-alone case either.

I came across the above code sample ( is_callable('static::method') / 
is_callable(['parent', __FUNCTION__]) with variable method calls) in a 
number of different packages when testing a (PHPCompatibility) sniff I 
am writing to detect the deprecation, so this code pattern looks to be 
relatively common.

Some examples:
* 
https://github.com/symfony/service-contracts/blob/bc0a2247c72d29241b5a06fb60dc1c9d9acf2a3a/ServiceSubscriberTrait.php#L39
* 
https://github.com/mockery/mockery/blob/c10a5f6e06fc2470ab1822fa13fa2a7380f8fbac/library/Mockery/Mock.php#L960
* 
https://github.com/simplepie/simplepie/blob/dacf0ed495d2e8fb306e526ca3f2a846af78a7c9/tests/oldtests/absolutize/RFC3986.5.4/base.php#L13


--------------030808070708090502020305--