Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:125759 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by qa.php.net (Postfix) with ESMTPS id D3B0E1A00BD for ; Sun, 6 Oct 2024 18:50:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1728240786; bh=i9+x8zy0jVHRbQZrGxo9l+dNxVD9Mjt2Gd6t94M1OP4=; h=From:Subject:Date:In-Reply-To:Cc:To:References:From; b=Cwh54kKyy3KpUWjxv9fBEQ6w+Pnwt88/OLXeRewbw+zOLSFkUsm7oX55SeBc+/Jz4 JNNlr1F1Dq1Y9ieqme58M03voOl9r8WMbtALOdrH3QwfNxWeZqMBnyUZsliNOEueQ9 V8yTqSYIvfb7kOOBv6TrH6KI8oKTfsEj4t88UD9nZno+7OctS9MCLHTT8BiQImNXlK CMG0ghJnBvsZnWe+2jbmnHhQP4ucVBEUFdmZAubRSY93mdCqMxIJhCxg6ysiNdE9ML /KudftOEDTaAEQSyBXcVjEpbGzLUpPAwmSQOyLAtZ2sfthrVr/zA/BFJEFRujf0q/O TOgm6T30Qro0g== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 177B6180078 for ; Sun, 6 Oct 2024 18:53:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DMARC_MISSING,HTML_MESSAGE,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from mail-yb1-f177.google.com (mail-yb1-f177.google.com [209.85.219.177]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Sun, 6 Oct 2024 18:53:04 +0000 (UTC) Received: by mail-yb1-f177.google.com with SMTP id 3f1490d57ef6-e260b266805so3355756276.0 for ; Sun, 06 Oct 2024 11:50:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=newclarity-net.20230601.gappssmtp.com; s=20230601; t=1728240648; x=1728845448; darn=lists.php.net; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:from:to:cc:subject:date:message-id:reply-to; bh=HjQe3FHiVQQ3MuY15e/HIt5wmQ82ZZDu/5kFG/4yc3A=; b=kFgrpxThupxF9Q5KlB1HcfOYfprJeQLH04h1XvNFarqwvYF1IN7wJO1Zgeev1XGMey KTtW9wx3De5t6Oxds6yOXINuIduspEJfrLQdDN7zc5BPSJD4hYKCLknDUW3J82MM4YHC ssqZHXLBHD1pnCok/QQ0Ct/dQEFsde08TODhovE3j73d2l+V/VJLJPxac33Eo8zdpB0/ pQ9AEiVyXyXCDVe76ZMcJqLYnEwkT7Il5GROi/S+OqvX/ULkowLTaveNI1cbec/KSN0w 6QYIszQPao+ZJLABEPwV1mewmZQzqD+RUZLFaQODKWUkQTs94FyjGBxZ/JeqP6nG8t64 vNhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728240648; x=1728845448; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=HjQe3FHiVQQ3MuY15e/HIt5wmQ82ZZDu/5kFG/4yc3A=; b=bnz+qQFxVLbYvVdc2hBTxXZ3wxxBpvqflT8bJIYcesKz3zNuEGF0VHi3OTCGCn5snf MgAat3YmxnOHvvPI5v4mj5l+9989ydIVOlywpC4a3UJEeJYGDTml/BtSH4Tt6G5cmmIR IUZDvbTJWFtjABC06k3zFLb5fYMI6b0M1aYsGQ0MD6A1pWl71snGkEMJn7ZfEC7goy/w X9AjLzCYYuZJORWKacMH1YVS1tzlAaJdvT2i6+WdOxsq7Wk2MnMXDPXbPkcCglrRdJW9 NkoIfsikb5Qz8JzFujGNTZubKSxxOV/M0KKkoBkIZU6gGsERr75CVi3IDrSm+qPLbwBw QVuw== X-Gm-Message-State: AOJu0YwwBoCJ5QC5m0BwifGLB5IucnZubbe0lfPjvRad6j4h2smSTTtC 0/bRwzMewFszdMc1fOfWicjfeKd7YZS+Afp7F+OKr3619MnctVJqYklA7y7rpDo= X-Google-Smtp-Source: AGHT+IEy+uBSgVCI+6XhHBB1IJ5wMf2lK++JqONterDCKMCXeGHvq82hVQxIqaOQNv+AuJb6aYXTsg== X-Received: by 2002:a05:6902:18c3:b0:e25:1b5:edbe with SMTP id 3f1490d57ef6-e2893918422mr7415081276.17.1728240648108; Sun, 06 Oct 2024 11:50:48 -0700 (PDT) Received: from smtpclient.apple (c-98-252-216-111.hsd1.ga.comcast.net. [98.252.216.111]) by smtp.gmail.com with ESMTPSA id 3f1490d57ef6-e28a5c51815sm698750276.32.2024.10.06.11.50.46 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 06 Oct 2024 11:50:47 -0700 (PDT) Message-ID: Content-Type: multipart/alternative; boundary="Apple-Mail=_A380E3E8-2EA0-4991-9295-057B2E5980CA" Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net x-ms-reactions: disallow Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.10\)) Subject: Re: [PHP-DEV] [RFC] [Discussion] Add get_declared_enums() function Date: Sun, 6 Oct 2024 14:50:46 -0400 In-Reply-To: <67023B59.6020301@adviesenzo.nl> Cc: internals@lists.php.net, nicolas.grekas+php@gmail.com, Ayesh Karunaratne To: Juliette Reinders Folmer References: <66FC7B9F.5070906@adviesenzo.nl> <67023B59.6020301@adviesenzo.nl> X-Mailer: Apple Mail (2.3696.120.41.1.10) From: mike@newclarity.net (Mike Schinkel) --Apple-Mail=_A380E3E8-2EA0-4991-9295-057B2E5980CA Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On Oct 6, 2024, at 3:25 AM, Juliette Reinders Folmer = wrote: >=20 > On 4-10-2024 13:44, Nicolas Grekas wrote: >=20 > Hi Nicolas, >> # Introduction of the new function: get_declared_enums() >>=20 >> 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". >=20 > The upside of a PHP native function would be one of performance. >=20 >> # Deprecation of using class_exists() on enum names >>=20 >> 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. >=20 > 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]. >=20 > 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). >=20 > The full break down of the scan results [2] are as follows: >=20 > 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 >=20 >=20 > 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 >=20 > Please note that not all of this code would need updating. >=20 >> 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 = =3D -1, $autoload =3D true), where $filter is a bitfield that'd allow = listing only classes, abstract classes, interfaces, enums, traits at = will? >>=20 >> # Change of the return value of get_declared_classes() >>=20 >> This proposal is problematic on two aspects: >> 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. >=20 > 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()`. >=20 > Also note that not all of this code would need updating, only that = code which is also targeting enums. >=20 >> The BC break is planned without any ahead-of-change deprecation = notice (except doc of course). =46rom 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()?) >=20 > 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). >=20 > 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. >=20 > 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). >=20 > 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. >=20 > On the downside, the alternative approach makes for a larger BC break = (if combined with deprecation and eventual removal of the old = functions). Given the argued downsides of the current proposal, would introducing = `get_declared_symbols()` + `symbols_exists()` really require deprecating = the other functions? It seems adding a `get_current_enums()` and `get_declared_symbols()` + = `symbols_exists()` functions would address all the same userland = developer needs as the proposed RFC (and more) with the only caveat = being `get_current_classes()` remains confusing.=20 To address that one remaining concern why not use add a highly visible = recommendation at the top of the `get_current_classes()` docs page = informing people to use `get_declared_symbols()` when they need only = classes. Is there a reason this approach would not be the best way forward = compared to the other options currently being discussed? -Mike P.S. And if it is deemed critically important to get rid of the = potential confusion regarding the existing `get_current_classes()` = behavior it could be "ultrasoft" deprecated, meaning removed in PHP 11, = or just left in forever with a warning (although for the life of me I = still do not understand why some people are against leaving a deprecated = function in forever, especially for things that are changed for clarity, = not because they cause harm by themselves.)= --Apple-Mail=_A380E3E8-2EA0-4991-9295-057B2E5980CA Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii
On = Oct 6, 2024, at 3:25 AM, Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl> wrote:

=20 =20
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()         &n= bsp;           &nbs= p;          131
2 out of 4: class_exists() + = trait_exists()          =             &n= bsp;           &nbs= p;   8
2 out of 4: class_exists() + = enum_exists()          &= nbsp;           &nb= sp;            = ;    1
2 out of 4: interface_exists() + = trait_exists()          =             &n= bsp;           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()          &= nbsp;       3
2 out of 4: class_exists() + = interface_exists()         &n= bsp;           &nbs= p;           32
2 out of 4: class_exists() + = enum_exists()          &= nbsp;           &nb= sp;            = ;    4
2 out of 4: class_exists() + = trait_exists()          =             &n= bsp;           &nbs= p;   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 =3D -1, $autoload =3D 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). =46rom 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).

Given the argued downsides of the current = proposal, would introducing `get_declared_symbols()` + = `symbols_exists()` really require deprecating the other = functions?

It seems adding a = `get_current_enums()` and `get_declared_symbols()` + `symbols_exists()` = functions would address all the same userland developer needs as the = proposed RFC (and more) with the only caveat being = `get_current_classes()` remains confusing. 

To address that one remaining concern why not use = add a highly visible  recommendation at the top of the = `get_current_classes()` docs page informing people to use = `get_declared_symbols()` when they need only classes.

Is there a reason this approach would not be = the best way forward compared to the other options currently being = discussed?


-Mike

P.S.  And if it is deemed = critically important to get rid of the potential confusion regarding the = existing `get_current_classes()` behavior it could be "ultrasoft" = deprecated, meaning removed in PHP 11, or just left in forever with a = warning (although for the life of me I still do not understand why some = people are against leaving a deprecated function in forever, especially = for things that are changed for clarity, not because they cause harm by = themselves.)
= --Apple-Mail=_A380E3E8-2EA0-4991-9295-057B2E5980CA--