Deprecated partially supported callables: should is_callable() throw a deprecation notice ? View as Markdown

The point is that this is not an usual deprecation, something that will
change to an error in the future.
In the end, it's just a change in behavior with no error before or after.
It does not fit the "deprecation".

As I dealt with several PHP upgrades, and I'm still going to deal with them
in the future, I proposed an idea at some point on the mailing list:
To find a way to "flag" a behavior change.
The problem is that it should be something outside E_ALL, or to be able to
handle it in a totally different way than setting an error handler for it.
Something that can be used when preparing for an PHP upgrade, something
like:
register_version_change_handler(callable $callback, $toPhpVersion)
While preparing for a future runtime version, it would be deployed to
production and would record behavioral changes in a logging system.
These changes need to be manually reviewed, of course, as a change can be
to return false instead of null and if that value would be used further in
an if.
When using in a library maybe there could be some token like @@@ to not
trigger the handler.

To me, adding another error/change handler system seems like a bad way
of dealing with this, not to mention that you need to introduce this
behavior in a PHP version first before anyone can then use it, so it
would be years before this could be useful, and even more years until
there would be established patterns and enough knowledge around it.
Using the existing way of notices, warnings and errors seems just as
good to me, and people have been handling those for many years. Any new
way of dealing with this should have a very high bar to clear and would
need to prove a huge gain in value.

To me this does fit the definition of a deprecation - behavior for the
given code will change intentionally in a future version, and you are
warning about that future change. But one could also emit a notice, or a
warning. The advantage of deprecation notices is that many tools ignore
those specifically, as they are something to look at, but not
necessarily right at this moment. Being more lenient in certain
situations with deprecation notices is therefore a good pattern for
easier adoption of new versions. I have included that behavior in my own
applications, by logging deprecations separately - I will check them out
once in a while, but they do not need immediate attention. Any other PHP
notice or warning would warrant immediate attention, as it would be
unexpected.

I too would rather have "extra" deprecation notices in 8.2 than
sudden errors / silent behavior changes in 9.0 (for the callable
type declaration / the is_callable() function)...

The point is that this is not an usual deprecation, something that will change to an error in the future.
In the end, it's just a change in behavior with no error before or after. It does not fit the "deprecation".

This has already been said earlier, and answered:

Le 19 avr. 2022 à 20:20, Andreas Hennings andreas@dqxtech.net a écrit :
A deprecation warning on is_callable() would imply that in a future
version of PHP that call will be illegal.

No, in the case of is_callable(), the deprecation warning will imply that, in a future version of PHP, the behaviour will change. There are precedents of deprecation warning for changing behaviour: https://3v4l.org/Iqo4N

Regards,

--
Guilliam Xavier

The point is that this is not an usual deprecation, something that will
change to an error in the future.
In the end, it's just a change in behavior with no error before or after.
It does not fit the "deprecation".

That was my instinct too, but Juliette's analysis elsewhere on the
thread has convinced me that it is very likely that anyone seeing this
deprecation would want to change their code, rather than allowing it to
change behaviour in 9.0.

Perhaps a reasonable comparison is the change in precedence of the
concatenation operator
[https://wiki.php.net/rfc/concatenation_precedence]. The expression
("sum: " . $a + $b) is valid in PHP 7 and PHP 8, but has different
results; PHP 7.4 included a deprecation notice because users would
almost certainly want to choose which behaviour they wanted.

The difference in this case is that the code change probably won't
happen in the same place as the is_callable() function call, but
somewhere further up the stack. For instance, a framework function might
accept arbitrary values and test them with is_callable; an application
using that framework might pass "parent::foo", which will change
behaviour in 9.0. It is the application's author who will benefit from
the deprecation notice, so they can pass a different value whose
behaviour isn't going to change.

Regards,

--
Rowan Tommins
[IMSoP]

A deprecation warning on is_callable() would imply that in a future
version of PHP that call will be illegal.
But this is not the case.
is_callable() is meant to accept any value, and return true or false.
is_callable('static::method') will still be a valid call in future
versions, only the result will be different.
This change simply cannot be covered with a deprecation warning.
Developers have to update their code when they upgrade to PHP 9.x, or
they need a PHP_VERSION_ID check, as sad as that is.

One solution could be to accept an additional parameter to enable the
forward behavior for is_callable().
But then this parameter would have to be removed again later.
Also, is_callable() already has two extra parameters, so it would get crowded.

-- Andreas

Le 18 mars 2022 à 18:03, Juliette Reinders Folmer php-internals_nospam@adviesenzo.nl a écrit :

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

As I think that it is a serious oversight of the RFC, I have open:

https://github.com/php/php-src/issues/8401 https://github.com/php/php-src/issues/8401

—Claude