On 4-10-2024 13:44, Nicolas Grekas wrote:
Hi Nicolas,
# Introduction of the new function: get_declared_enums()
About this proposal, I shared a one-liner in the previous thread that
shows listing only enums is trivial already.
IMHO we don't need this function since the engine already provides
everything one needs if they want to list enums. I won't object
either, I'm just "-0".
The upside of a PHP native function would be one of performance.
# Deprecation of using class_exists() on enum names
This is a big NO on my side. This will break perfectly fine code for
the sake of some high level ideas that matter less is practice than
ensuring stability of the PHP platform. A BC break has to be worth it
and this is clearly not the case to me. The canonical examples are
checks like `class_exists($c) || interface_exists($c, false) ||
trait_exists($c, false)`. This is common code to check if a symbol
exists in current PHP. Yet, with your RFC, all such checks would
become broken immediately.
Well, this statement made me curious to see just _how_ common this type
of code is, so I've done a scan of the Packagist top 2000 projects [1].
The check you mention is used a total of 36 times in ~20 packages out of
2000 (includes some duplication, mostly within Symfony, which also
contains the fast majority of the uses of the above mentioned combination).
The full break down of the scan results [2] are as follows:
Combination of the following function calls within a set of parentheses
(typically: control structure condition):
4 out of 4: class_exists() + interface_exists() + trait_exists() +
enum_exists() 3
3 out of 4: class_exists() + interface_exists() +
trait_exists() 36
2 out of 4: class_exists() +
interface_exists() 131
2 out of 4: class_exists() +
trait_exists() 8
2 out of 4: class_exists() +
enum_exists() 1
2 out of 4: interface_exists() +
trait_exists() 1
Combination of the following function calls within the same function
scope (not necessarily in the same statement, excluding the above):
4 out of 4: class_exists() + interface_exists() + trait_exists() +
enum_exists() 2
3 out of 4: class_exists() + interface_exists() +
trait_exists() 17
3 out of 4: class_exists() + interface_exists() +
enum_exists() 3
2 out of 4: class_exists() +
interface_exists() 32
2 out of 4: class_exists() +
enum_exists() 4
2 out of 4: class_exists() +
trait_exists() 2
Please note that not all of this code would need updating.
BTW, this makes me wonder if we could have a new symbol_exists()
function, that'd basically do the above checks in one go? Mixing this
idea with Claude's, the signature could be symbol_exists($class,
$filter = -1, $autoload = true), where $filter is a bitfield that'd
allow listing only classes, abstract classes, interfaces, enums,
traits at will?
# Change of the return value of get_declared_classes()
This proposal is problematic on two aspects:
1. The planned BC break feels needless to me. Its motivation is very
moot compared to its impact on the PHP community, which will be
forced to update perfectly fine code.
Again, let's look at the real world impact based on a scan of the
Packagist top 2000.
In the top 2000 packages, there are only 47 calls to the
`get_declared_classes()` function. These calls are found in 29 packages.
[1][3]
And for the record, there are only 16 calls each to
`get_declared_interfaces()` and `get_declared_traits()`.
Also note that not all of this code would need updating, only that code
which is also targeting enums.
1. The BC break is planned without any ahead-of-change deprecation
notice (except doc of course). From a deprecation policy POV, we
reject this practice in the Symfony community. We don't do "hard
BC breaks", or "unannounced" ones: we mandate that any BC break is
first announced in the current major. This ensures the community
won't miss the notice, and won't discover the BC break when it's
very late and thus costly. There is always a way to follow that
policy, so I'd strongly recommend adopting this practice in PHP
itself, and in this RFC today. Here, this could be done by either
keeping the function as is in PHP 9, or just deprecating it (in
favor of get_declared_symbols()?)
I hear what you are saying and largely agree with you. The problem is,
of course, that it seems unlikely that we can find a way to throw a
deprecation for this, as PHP would then also throw a deprecation for
code which needs no changes (which only _intends_ to look at classes,
not enums).
In the RFC, we mention an alternative approach [4], building upon the
suggestion by Claude. This alternative approach would allow for
deprecations to be thrown, but would, in my estimation, need a longer
lead-time. Something like: introduce the new function in PHP 8.5,
deprecate use of the old functions in PHP 9.last and remove in PHP 10.0.
I can imagine combining the alternative approach via
get_declared_symbols() with a new symbol_exists() function like you
suggest above (with a similar slow path to deprecate and remove the old
functions).
On the plus side, the alternative approach makes for much more versatile
functionality. In a number of the cases I looked at, the results from
various get_declared_*() functions are combined before further
processing, so having a `get-declared_symbols()` function would allow
for simplifying that code. The same can be said for the *_exists()
functions.
On the downside, the alternative approach makes for a larger BC break
(if combined with deprecation and eventual removal of the old functions).
Would you consider that approach preferable ?
You've been a rightfully vocal criticizer of BC breaks / deprecations
in PHP. Please help the vast part of the community that favors
stability 🙏
I evaluate each RFC on their merit and weight the impact and the effort
which would be needed to handle the deprecation (and always have). For
short: do the benefits outweigh the costs ?
There are plenty of deprecations I've stayed silent about as those (IMO)
weren't particularly problematic or even if they were, the arguments to
deprecate were particularly strong.
Just saying this, as I want to make sure people aren't confusing
"speaking up when I see a problematic change" (which I do) with "being
against deprecations" (which I'm not).
Smile,
Juliette
1: Based on the latest published release of each package and providing
the package contained source code and not just a PHAR file. Excluding
vendor and test directories. May contain some duplicates. Total number
of files scanned: 233,968.
2: Detailed results of the scan for combinations of the *_exists()
functions: https://gist.github.com/jrfnl/5ac6cf10e4553a5e2174cf0f58d5e305
3: Detailed results of the scan for usage of the get_declared_*()
functions: https://gist.github.com/jrfnl/ccb11e73d804cc5bb7fe25c6899c43d1
4:
https://wiki.php.net/rfc/get_declared_enums#were_any_alternative_approaches_considered
