Le mar. 25 juin 2024 à 22:01, Benjamin Außenhofer <kont...@beberlei.de> a écrit :
> > > On Mon, Jun 24, 2024 at 5:07 PM Nicolas Grekas < > nicolas.grekas+...@gmail.com> wrote: > >> >> >> Le mer. 5 juin 2024 à 10:18, Benjamin Außenhofer <kont...@beberlei.de> a >> écrit : >> >>> >>> >>> On Wed, May 22, 2024 at 9:22 AM Benjamin Außenhofer <kont...@beberlei.de> >>> wrote: >>> >>>> The vote for the RFC #[\Deprecated] attribute is now open: >>>> >>>> https://wiki.php.net/rfc/deprecated_attribute >>>> >>>> Voting will close on Wednesday 5th June, 08:00 GMT. >>>> >>> >>> The #[\Deprecated] attribute has been accepted with 23 (Yes) to 6 (No) >>> votes, 79%. >>> >>> >>> The secondary vote to include Deprecated::$since has also been accepted >>> with 22 (Yes) to 1 (No) votes, 96%. >>> >>> >>> Thank you to everyone for voting! >>> >>> >>> Tim will finalize the implementation PR now and work on its merge in the >>> upcoming days. >>> >> >> Hi Benjamin, >> >> The vote for the RFC #[\Deprecated] attribute is now open: >>>> >>>> https://wiki.php.net/rfc/deprecated_attribute >>>> >>>> Voting will close on Wednesday 5th June, 08:00 GMT. >>>> >>> >>> The #[\Deprecated] attribute has been accepted with 23 (Yes) to 6 (No) >>> votes, 79%. >>> >>> >>> The secondary vote to include Deprecated::$since has also been accepted >>> with 22 (Yes) to 1 (No) votes, 96%. >>> >>> >>> Thank you to everyone for voting! >>> >>> >>> Tim will finalize the implementation PR now and work on its merge in the >>> upcoming days. >>> >> >> Since the vote passed, we're discussing how we might use the attribute in >> Symfony. >> 2 things on the topic: >> >> 1/ We're wondering about using it at the class level despite the missing >> Attribute::TARGET_CLASS. ReflectionAttribute does allow reading >> attributes without checking these flags and we might leverage this >> capability to make our DebugClassLoader able to inspect those at the class >> level. >> >> Would you consider adding Attribute::TARGET_CLASS in 8.4 already? It >> would just make sense to me. We don't need the engine to do anything about >> deprecated classes ever since all we need in userland is a class-loader >> that checks for the attribute. Keeping the engine simpler and empowering >> userland with maximum flexiblity FTW. I'm up for a quick RFC if the >> consensus is this needs one. >> > > We have started loosly thinking about the behavior of > Attribute::TARGET_CLASS for a next PHP RFC already, and allowing this > before the behavior is introduced seems like a bad precedent to me, so I > agree with Tim that we probably shouldn't do that. > That's sad news, because I keep explaining why engine-triggered runtime notices are a terrible idea, yet you're planning to add more of them. The consistency argument Tim wrote in another email isn't sound to me: consistency with a bad idea doesn't make a good idea. In the hope it may be used as food for thoughts: when we use `@deprecated` on a class, Symfony's DebugClassLoader throws a deprecation ONLY when a class *extends* one of those `@deprecated` classes. For the runtime notice itself, we decide case by case among two possibilities: either triggering the notice just before the class declaration, or in the constructor. The reason is that sometimes we have to load the class but we don't want to trigger the deprecation because loading the class doesn't trigger any side-effects that users should be warned about in relation to the deprecation. In such situations, we trigger in the constructor. About Attribute::TARGET_CLASS itself, not adding it right now will lead to a situation where userland will have a hard time writing code that's compatible with both 8.4 and 8.5 (assuming 8.5 is the moment where the flag is added): using #[Deprecated] will be "illegal" when running on 8.4, yet legal when running in 8.5? That's another reason to allow the flag right away: smooth upgrades. My suggestion is quite restricted, and that'd solve all my concerns: it is to enable the flag right now, and to add ReflectionClass::isDeprecated() right now also if you want. But don't plan anything more on the topic, except maybe a deprecation notice when *extending* a #[Deprecated] class. But nothing more please. No runtime notice when loading the class. Nicolas
