Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117830
References: <627D3BCF.7020903@adviesenzo.nl> <628F685B.8090909@adviesenzo.nl>
 <CA+kxMuQb3OXV-zSQBBiVXHgSt+u06MBOZgw-xqgMEJ05_i+ebw@mail.gmail.com>
 <6291DBBC.8020403@adviesenzo.nl>
 <CA+kxMuT0XWyL6NjEZKfUH62SmEZiiau7R7Sa4jZYuJm9iP-Twg@mail.gmail.com>
Cc: Dan Ackroyd <Danack@basereality.com>
To: internals@lists.php.net
Message-ID: <6293CEB6.10209@adviesenzo.nl>
Date: Sun, 29 May 2022 21:51:18 +0200
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: <CA+kxMuT0XWyL6NjEZKfUH62SmEZiiau7R7Sa4jZYuJm9iP-Twg@mail.gmail.com>
Content-Type: multipart/alternative;
 boundary="------------060409080502070608040002"
Subject: Re: [PHP-DEV] [Discussion] Expand deprecation notice scope for
 partially supported callables
From: php-internals_nospam@adviesenzo.nl (Juliette Reinders Folmer)

--------------060409080502070608040002
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit


Hi Dan,

On 29-5-2022 17:34, Dan Ackroyd wrote:
> 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.

>> 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.

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

>> 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.
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

--------------060409080502070608040002--