On 19-6-2023 23:16, Máté Kocsis wrote:

The impact analysis on userland code seems to be missing for some of the
proposals, most notably for the proposals which I expect will have the
highest impact. I'd like to ask for an impact analysis to be added to
each of these:
* array_keys()
* ReflectionProperty::setValue()

These are intentionally left out due to different reasons:

* Even though the 1 parameter version of array_keys() is used a lot, its 2+
parameter signature is much less known.
Personally, I've never used it. Of course, this doesn't mean no one uses
it, but I don't think this deprecation will really
matter in practice, so I didn't care to write a script for this (the rest
of the deprecations were easy to analyze via regexes).

* ReflectionProperty::setValue() would be a lot of work to analyze since
it's very difficult to track whether the
setValue() method is called on a ReflectionProperty instance. Not to
mention the fact that the deprecation only affects
function invocations where either a single parameter is provided or static
properties where the first parameter is not null
or an object instance. Since the suggested alternative is basically
available since forever, I think it's OK not to do
an exhaustive analysis in this case.

For the `get*_class()` deprecation, I wonder if an additional impact
analysis is needed for packages which may not have removed usages of
`get_class( null )`, which would now be double-impacted (and not caught
by the current analysis).

I'm not sure I can follow you, but get_class(null) is a TypeError since 8.0
(and have been warning since 7.2). If a package
didn't feel the need to migrate its get_class() usages then it is likely a
dead project. And if someone tries to upgrade their
(proprietary) code from PHP 7.1 to PHP 8.3 directly then they will notice
the exception first and also - if they followed the upgrading
notes carelly - will learn the relevant suggestions regarding the
deprecation. But realistically speaking, these projects will most probably
be bound by lots of other more severe issues, so I don't think the above is
a viable upgrade path...


Thanks for the reply Máté.

I understand what you are saying and appreciate it may be difficult to get an accurate impact analysis for `ReflectionProperty::setValue()`.

Respectfully though, in my opinion selectively leaving out impact analysis without mentioning why they are missing in the RFC, reeks of trying to hide information which could influence the vote. Maybe just mentioning why the impact analysis is missing in these cases in the RFC could take that stench away ?


Reply via email to