On 28-4-2022 17:00, Christoph M. Becker wrote:
On 28.04.2022 at 16:09, Juliette Reinders Folmer wrote:
On 22-4-2022 8:47, Rowan Tommins wrote:
On 22 April 2022 02:02:58 BST,php-internals_nos...@adviesenzo.nl wrote:
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.
That's a fair point, but of course "somebody else should have done it
already" isn't a good excuse not to do it now.
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_.
Since there has been some misunderstanding, the usage I think we need
to look for is where a deprecation notice would *not* be useful. In
other words, are there people using is_callable in such a way that
even if a value like "parent::foo" changes from returning "true" to
"false", there won't be anything that needs changing, because the code
is equally "happy" with both return values?
I freely admit that I can't think of any such usage off the top of my
head, but I wouldn't have thought of some of the examples already
raised either.
If an actual search fails to find such usages, or provides evidence
that it is very rare, then I am absolutely in favour of adding a
deprecation notice.
Ah! So you're basically asking for the impossible. Search for a code
pattern where a deprecation would not be useful, while noone has been
able to come up with one.
In that case: search done. Nothing found. Let's get this fixed.
Isn't the following a *very* typical use case?
if (is_callable($callback)) {
call_user_func($callback);
}
If $callback is something like "parent::foo", you get the deprecation
warning when you're actually calling the $callback. An additional
deprecation warning for the is_callable() call would not be really
helpful in this case.
I do not agree with that assessment. With the huge amount of deprecation
notices over the last few versions, it is pretty typical for people to
focus *only* on the code flagged by the deprecation notice and not look
at the wider context of the code.
So, like already happened in Symfony, the calls involving
`call_user_func()` were fixed, but the deprecated usages in calls to
`is_callable()` were not.
That said, I see the point in hinting at other usages (such as shown in
the OP), but I'm not really happy about a deprecation. What about
providing a patch which warns about such usages without planning to
integrate that patch into php-src? Similar like it was done with
<https://github.com/php/php-src/pull/3917>.
Eh... sorry, but I'm not sure what you mean. I can't imagine you mean
creating a patch which is never intended to get merged ? That doesn't
sound very useful.
Smile,
Juliette
