Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123838
Message-ID: <df64d0e7-1f3b-47ad-b98b-4e052d5d9bda@bastelstu.be>
Date: Tue, 25 Jun 2024 20:39:20 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PHP-DEV] Re: [RFC] [Vote] #[\Deprecated] attribute
To: Nicolas Grekas <nicolas.grekas+php@gmail.com>,
 =?UTF-8?Q?Benjamin_Au=C3=9Fenhofer?= <kontakt@beberlei.de>
Cc: PHP Internals <internals@lists.php.net>
References: <CAEPJdu=ZTyPQ3yabhOx=rxnv5niuA+9O4BP6EsvAdkZRSS5Q2g@mail.gmail.com>
 <CAEPJdu=Qum4-HygCbs8xgpiNhm+gdeP6rG6Ua8jApz7pZTKfvA@mail.gmail.com>
 <CAOWwgp=TXMJC2+mgnCGJH0jXLo8WiY2uMb8i5LVZJSdoodVH3w@mail.gmail.com>
Content-Language: en-US
In-Reply-To: <CAOWwgp=TXMJC2+mgnCGJH0jXLo8WiY2uMb8i5LVZJSdoodVH3w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=C3=BCsterhus?=)

Hi

On 6/24/24 17:06, Nicolas Grekas wrote:
> 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.

I didn't consult with Benjamin and thus I'm not speaking for him, but 
for me this definitely requires an additional RFC:

A main point of our RFC was to make the *existing* deprecation 
functionality available to userland. There is no existing deprecation 
functionality for classes (e.g. no ReflectionClass::isDeprecated()) and 
thus supporting the attribute on classes was out of scope / left for 
future scope.

Personally I would against such an RFC, because it adds to the 
inconsistency of the language by not emitting a deprecation warning.

> 2/ About  the "since" parameter, we're going to standardize around a
> package+version string, e.g.
> #[Deprecated(since: "symfony/yaml v7.2"]
> 
> I'm sharing this hoping this can spread outside of Symfony's boundary
> because I think this would be a very useful practice that'd allow building
> nice reporting tools.

You might be interested in this thread on the implementation PR then: 
https://github.com/php/php-src/pull/11293#discussion_r1647339607

Best regards
Tim Düsterhus