Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On 05/05/2022 06:37, Juliette Reinders Folmer wrote: I was going to mention the same example as the precedent ;-) Another relevant precedent, since the original RFC talked about keeping things "side-effect free", 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. Either way, how will this get picked up now ? What are the next steps to put this thing in motion ? (Note: please don't suggest for me to create a PR - I'm a PHP dev, not a C-dev) If nobody else does, I can have a look at the implementation, which I think should be fairly straight-forward. However, since the current behaviour was explicitly mentioned in the accepted RFC, and the response here wasn't immediately unanimous, it's probably sensible to write it up as its own RFC to be voted on. It doesn't need to say much, just summarise the key points from this discussion. See https://wiki.php.net/rfc/howto for the steps, and feel free to send me a message off-list if you want a hand drafting or proof-reading it. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] Re: ***SPAM*** Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On 2-5-2022 14:43, Rowan Tommins wrote:
On 02/05/2022 12:56, Alexandru Pătrănescu wrote:
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.
I was going to mention the same example as the precedent ;-)
Either way, how will this get picked up now ? What are the next steps to
put this thing in motion ?
(Note: please don't suggest for me to create a PR - I'm a PHP dev, not a
C-dev)
Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On 02/05/2022 12:56, Alexandru Pătrănescu wrote:
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]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On Mon, May 2, 2022 at 1:57 PM Alexandru Pătrănescu wrote: > On Mon, May 2, 2022 at 2:15 PM Guilliam Xavier > wrote: >> 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: On Wed, Apr 20, 2022 at 12:22 AM Claude Pache wrote: > > Le 19 avr. 2022 à 20:20, Andreas Hennings 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On 02.05.22 13:56, Alexandru Pătrănescu wrote: 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. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On Mon, May 2, 2022 at 2:15 PM Guilliam Xavier
wrote:
> On Wed, Mar 16, 2022 at 9:57 AM Christian Schneider
> wrote:
> >
> > Am 16.03.2022 um 06:52 schrieb Juliette Reinders Folmer <
> php-internals_nos...@adviesenzo.nl>:
> > > 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.
> > >
> > > [...] I wonder if the decision [...] is the right one (though I
> understand the desire to keep [them] side-effect free).
> > >
> > > Consider these code samples:
> > >
> > > function foo(callable $callback) {}
> > > foo('static::method');
> > >
> > > [...] in PHP 9.0 the function will start throwing a TypeError.
> >
> > [...] This is a major problem because code which was "just working"
> directly goes to a TypeError without a migration phase warning about it.
> This is something I've repeatedly advocated against.
> >
> > > if (is_callable('static::method')) {
> > > static::method();
> > > }
> > >
> > > [...] 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.
> >
> > I agree with you here: Code which silently changes behavior is also a
> migration hassle.
>
> Hi,
>
> 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".
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.
So yeah, this cannot really be an usual deprecation, it needs to be
something else, IMO.
Regards,
Alex
Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On Wed, Mar 16, 2022 at 9:57 AM Christian Schneider
wrote:
>
> Am 16.03.2022 um 06:52 schrieb Juliette Reinders Folmer
> :
> > 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.
> >
> > [...] I wonder if the decision [...] is the right one (though I understand
> > the desire to keep [them] side-effect free).
> >
> > Consider these code samples:
> >
> > function foo(callable $callback) {}
> > foo('static::method');
> >
> > [...] in PHP 9.0 the function will start throwing a TypeError.
>
> [...] This is a major problem because code which was "just working" directly
> goes to a TypeError without a migration phase warning about it. This is
> something I've repeatedly advocated against.
>
> > if (is_callable('static::method')) {
> > static::method();
> > }
> >
> > [...] 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.
>
> I agree with you here: Code which silently changes behavior is also a
> migration hassle.
Hi,
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)...
Regards,
--
Guilliam Xavier
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
On 22-4-2022 3:02, php-internals_nos...@adviesenzo.nl wrote: On 21-4-2022 21:56, Andreas Hennings wrote: On Thu, 21 Apr 2022 at 11:18, Rowan Tommins wrote: On Wed, 20 Apr 2022 at 19:16, Claude Pache wrote: You make a very important claim in your bug report: However, in real world, is_callable() is almost never given totally arbitrary stuff My concern is that we have no way of knowing whether this is true, and whether there will always be a clear action for users who see the deprecation notice. In this case, there is always the possibility for them to use the error suppression operator. That remains easy, and looks like a reasonable use case of that operator. The error suppression operator is a big blunt tool, and using it to squash a deprecation notice would be an absolute last resort. Note that in contrast to the strpos() case where both behaviours can be achieved with *permanent* changes, the error suppression would only be a temporary workaround for error logging, with no value in itself. If we have to balance between addressing the verified issue of not emitting deprecation notice for well-known and fairly frequent code patterns, and addressing the potential and non-fatal issue of emitting too much notices for totally unknown use cases, the choice should be straightforward. Again, this is true IF you are correct that one set of patterns is common and the other is not, but so far I've not seen any attempt to find out whether that's true. The idea of ignoring "unknown" use cases seems really dangerous - like closing your eyes so you can't see the bad effects of what you do. What we need to know is whether such use cases are *rare*, or whether the people using them are just not subscribed to this list. One starting point would be to find usages in the top 100 or 1000 packages on Packagist usinghttps://github.com/nikic/popular-package-analysis - perhaps Juliette has already done that when testing their PHPCompatibility sniff? As a package author, you might not really know where the value is coming from. A parameter will be declared as `@param mixed $callback_or_not`, if it internally calls the`is_callable()` method. And a static analysis tool will only see `mixed`, not "mixed, but if it is a string, it does not start with 'static::', or if i's an array, the first value is not 'static'.". On the other hand, the value probably won't be completely arbitrary. It might be coming from a discovery process that reads yml files or annotations, or that calls some kind of hook in php. Or it is generated with string concatenation. Whatever it is, it is probably explicitly or implicitly hard-coded somewhere. Here is a starting point for a search. But only for explicit calls. https://sourcegraph.com/search?q=context:global+is_callable%5C%28%28%5C%5B%27%28static%7Cself%29%27%7C%27%28static%7Cself%29::.*%27%29%2C+lang:php=regexp I appreciate it very much that Claude is trying to move the issue I raised forward. Some responses to the discussion which has ensued: 1. In my opinion, the deprecation should only be shown when one of the deprecated syntaxes is passed either to a function with a `callable` type declaration or to `is_callable()`. The deprecation notice should not be shown in any other circumstances, so an arbitrary invalid value being passed to `is_callable()` should still return `false` without deprecation notice. 2. While I do extensively test new PHPCompatibility sniffs, I have not run the WIP sniff against the top 1000 projects from Packagist, nor do I intend to or have the setup to do so. I agree it would be a good idea to run a package analysis, but to be fair, in all honesty that should have been done for the original RFC, which was completely missing an impact analysis. 3. As for the pattern being common or not - the fact that I found it so easily in multiple random projects which I elected to test the sniff against, makes me believe the pattern is not _uncommon_. I should also point out that all three of the projects which I highlighted as examples in my previous response are used extensively, the first two both have over 100 million downloads via Packagist, with the first being a dependency for PHPUnit. The third doesn't show anywhere near as many downloads, but is a dependency of WordPress, which doesn't use Composer/Packagist for distribution, but is - as we're all aware - widely used. In case anyone is looking for them - these were the examples I posted previously: > 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 Kind regards, Juliette P.S.: and yes, the issues in the first two examples
Re: [PHP-DEV] Deprecated partially supported callables: should is_callable() throw a deprecation notice ?
Am 16.03.2022 um 06:52 schrieb Juliette Reinders Folmer
:
> 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.
My reaction to your last sentence is actually quite the opposite: This is a
major problem because code which was "just working" directly goes to a
TypeError without a migration phase warning about it. This is something I've
repeatedly advocated against.
> 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.
I agree with you here: Code which silently changes behavior is also a migration
hassle.
- Chris
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
