# [RFC] [Vote] #[\Deprecated] attribute

- Thread: https://externals.io/message/123397
- Original archive: https://news-web.php.net/php.internals/123397
- Started by kontakt@beberlei.de on 2024-05-22T07:22:55+00:00
- Messages: 21
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #123397 — kontakt@beberlei.de — 2024-05-22T07:22:55+00:00

URL: https://externals.io/message/123397#123397

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.

---

## #123398 — Nicolas Grekas — 2024-05-22T07:33:21+00:00

URL: https://externals.io/message/123397#123398 · In reply to: #123397

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.
>

I voted "no" because I think this is better addressed in userland, as this
gives more flexibility.
I would better have an attribute that is made only for static analysis with
no runtime side effects at all.
Being forced to make the attribute final because the implementation in the
engine requires is an example of why the engine is not the correct place to
send this notice. Another example is not being able to add the attribute on
classes because [engine reasons].

 `trigger_error()` is better fitted for the runtime side-effect when it's
desired.
In my opinion, #[Deprecated] should be only for static analysers /
reflection (although this would need another discussion - I'm not sure this
would benefit being in the engine vs in a userland package.

Thanks for the RFC anyway.

Nicolas

---

## #123399 — Marco Pivetta — 2024-05-22T07:39:15+00:00

URL: https://externals.io/message/123397#123399 · In reply to: #123397

Hey Benjamin,

I voted against it:

* I very much like having reflection methods that work on internal symbols
relying on `#[\Deprecated]`. I can rely on these in my static analysis
tooling, test suites, decorators, dependency injection tools, etc.
* I am still disagree with abusing global runtime effects (`E_DEPRECATED` /
`E_USER_DEPRECATED`) to signal deprecations. Insert "PTSD Chihuaua" meme
here regarding PHP and library upgrades I had to work on over the last
decade. No: don't want. Don't add side-effects to otherwise functionally
pure code.

Marco Pivetta

https://mastodon.social/@ocramius

https://ocramius.github.io/


On Wed, 22 May 2024 at 09:23, Benjamin Außenhofer <kontakt@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.

---

## #123400 — Pierre — 2024-05-22T08:00:26+00:00

URL: https://externals.io/message/123397#123400 · In reply to: #123398

Le 22/05/2024 à 09:33, Nicolas Grekas a écrit :
> 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.
>
>
> I voted "no" because I think this is better addressed in userland, as 
> this gives more flexibility.
> I would better have an attribute that is made only for static analysis 
> with no runtime side effects at all.
> Being forced to make the attribute final because the implementation in 
> the engine requires is an example of why the engine is not the correct 
> place to send this notice. Another example is not being able to add 
> the attribute on classes because [engine reasons].
>
> `trigger_error()` is better fitted for the runtime side-effect when it's 
> desired.
> In my opinion, #[Deprecated] should be only for static analysers / 
> reflection (although this would need another discussion - I'm not sure 
> this would benefit being in the engine vs in a userland package.
>
> Thanks for the RFC anyway.
>
> Nicolas

I'm always dubious when I read the "this can be addressed or better 
addressed in userland": as soon as you need something as little as the 
Deprecated attribute, if it's not in core but in userland, you encounter 
two very serious issues: first one, fragmentation, every framework, 
organisation, or static analysis tool will have its own variant, second 
one if the community converges toward a single one, you then will be 
dependent on a certain framework or huge tooling.

I can see many benefits in having this in core, one being that every 
static analyzer will implement it using the core attribute, so any 
project and developer using it can choose any static analysis tooling 
without having to change or adapt its code everywhere, or learning a 
different convention in order to use different tools.

That's my 2 cents, but the "better in userland" argument seems to me 
almost always biased or wrong. Especially for this Deprecated argument, 
this a very common need, and each project Doctrine, Symfony, any tool 
implement it in a different manner. I don't want to be rude, but as a 
developer using all of those, it's very annoying having to learn 
different ways of doing something as simple and naive that saying that a 
function is deprecated. One manner is sufficient, even if not perfect.

--

Pierre

---

## #123401 — kontakt@beberlei.de — 2024-05-22T08:13:20+00:00

URL: https://externals.io/message/123397#123401 · In reply to: #123398

On Wed, May 22, 2024 at 9:33 AM Nicolas Grekas <nicolas.grekas+php@gmail.com>
wrote:

> Hi Benjamin,
>
> The vote for the RFC #[\Deprecated] attribute is now open:
> > […]
>
> I voted "no" because I think this is better addressed in userland, as this
> gives more flexibility.
> I would better have an attribute that is made only for static analysis
> with no runtime side effects at all.
> Being forced to make the attribute final because the implementation in the
> engine requires is an example of why the engine is not the correct place to
> send this notice. Another example is not being able to add the attribute on
> classes because [engine reasons].
>

This is a misrepresentation of what this RFC is. There are no engine
reasons for not being able to set it on classes. It just has not been
implemented yet.

Allowing deprecations on classes is something that needs to be figured out
in a separate RFC in how these semantics work. Then adding Deprecated
attribute would just be a trivial matter.

You could argue about the order of things, we could have first made an RFC
for deprecated classes, but we should avoid a waterfall process of
dependencies in things to do in the language.


>
> `trigger_error()` is better fitted for the runtime side-effect when it's
> desired.
> In my opinion, #[Deprecated] should be only for static analysers /
> reflection (although this would need another discussion - I'm not sure this
> would benefit being in the engine vs in a userland package.
>

This is the same as above, changing how deprecations work in the engine is
orthogonal to this attribute. These attributes are concerned with tagging
code elements with flags that the engine already provides, already makes
available for internal code elements.


>
> Thanks for the RFC anyway.
>
> Nicolas

---

## #123402 — Arvids Godjuks — 2024-05-22T08:23:19+00:00

URL: https://externals.io/message/123397#123402 · In reply to: #123400

> Le 22/05/2024 à 09:33, Nicolas Grekas a écrit :
>
> Hi Benjamin,
>
> The vote for the RFC #[\Deprecated] attribute is now open:
> > […]
>
> I voted "no" because I think this is better addressed in userland, as this
> gives more flexibility.
> I would better have an attribute that is made only for static analysis
> with no runtime side effects at all.
> Being forced to make the attribute final because the implementation in the
> engine requires is an example of why the engine is not the correct place to
> send this notice. Another example is not being able to add the attribute on
> classes because [engine reasons].
>
> `trigger_error()` is better fitted for the runtime side-effect when it's
> desired.
> In my opinion, #[Deprecated] should be only for static analysers /
> reflection (although this would need another discussion - I'm not sure this
> would benefit being in the engine vs in a userland package.
>
> Thanks for the RFC anyway.
>
> Nicolas
>
> I'm always dubious when I read the "this can be addressed or better
> addressed in userland": as soon as you need something as little as the
> Deprecated attribute, if it's not in core but in userland, you encounter
> two very serious issues: first one, fragmentation, every framework,
> organisation, or static analysis tool will have its own variant, second one
> if the community converges toward a single one, you then will be dependent
> on a certain framework or huge tooling.
>
> I can see many benefits in having this in core, one being that every
> static analyzer will implement it using the core attribute, so any project
> and developer using it can choose any static analysis tooling without
> having to change or adapt its code everywhere, or learning a different
> convention in order to use different tools.
>
> That's my 2 cents, but the "better in userland" argument seems to me
> almost always biased or wrong. Especially for this Deprecated argument,
> this a very common need, and each project Doctrine, Symfony, any tool
> implement it in a different manner. I don't want to be rude, but as a
> developer using all of those, it's very annoying having to learn different
> ways of doing something as simple and naive that saying that a function is
> deprecated. One manner is sufficient, even if not perfect.
>
> --
>
> Pierre
>
Hello everyone.

I have to agree with Pierre on this one as a userland developer.
To give a good illustration: Guzzle. I'm trying to get rid of it in my code
base and use SF HttpClient (mostly for the sole reason that guzzle
truncates replies in their exceptions without any ability to configure that
and cutting off all actually useful information about why request failed)
and... I can't... Because some other package is hardwired to Guzzle and
refuses any attempts to inject a different HttpClient. Which forces me to
roll my own implementations, thankfully making API clients is fairly simple.
A Deprecated attribute belongs in the core.

-- 

Arvīds Godjuks
+371 26 851 664
arvids.godjuks@gmail.com
Telegram: @psihius https://t.me/psihius

---

## #123403 — Gina P. Banyard — 2024-05-22T11:53:08+00:00

URL: https://externals.io/message/123397#123403 · In reply to: #123397

> 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.

I have voted in favour of the RFC, I don't have any strong opinions about the "since" parameter, so I will let other people judge on this matter.
However, any discussion about how deprecations are currently being triggered is, IMHO, irrelevant to this discussion.
PHP, at this point in time, triggers a deprecation diagnostic via the same mechanism that notices, warnings, etc. diagnostics are triggered.
There might be value to separate the deprecation diagnostic mechanism from the other diagnostics to have them be globally toggled on/off and sent logs into a different file than the other diagnostics,
but like said by others, this is an orthogonal issue to this one, and one that has long existed in the language.

Every other internal attribute that has been added to PHP has some visible effects to the user, having it do nothing seems rather odd to me.

Best regards,
Gina P. Banyard

---

## #123489 — Matthew Weier O'Phinney — 2024-06-03T13:15:10+00:00

URL: https://externals.io/message/123397#123489 · In reply to: #123397

On Wed, May 22, 2024 at 2:24 AM Benjamin Außenhofer <kontakt@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.
>


I have voted no for a few reasons:

- Ideally, I'd like to be able to mark _anything_ as deprecated. In
particular, not being able to mark a _class/interface/enum/etc_ as
deprecated makes this far less useful.

- The "since" parameter is basically worthless to me. It's very easy to
find out the last version that wasn't deprecated. What would be far more
useful to a consumer is an argument indicating when something will be
removed (e.g. $toRemoveInVersion, $versionForRemoval, etc.). This helps me
as a user plan for the future.

-- 
Matthew Weier O'Phinney
mweierophinney@gmail.com
https://mwop.net/
he/him

---

## #123490 — Ilija Tovilo — 2024-06-03T14:44:40+00:00

URL: https://externals.io/message/123397#123490 · In reply to: #123489

Hi Matthew

On Mon, Jun 3, 2024 at 3:15 PM Matthew Weier O'Phinney
<mweierophinney@gmail.com> wrote:
>
> 
> > […]
>
> I have voted no for a few reasons:
>
> - Ideally, I'd like to be able to mark _anything_ as deprecated. In particular, not being able to mark a _class/interface/enum/etc_ as deprecated makes this far less useful.

While it's true that extending #[Deprecated] to classes would be
useful, deprecation already exists as a language concept, and it can
be extended to class-like structures without a BC break.

> - The "since" parameter is basically worthless to me. It's very easy to find out the last version that wasn't deprecated. What would be far more useful to a consumer is an argument indicating when something will be removed (e.g. $toRemoveInVersion, $versionForRemoval, etc.). This helps me as a user plan for the future.

Did you vote yes in the secondary vote by accident? I voted no on the
$since parameter for the same reason:

* "How long have I not fixed this?" is not a particularly useful
question to ask. "When do I have to fix this?" is more relevant.
* The format of $since is intentionally left unstandardized, and it's
unclear (to me?) what it refers to. For example, some packages are
split into multiple, smaller ones (e.g. Doctrine) with diverging
version numbers. The sub-package version number may not be useful to
the end-user, who never requires it directly. Similarly, referencing
the main package version may be confusing, especially if the ranges of
recent main and sub-package versions overlap.

Ilija

> - The "since" parameter is basically worthless to me. It's very easy to find out the last version that wasn't deprecated. What would be far more useful to a consumer is an argument indicating when something will be removed (e.g. $toRemoveInVersion, $versionForRemoval, etc.). This helps me as a user plan for the future.
>
> --
> Matthew Weier O'Phinney
> mweierophinney@gmail.com
> https://mwop.net/
> he/him

---

## #123491 — Matthew Weier O'Phinney — 2024-06-03T16:03:28+00:00

URL: https://externals.io/message/123397#123491 · In reply to: #123490

> Hi Matthew
>
> On Mon, Jun 3, 2024 at 3:15 PM Matthew Weier O'Phinney
> <mweierophinney@gmail.com> wrote:
> > […]
> wrote:
> > […]
> particular, not being able to mark a _class/interface/enum/etc_ as
> deprecated makes this far less useful.
>
> While it's true that extending #[Deprecated] to classes would be
> useful, deprecation already exists as a language concept, and it can
> be extended to class-like structures without a BC break.
>

Right, but I'd rather have the ability to have it _now_. It's far less
often the case that I'm deprecating a single method or constant, and more
often the case that I'm deprecating a class or interface. Not having
support for those immediately makes the feature  "meh" for me.

>
> > […]
> find out the last version that wasn't deprecated. What would be far more
> useful to a consumer is an argument indicating when something will be
> removed (e.g. $toRemoveInVersion, $versionForRemoval, etc.). This helps me
> as a user plan for the future.
>
> Did you vote yes in the secondary vote by accident? I voted no on the
> $since parameter for the same reason:
>
> * "How long have I not fixed this?" is not a particularly useful
> question to ask. "When do I have to fix this?" is more relevant.
> * The format of $since is intentionally left unstandardized, and it's
> unclear (to me?) what it refers to. For example, some packages are
> split into multiple, smaller ones (e.g. Doctrine) with diverging
> version numbers. The sub-package version number may not be useful to
> the end-user, who never requires it directly. Similarly, referencing
> the main package version may be confusing, especially if the ranges of
> recent main and sub-package versions overlap.
>

I voted yes on it because it's not _worthless_, and if the RFC passes, I'd
rather have it than not have it. But I agree that the lack of a standard
format for it, and the lack of a parallel "will remove by" argument make it
far less useful.

>
>

-- 
Matthew Weier O'Phinney
mweierophinney@gmail.com
https://mwop.net/
he/him

---

## #123513 — kontakt@beberlei.de — 2024-06-05T08:16:06+00:00

URL: https://externals.io/message/123397#123513 · In reply to: #123397

On Wed, May 22, 2024 at 9:22 AM Benjamin Außenhofer <kontakt@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.

---

## #123568 — tim@bastelstu.be — 2024-06-10T17:22:12+00:00

URL: https://externals.io/message/123397#123568 · In reply to: #123513

Hi


> Tim will finalize the implementation PR now and work on its merge in the
> upcoming days.
> 

During review Ilija pointed out that adding the internal 
ZEND_ACC_DEPRECATED flag was skipped for abstract methods and interface 
methods, questioning the decision. The decision dates back to Benjamin's 
initial PoC implementation and I've had a short discussion with him 
about this, with the result that it's probably best to ask the list, as 
it was not specified in the RFC.

Currently adding the #[\Deprecated] attribute on an abstract method or 
interface method does not do anything special. It only applies the 
attribute, which then can be accessed via Reflection.

It would be easy to change the implementation to also apply the 
ZEND_ACC_DEPRECATED flag. This would have the following consequences:

1. `ReflectionMethod::isDeprecated()` will return `true`.
2. No deprecation error will be emitted. Neither when implementing the 
method in a child class, nor when calling the method of the child class.

(1) is probably the obviously correct behavior. For (2) the reasoning is 
as follows:

- The implementation relies on the pre-existing behavior of 
ZEND_ACC_DEPRECATED, which does not emit a deprecation error for 
overridden methods / implemented abstract or interface methods.

- If the deprecation error would be emitted when implementing the 
method, then the error would not be actionable. Child classes are still 
forced to implement the abstract method / interface method, thus there 
is no way to change the code to not emit the error.

- It is not obviously correct to inherit the deprecation when overriding 
a non-abstract method of a parent class, just like attributes themselves 
are not inherited [1]. Abstract methods and interface methods should 
keep consistency with non-abstract methods. The reasoning why it's not 
obviously correct to inherit the deprecation is that due to not 
inheriting the Attribute, a custom deprecation message would not be applied.

It goes without saying that IDEs can opt to suggest adding the 
#[\Deprecated] attribute if the overridden method is deprecated itself 
by means of the existing code diagnosis functionality, making the 
attribute on abstract methods / interface methods useful by that means.

We plan to adjust the implementation to also apply to abstract and 
interface methods with the semantics outlined in (1) and (2). Does 
anyone have an opinion about this / does anyone disagree with that decision?

Best regards
Tim Düsterhus

[1] https://3v4l.org/GIZRj

---

## #123782 — Nicolas Grekas — 2024-06-24T15:06:55+00:00

URL: https://externals.io/message/123397#123782 · In reply to: #123513

Le mer. 5 juin 2024 à 10:18, Benjamin Außenhofer <kontakt@beberlei.de> a
écrit :

>
>
> On Wed, May 22, 2024 at 9:22 AM Benjamin Außenhofer <kontakt@beberlei.de>
> wrote:
>
> > […]
>
> 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:
> > […]
>
> 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.

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.

Nicolas

---

## #123784 — Matthew Weier O'Phinney — 2024-06-24T16:33:31+00:00

URL: https://externals.io/message/123397#123784 · In reply to: #123782

On Mon, Jun 24, 2024 at 10:08 AM Nicolas Grekas <
nicolas.grekas+php@gmail.com> wrote:

>
>
> Le mer. 5 juin 2024 à 10:18, Benjamin Außenhofer <kontakt@beberlei.de> a
> écrit :
>
> > […]
>
> Hi Benjamin,
>
> The vote for the RFC #[\Deprecated] attribute is now open:
> > […]
>
> 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.
>
> 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.
>
>
Personally, I'd prefer the "since" format to mimic the notation that
Composer uses on the CLI when specifying a package with a constraint:
"symfony/yaml:7.2.0". This can be parsed easily, and won't suffer from
having arbitrary spacing and version naming prefixes.

(Still would prefer a "scheduledRemoval" field, as knowing when it was
deprecated is far less interesting than knowing when it will be removed.
Yes, I can assume the next major version, but what if it's major version +
1? What if a project allows removals during minor releases? Knowing what
version it will be removed in makes it far easier to understand what will
happen when I upgrade next.)

-- 
Matthew Weier O'Phinney
mweierophinney@gmail.com
https://mwop.net/
he/him

---

## #123799 — Stephen Reay — 2024-06-25T00:34:42+00:00

URL: https://externals.io/message/123397#123799 · In reply to: #123784



---

## #123800 — Matthew Weier O'Phinney — 2024-06-25T01:23:16+00:00

URL: https://externals.io/message/123397#123800 · In reply to: #123799

>
> Sent from my iPhone
>
> On 24 Jun 2024, at 23:43, Matthew Weier O'Phinney <
> mweierophinney@gmail.com> wrote:
>
> 
>
>
> On Mon, Jun 24, 2024 at 10:08 AM Nicolas Grekas <
> nicolas.grekas+php@gmail.com> wrote:
>
> > […]
> Personally, I'd prefer the "since" format to mimic the notation that
> Composer uses on the CLI when specifying a package with a constraint:
> "symfony/yaml:7.2.0". This can be parsed easily, and won't suffer from
> having arbitrary spacing and version naming prefixes.
>
> (Still would prefer a "scheduledRemoval" field, as knowing when it was
> deprecated is far less interesting than knowing when it will be removed.
> Yes, I can assume the next major version, but what if it's major version +
> 1? What if a project allows removals during minor releases? Knowing what
> version it will be removed in makes it far easier to understand what will
> happen when I upgrade next.)
>
>
> If you're marking a piece of code in  a project as deprecated, why does
> the deprecation notice need to re-specify the package the code is in?
> Wouldn't a regular version string be sufficient?
>

Because the consumer of the code might not know what package supplied it,
particularly if multiple packages share a namespace.

---

## #123838 — tim@bastelstu.be — 2024-06-25T18:39:20+00:00

URL: https://externals.io/message/123397#123838 · In reply to: #123782

Hi


> 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

---

## #123840 — kontakt@beberlei.de — 2024-06-25T20:00:50+00:00

URL: https://externals.io/message/123397#123840 · In reply to: #123782

On Mon, Jun 24, 2024 at 5:07 PM Nicolas Grekas <nicolas.grekas+php@gmail.com>
wrote:

>
>
> Le mer. 5 juin 2024 à 10:18, Benjamin Außenhofer <kontakt@beberlei.de> a
> écrit :
>
> > […]
>
> Hi Benjamin,
>
> The vote for the RFC #[\Deprecated] attribute is now open:
> > […]
>
> 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.


> 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.
>
> Nicolas

---

## #123937 — Nicolas Grekas — 2024-06-27T11:10:02+00:00

URL: https://externals.io/message/123397#123937 · In reply to: #123840

Le mar. 25 juin 2024 à 22:01, Benjamin Außenhofer <kontakt@beberlei.de> a
écrit :

>
>
> On Mon, Jun 24, 2024 at 5:07 PM Nicolas Grekas <
> nicolas.grekas+php@gmail.com> wrote:
>
> > […]
>
> 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

---

## #123948 — tim@bastelstu.be — 2024-06-27T17:55:29+00:00

URL: https://externals.io/message/123397#123948 · In reply to: #123937

Hi


> 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.

I've yet to hear a better solution for deprecation handling that does 
not involve third-party tooling. Being able to deprecate functionality 
is a core part of being able to evolve a programming language and just a 
note in the documentation no one reads anyways does not suffice.

> 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.

Thank you. I'll keep that in mind if/when we'll actually start working 
on an RFC adding support for deprecating 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.

I agree that just loading or defining a class should not emit a 
deprecation warning, just like loading or defining a function does not 
emit a deprecation warning.

> 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.

That is a fair concern.

I could imagine backporting the Attribute::TARGET_CLASS to all supported 
version if/when support for deprecating classes is actually added to the 
language and semantics are clear and considering that to be effectively 
a bugfix then.

> 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.

The PR implementing the #[\Deprecated] attribute will implement exactly 
the semantics that were decided in the RFC. The implementation is 
already be complete and is currently pending JIT review by Dmitry. 
Assuming no change requests during the review I'll make a final cleanup 
pass and then it should be ready to merge.

Anything that is not in the RFC will (need to) be part of a separate PR.

Best regards
Tim Düsterhus

---

## #124367 — tim@bastelstu.be — 2024-07-10T18:44:17+00:00

URL: https://externals.io/message/123397#124367 · In reply to: #123782

Hi

I'm specifically replying to Nicolas, but this email is directed to the 
general public, so please feel free to reply even if you are not called 
Nicolas :-)


> 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.
> 

I have merged the follow-up PR to apply the attribute earlier today:

https://github.com/php/php-src/commit/29f98e748568ebd66aaae061c0dcefbba92ca058

For now I've filled in the `$since` attribute with a bare version 
number, since this most closely matches what was described in the RFC.

I'm happy to do another follow-up on the format of the `$since` property 
for internal functions if we can reach a simple consensus here on the 
list. I think it should also be acceptable to do some basic processing 
of the `$since` property for the display in the human readable error 
message, making it easier to consume it programmatically without looking 
ugly to the human reader.

For PHP internal attributes the obvious choice would be:

     #[\Deprecated(since: 'PHP 8.4')]
     function foo() { }

which would result in an error message similar to:

     Deprecated: Function foo() is deprecated since PHP 8.4

The space probably is easy enough to understand for programmatic 
consumption in frameworks / error tracking libraries and the 'PHP' 
matches how the PHP requirements are defined in composer dependencies 
(once lowercased). I don't like the 'v' prefix for the version number, 
because to the best of my knowledge nothing else uses that (except for 
git tags).

You can find the discussion about the `$since` property in the first PR 
at https://github.com/php/php-src/pull/11293#discussion_r1647339607, if 
you want additional context.

Best regards
Tim Düsterhus