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
