Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:125756
Subject: Re: [PHP-DEV] [RFC] [Discussion] Add get_declared_enums() function
To: internals@lists.php.net
References: <66FC7B9F.5070906@adviesenzo.nl>
 <CAOWwgp=--t8xMEE6Dso9whYKydjHPiXz69PTp=AYOU8en3TjcQ@mail.gmail.com>
Cc: nicolas.grekas+php@gmail.com, Ayesh Karunaratne <ayesh@php.watch>
Message-ID: <67023B59.6020301@adviesenzo.nl>
Date: Sun, 6 Oct 2024 09:25:13 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <CAOWwgp=--t8xMEE6Dso9whYKydjHPiXz69PTp=AYOU8en3TjcQ@mail.gmail.com>
Content-Type: multipart/alternative;
 boundary="------------040801060301090605020109"
From: php-internals_nospam@adviesenzo.nl (Juliette Reinders Folmer)

This is a multi-part message in MIME format.
--------------040801060301090605020109
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

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


--------------040801060301090605020109
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">On 4-10-2024 13:44, Nicolas Grekas
      wrote:<br>
    </div>
    <br>
    Hi Nicolas,<br>
    <blockquote
cite="mid:CAOWwgp=--t8xMEE6Dso9whYKydjHPiXz69PTp=AYOU8en3TjcQ@mail.gmail.com"
      type="cite">
      <div dir="ltr"># Introduction of the new function:
        get_declared_enums()
        <div class="gmail_quote">
          <div><br>
          </div>
          <div>About this proposal, I shared a one-liner in the previous
            thread that shows listing only enums is trivial already.</div>
          <div>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".<br>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
    The upside of a PHP native function would be one of performance.<br>
    <br>
    <blockquote
cite="mid:CAOWwgp=--t8xMEE6Dso9whYKydjHPiXz69PTp=AYOU8en3TjcQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_quote">
          <div># Deprecation of using class_exists() on enum names</div>
          <div><br>
          </div>
          <div>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.</div>
        </div>
      </div>
    </blockquote>
    <br>
    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].<br>
    <br>
    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).<br>
    <br>
    The full break down of the scan results [2] are as follows:<br>
    <br>
    Combination of the following function calls within a set of
    parentheses (typically: control structure condition):<br>
    <tt>4 out of 4: class_exists() + interface_exists() + trait_exists()
      + enum_exists() 3<br>
      3 out of 4: class_exists() + interface_exists() +
      trait_exists()                36<br>
      2 out of 4: class_exists() +
      interface_exists()                                131<br>
      2 out of 4: class_exists() +
      trait_exists()                                      8<br>
      2 out of 4: class_exists() +
      enum_exists()                                       1<br>
      2 out of 4: interface_exists() +
      trait_exists()                                  1<br>
    </tt><tt><br>
    </tt><br>
    Combination of the following function calls within the same function
    scope (not necessarily in the same statement, excluding the above):<br>
    <tt><tt>4 out of 4: class_exists() + interface_exists() +
        trait_exists() + enum_exists() 2<br>
        3 out of 4: class_exists() + interface_exists() +
        trait_exists()                17<br>
        3 out of 4: class_exists() + interface_exists() +
        enum_exists()                  3<br>
        2 out of 4: class_exists() +
        interface_exists()                                 32<br>
        2 out of 4: class_exists() +
        enum_exists()                                       4<br>
        2 out of 4: class_exists() +
        trait_exists()                                      2</tt></tt><tt></tt><br>
    <br>
    Please note that not all of this code would need updating.<br>
    <br>
    <blockquote
cite="mid:CAOWwgp=--t8xMEE6Dso9whYKydjHPiXz69PTp=AYOU8en3TjcQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_quote">
          <div>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?</div>
          <div><br>
          </div>
          <div># Change of the return value of get_declared_classes()</div>
          <div><br>
          </div>
          <div>This proposal is problematic on two aspects:</div>
          <div>
            <ol>
              <li>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.</li>
            </ol>
          </div>
        </div>
      </div>
    </blockquote>
    Again, let's look at the real world impact based on a scan of the
    Packagist top 2000.<br>
    <br>
    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]<br>
    And for the record, there are only 16 calls each to
    `get_declared_interfaces()` and `get_declared_traits()`.<br>
    <br>
    Also note that not all of this code would need updating, only that
    code which is also targeting enums.<br>
    <br>
    <blockquote
cite="mid:CAOWwgp=--t8xMEE6Dso9whYKydjHPiXz69PTp=AYOU8en3TjcQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_quote">
          <div>
            <ol>
              <li>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()?)</li>
            </ol>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
    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).<br>
    <br>
    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.<br>
    <br>
    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).<br>
    <br>
    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.<br>
    <br>
    On the downside, the alternative approach makes for a larger BC
    break (if combined with deprecation and eventual removal of the old
    functions).<br>
    <br>
    Would you consider that approach preferable ?<br>
    <br>
    <blockquote
cite="mid:CAOWwgp=--t8xMEE6Dso9whYKydjHPiXz69PTp=AYOU8en3TjcQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_quote">
          <div>
            <div>You've been a rightfully vocal criticizer of BC breaks
              / deprecations in PHP. Please help the vast part of the
              community that favors stability 🙏</div>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
    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 ?<br>
    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. <br>
    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).<br>
    <br>
    Smile,<br>
    Juliette<br>
    <br>
    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.<br>
    2: Detailed results of the scan for combinations of the *_exists()
    functions:
    <a class="moz-txt-link-freetext" href="https://gist.github.com/jrfnl/5ac6cf10e4553a5e2174cf0f58d5e305">https://gist.github.com/jrfnl/5ac6cf10e4553a5e2174cf0f58d5e305</a><br>
    3: Detailed results of the scan for usage of the get_declared_*()
    functions:
    <a class="moz-txt-link-freetext" href="https://gist.github.com/jrfnl/ccb11e73d804cc5bb7fe25c6899c43d1">https://gist.github.com/jrfnl/ccb11e73d804cc5bb7fe25c6899c43d1</a><br>
    4:
<a class="moz-txt-link-freetext" href="https://wiki.php.net/rfc/get_declared_enums#were_any_alternative_approaches_considered">https://wiki.php.net/rfc/get_declared_enums#were_any_alternative_approaches_considered</a><br>
    <br>
  </body>
</html>

--------------040801060301090605020109--