Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] <<Deprecated>> Attribute »

[RFC] <<Deprecated>> Attribute

php.internals

Benjamin Eberlei

6 years ago
Hi everyone, The attributes RFC specifically looked into the use of attributes at the engine/compiler level. To have a showcase of this with the 8.0 release I want to propose this RFC for a <<Deprecated>> attribute: https://wiki.php.net/rfc/deprecated_attribute <<Deprecated>> is the most obvious engine attribute to add at the beginning in my opinion, as - all other languages with Attributes/Annotations support have it as well - its effects are quite obvious to understand, as such offer a good example and intro to attributes - with potential property/class constant deprecation it adds something that was not possible in userland before. Let me know what you think! greetings Benjamin

Davey

6 years ago
On Thu, May 7, 2020 at 00:22 Benjamin Eberlei <kontakt@beberlei.de> wrote:
> Hi everyone, > > The attributes RFC specifically looked into the use of attributes at the > engine/compiler level. > > To have a showcase of this with the 8.0 release I want to propose this RFC > for a <<Deprecated>> attribute: > > https://wiki.php.net/rfc/deprecated_attribute > > <<Deprecated>> is the most obvious engine attribute to add at the beginning > in my opinion, as > > - all other languages with Attributes/Annotations support have it as well > - its effects are quite obvious to understand, as such offer a good example > and intro to attributes > - with potential property/class constant deprecation it adds something that > was not possible in userland before. > > Let me know what you think! > > greetings > Benjamin
Hi Benjamin, Two things: 1) as a matter of course, I prefer things that can be done in userland be done in userland unless there is a clear performance or other win. This doesn't qualify for me, except that it would be possible to use the same mechanism in core, but that really only gives us the ability to reason around depredations with reflection and I don't see much value in that. I'm not entirely convinced that a PSR and a defecto implementation isn't a better solution here. This isn't a show-stopper though. 2) AFAICT you cannot add an attribute to a function argument, and the only way I can see to resolve that is to add a "DeprecatedArgument" attribute applied to the function that takes the name as the first arg (and the message as the second). Now you're looking at: <<DeprecatedArgument('foo')>> <<DeprecatedArgument('bar')>> function bat($foo = null, $bar = null) { } It's not great IMO. Having said that, I don't think this bit is really necessary — unlike the other suggested uses, an argument isn't an isolated thing, so much as part of the function signature as a whole and removing arguments is better done by deprecating the entire function and introducing a new version with a different name. So maybe we just don't need this?

Benjamin Eberlei

6 years ago
On Thu, May 7, 2020 at 10:10 AM Davey Shafik <davey@php.net> wrote:
> > > On Thu, May 7, 2020 at 00:22 Benjamin Eberlei <kontakt@beberlei.de> wrote: > >> Hi everyone, >> >> The attributes RFC specifically looked into the use of attributes at the >> engine/compiler level. >> >> To have a showcase of this with the 8.0 release I want to propose this RFC >> for a <<Deprecated>> attribute: >> >> https://wiki.php.net/rfc/deprecated_attribute >> >> <<Deprecated>> is the most obvious engine attribute to add at the >> beginning >> in my opinion, as >> >> - all other languages with Attributes/Annotations support have it as well >> - its effects are quite obvious to understand, as such offer a good >> example >> and intro to attributes >> - with potential property/class constant deprecation it adds something >> that >> was not possible in userland before. >> >> Let me know what you think! >> >> greetings >> Benjamin > > > Hi Benjamin, > > Two things: > > 1) as a matter of course, I prefer things that can be done in userland be > done in userland unless there is a clear performance or other win. This > doesn't qualify for me, except that it would be possible to use the same > mechanism in core, but that really only gives us the ability to reason > around depredations with reflection and I don't see much value in that. I'm > not entirely convinced that a PSR and a defecto implementation isn't a > better solution here. This isn't a show-stopper though. >
Can you clarify a bit your argument "ability to reason around deprecations only with reflection"? At the moment we can only reason about deprecations by using both token_get_all and finding that a method calls trigger_error E_USER_DEPRECATED and by looking at the doc comments (via reflection) for "@deprecated". My argument for this attribute is that it leads to both a documentation based deprecation, and a runtime declaration at the same time. It is a better result than what we can do in userland in my opinion.
> > 2) AFAICT you cannot add an attribute to a function argument, and the only > way I can see to resolve that is to add a "DeprecatedArgument" attribute > applied to the function that takes the name as the first arg (and the > message as the second). Now you're looking at: > > <<DeprecatedArgument('foo')>> > <<DeprecatedArgument('bar')>> > function bat($foo = null, $bar = null) { } > > It's not great IMO. Having said that, I don't think this bit is really > necessary — unlike the other suggested uses, an argument isn't an isolated > thing, so much as part of the function signature as a whole and removing > arguments is better done by deprecating the entire function and introducing > a new version with a different name. So maybe we just don't need this? >
It is possible to add attributes to parameters. This was added as an improvement during the Attributes RFC (it was not possible in the first draft).

Davey

6 years ago
On Thu, May 7, 2020 at 01:18 Benjamin Eberlei <kontakt@beberlei.de> wrote:
> > > On Thu, May 7, 2020 at 10:10 AM Davey Shafik <davey@php.net> wrote: > >> >> >> On Thu, May 7, 2020 at 00:22 Benjamin Eberlei <kontakt@beberlei.de> >> wrote: >> >>> Hi everyone, >>> >>> The attributes RFC specifically looked into the use of attributes at the >>> engine/compiler level. >>> >>> To have a showcase of this with the 8.0 release I want to propose this >>> RFC >>> for a <<Deprecated>> attribute: >>> >>> https://wiki.php.net/rfc/deprecated_attribute >>> >>> <<Deprecated>> is the most obvious engine attribute to add at the >>> beginning >>> in my opinion, as >>> >>> - all other languages with Attributes/Annotations support have it as well >>> - its effects are quite obvious to understand, as such offer a good >>> example >>> and intro to attributes >>> - with potential property/class constant deprecation it adds something >>> that >>> was not possible in userland before. >>> >>> Let me know what you think! >>> >>> greetings >>> Benjamin >> >> >> Hi Benjamin, >> >> Two things: >> >> 1) as a matter of course, I prefer things that can be done in userland be >> done in userland unless there is a clear performance or other win. This >> doesn't qualify for me, except that it would be possible to use the same >> mechanism in core, but that really only gives us the ability to reason >> around depredations with reflection and I don't see much value in that. I'm >> not entirely convinced that a PSR and a defecto implementation isn't a >> better solution here. This isn't a show-stopper though. >> > > Can you clarify a bit your argument "ability to reason around deprecations > only with reflection"? > > At the moment we can only reason about deprecations by using both > token_get_all and finding that a method calls trigger_error > E_USER_DEPRECATED and by looking at the doc comments (via reflection) for > "@deprecated". My argument for this attribute is that it leads to both a > documentation based deprecation, and a runtime declaration at the same > time. It is a better result than what we can do in userland in my opinion. >
I don't see any valid reason to be checking for deprecations at runtime with reflection. This attribute has more value when reading the code or doing static analysis. Neither of which requires that it be implemented in core. Plus, userland gives us the reflection ability *anyway*.
> > >> >> 2) AFAICT you cannot add an attribute to a function argument, and the >> only way I can see to resolve that is to add a "DeprecatedArgument" >> attribute applied to the function that takes the name as the first arg (and >> the message as the second). Now you're looking at: >> >> <<DeprecatedArgument('foo')>> >> <<DeprecatedArgument('bar')>> >> function bat($foo = null, $bar = null) { } >> >> It's not great IMO. Having said that, I don't think this bit is really >> necessary — unlike the other suggested uses, an argument isn't an isolated >> thing, so much as part of the function signature as a whole and removing >> arguments is better done by deprecating the entire function and introducing >> a new version with a different name. So maybe we just don't need this? >> > > It is possible to add attributes to parameters. This was added as an > improvement during the Attributes RFC (it was not possible in the first > draft). >
I went back and looked and still missed that, oops! I still stand by my statement about it being bad practice ;) - Davey

Benjamin Eberlei

6 years ago
On Thu, May 7, 2020 at 10:27 AM Davey Shafik <davey@php.net> wrote:
> > > On Thu, May 7, 2020 at 01:18 Benjamin Eberlei <kontakt@beberlei.de> wrote: > >> >> >> On Thu, May 7, 2020 at 10:10 AM Davey Shafik <davey@php.net> wrote: >> >>> >>> >>> On Thu, May 7, 2020 at 00:22 Benjamin Eberlei <kontakt@beberlei.de> >>> wrote: >>> >>>> Hi everyone, >>>> >>>> The attributes RFC specifically looked into the use of attributes at the >>>> engine/compiler level. >>>> >>>> To have a showcase of this with the 8.0 release I want to propose this >>>> RFC >>>> for a <<Deprecated>> attribute: >>>> >>>> https://wiki.php.net/rfc/deprecated_attribute >>>> >>>> <<Deprecated>> is the most obvious engine attribute to add at the >>>> beginning >>>> in my opinion, as >>>> >>>> - all other languages with Attributes/Annotations support have it as >>>> well >>>> - its effects are quite obvious to understand, as such offer a good >>>> example >>>> and intro to attributes >>>> - with potential property/class constant deprecation it adds something >>>> that >>>> was not possible in userland before. >>>> >>>> Let me know what you think! >>>> >>>> greetings >>>> Benjamin >>> >>> >>> Hi Benjamin, >>> >>> Two things: >>> >>> 1) as a matter of course, I prefer things that can be done in userland >>> be done in userland unless there is a clear performance or other win. This >>> doesn't qualify for me, except that it would be possible to use the same >>> mechanism in core, but that really only gives us the ability to reason >>> around depredations with reflection and I don't see much value in that. I'm >>> not entirely convinced that a PSR and a defecto implementation isn't a >>> better solution here. This isn't a show-stopper though. >>> >> >> Can you clarify a bit your argument "ability to reason around >> deprecations only with reflection"? >> >> At the moment we can only reason about deprecations by using both >> token_get_all and finding that a method calls trigger_error >> E_USER_DEPRECATED and by looking at the doc comments (via reflection) for >> "@deprecated". My argument for this attribute is that it leads to both a >> documentation based deprecation, and a runtime declaration at the same >> time. It is a better result than what we can do in userland in my opinion. >> > > I don't see any valid reason to be checking for deprecations at runtime > with reflection. This attribute has more value when reading the code or > doing static analysis. Neither of which requires that it be implemented in > core. Plus, userland gives us the reflection ability *anyway*. >
This attribute is "reflected" by the engine during compilation, userland doesn't have to reflect it again itself. on a code level the engine "patches" each function automatically, this would do the following at compile time: @@ -1,6 +1,6 @@ <?php -<<Deprecated>> function foo() { + trigger_error('Function foo() is deprecated', E_USER_DEPRECATED); return 'foo'; }
> > >> >> >>> >>> 2) AFAICT you cannot add an attribute to a function argument, and the >>> only way I can see to resolve that is to add a "DeprecatedArgument" >>> attribute applied to the function that takes the name as the first arg (and >>> the message as the second). Now you're looking at: >>> >>> <<DeprecatedArgument('foo')>> >>> <<DeprecatedArgument('bar')>> >>> function bat($foo = null, $bar = null) { } >>> >>> It's not great IMO. Having said that, I don't think this bit is really >>> necessary — unlike the other suggested uses, an argument isn't an isolated >>> thing, so much as part of the function signature as a whole and removing >>> arguments is better done by deprecating the entire function and introducing >>> a new version with a different name. So maybe we just don't need this? >>> >> >> It is possible to add attributes to parameters. This was added as an >> improvement during the Attributes RFC (it was not possible in the first >> draft). >> > > I went back and looked and still missed that, oops! I still stand by my > statement about it being bad practice ;) >
Yes you might be right that the parameter deprecation is the one with the "least" value from all of them.

Marco Pivetta

6 years ago
Hey Benjamin, On Thu, May 7, 2020, 10:42 Benjamin Eberlei <kontakt@beberlei.de> wrote:
> > This attribute is "reflected" by the engine during compilation, userland > doesn't have to reflect it again itself. on a code level the engine > "patches" each function automatically, this would do the following at > compile time: > > @@ -1,6 +1,6 @@ > <?php > > -<<Deprecated>> > function foo() { > + trigger_error('Function foo() is deprecated', E_USER_DEPRECATED); > return 'foo'; > } >
Already discussed it elsewhere, but I don't think that adding more runtime side-effects, especially on otherwise referentially transparent code, is a good idea. Very much against the current wave on `@trigger_error()` wave that symfony introduced. The rationale behind that hacky/messy approach is that sometimes deprecations are not determined statically (for example, if a specific parameter **value** is given), but I see no reason to do the same for static references. Since this would declare deprecations on static symbols, it can be detected statically, not at runtime. You will also get deprecations for very deeply nested components, which are transitive dependencies of what you effectively use in your sources: seen that in the wild already, and it wasn't a fun ride either. If you still want warnings at runtime, you can plug in something like GO-AOP-PHP, which kinda does exactly the same thing, but opt-in and not as a standardized side-effects engine (bad). Very similar PoV of Davey: since recently, we have the static analysis tools; let's endorse static analysis instead.

Benjamin Eberlei

6 years ago
On Thu, May 7, 2020 at 10:54 AM Marco Pivetta <ocramius@gmail.com> wrote:
> Hey Benjamin, > > On Thu, May 7, 2020, 10:42 Benjamin Eberlei <kontakt@beberlei.de> wrote: > >> >> This attribute is "reflected" by the engine during compilation, userland >> doesn't have to reflect it again itself. on a code level the engine >> "patches" each function automatically, this would do the following at >> compile time: >> >> @@ -1,6 +1,6 @@ >> <?php >> >> -<<Deprecated>> >> function foo() { >> + trigger_error('Function foo() is deprecated', E_USER_DEPRECATED); >> return 'foo'; >> } >> > > Already discussed it elsewhere, but I don't think that adding more runtime > side-effects, especially on otherwise referentially transparent code, is a > good idea. > > Very much against the current wave on `@trigger_error()` wave that symfony > introduced. The rationale behind that hacky/messy approach is that > sometimes deprecations are not determined statically (for example, if a > specific parameter **value** is given), but I see no reason to do the same > for static references. >
While I agree with you on @trigger_errror, this is a seperate issue. PHP itself triggers all its deprecations with trigger_error(..., E_DEPRECATED), so this feature should do the same thing. I started a discussion about how we can improve error handling in general here: https://externals.io/message/110000 - first with some internal changes. Based on that userland changes are the next thing that should follow, but need a lot more care to avoid BC breaks.
> Since this would declare deprecations on static symbols, it can be > detected statically, not at runtime. > > You will also get deprecations for very deeply nested components, which > are transitive dependencies of what you effectively use in your sources: > seen that in the wild already, and it wasn't a fun ride either. >
Based on <<Deprecated>> there is at least the chance to avoid transitive dependencies, that means preventing deprecation errors to get triggered, when we are already in a deprecated function that triggered an error in the stack above. but I haven't investigated this further yet.
> > If you still want warnings at runtime, you can plug in something like > GO-AOP-PHP, which kinda does exactly the same thing, but opt-in and not as > a standardized side-effects engine (bad). >
You can always disable this at runtime by removing E_USER_DEPRECATED from error_reporting.

Côme Chilliet

6 years ago
This is a really good proposal. I did not participate in the attribute discussion/vote as I do not really understand what attributes are and what they are good for. Having this kind of examples in core will help a lot, and is useful too (I do have functions with only a trigger_error of deprecation in my code already).
-- Côme Chilliet FusionDirectory - https://www.fusiondirectory.org

Nicolas Grekas

6 years ago
Hi Benjamin, https://wiki.php.net/rfc/deprecated_attribute Thanks for the RFC. Why didn't you mention support for deprecating classes/interfaces/traits themselves? If there is a reason, it should be in the RFC at least to me. - all other languages with Attributes/Annotations support have it as well
>
Did you research what runtime side effects the other languages provide? I'm concerned about runtime side-effects. Attributes should lead to none of them to me. The moment an attribute leads to runtime side effects, it should be turned into code instead, because it looses its "declarative" property. From my experience, e.g. a method can be declared as deprecated but can *still* be called internally without any issue. This should not be reported to anyone as its internal. Preserving BC often requires this, and only runtime logic can decide when a deprecation should be reported to the end-user. Note that this is different from configuring the global error reporting level: deciding *locally* that some declaration should not be reported is the important part of this. That's also why I very much favor @trigger_error() and why static analysis works only for the simpler cases. If one wants to save the duplicate "declaration" (attribute + explicit call inline), then this should be opt-in to me, e.g. <<Deprecated(true)>>. As an additional step, I very much which that the attribute could accept two arguments: the package and the version of it. From my experience, making deprecations actionable is critical to help ppl to fix them. And making them actionable starts with allowing ppl to quickly identify which package cannot be upgraded to the next major yet. This is what make us work on symfony/deprecation-contracts <https://github.com/symfony/deprecation-contracts>, which provide this single trigger_deprecation(string $package, string $version, string $message , ...$args); function. From the example in the readme: trigger_deprecation('symfony/blockchain', '8.9', 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin'); Generates: Since symfony/blockchain 8.9: Using "bitcoin" is deprecated, use "fabcoin" instead. Joke aside, this is very useful and I wish the attribute could borrow from the experience we gathered on the topic. This would be nice: <<Deprecated('symfony/blockchain', '8.9', 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin')>> It could also be quiteuseful to have this trigger_deprecation() function in core, but that's another topic :) Cheers, Nicolas

Benjamin Eberlei

6 years ago
On Thu, May 7, 2020 at 12:43 PM Nicolas Grekas <nicolas.grekas+php@gmail.com> wrote:
> Hi Benjamin, > > https://wiki.php.net/rfc/deprecated_attribute > > > Thanks for the RFC. > Why didn't you mention support for deprecating classes/interfaces/traits > themselves? > If there is a reason, it should be in the RFC at least to me. >
It feels complicated to me, because there are so many entry points to a class: 0. declaring/loading it 1. extending it, but not always because the replacement could extend it 2. calling it statically 3. instantiating it 4. i probably forget a bunch. I think this requires spreading the code to handle this around into many places and is not easy to do without overhead for classes that are *not* themselves deprecated. So I decided to keep this out for now.
> - all other languages with Attributes/Annotations support have it as well >> > > Did you research what runtime side effects the other languages provide? >
They mostly warn or fail on compile and don't have runtime effects. The same goes for any other deprecation that PHP triggers though, they are all at runtime. So I don't think this can be compared, similar to how the error stack of PHP cannot be compared to any other language.
> > I'm concerned about runtime side-effects. Attributes should lead to none > of them to me. The moment an attribute leads to runtime side effects, it > should be turned into code instead, because it looses its "declarative" > property. >
To me Symfony using Annotations at the moment is also all about side effects based on declared annotations that are then automatically turned into code. This is the same happening here. The difference to this example here https://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/security.html#usage is only that <<Deprecated>> will generate the code into the function/method, whereas the annotations in the Symfony example generate the code "around" (before) the method call.
> > From my experience, e.g. a method can be declared as deprecated but can > *still* be called internally without any issue. This should not be reported > to anyone as its internal. Preserving BC often requires this, and only > runtime logic can decide when a deprecation should be reported to the > end-user. Note that this is different from configuring the global error > reporting level: deciding *locally* that some declaration should not be > reported is the important part of this. >
Can I ask how you are handling this use-case in Symfony at the moment? If you catch that in the error handler by looking at the backtrace, then this is exactly possible with this declarative approach as well, in fact the generated code is just calling trigger_error() and then triggering user error handlers.
> > That's also why I very much favor @trigger_error() and why static analysis > works only for the simpler cases. >
> If one wants to save the duplicate "declaration" (attribute + explicit > call inline), then this should be opt-in to me, e.g. <<Deprecated(true)>>. >
If you only want the static analysis effect, developers are still free to use /** @deprecated */ instead, which I don't expect either static analysis tools nor IDEs will drop support for.
> > As an additional step, I very much which that the attribute could accept > two arguments: the package and the version of it. From my experience, > making deprecations actionable is critical to help ppl to fix them. And > making them actionable starts with allowing ppl to quickly identify which > package cannot be upgraded to the next major yet. >
> This is what make us work on symfony/deprecation-contracts > <https://github.com/symfony/deprecation-contracts>, which provide this > single trigger_deprecation(string $package, string $version, string $ > message, ...$args); function. From the example in the readme: > > trigger_deprecation('symfony/blockchain', '8.9', 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin'); > > Generates: > Since symfony/blockchain 8.9: Using "bitcoin" is deprecated, use "fabcoin" instead. > > Joke aside, this is very useful and I wish the attribute could borrow from the experience we gathered on the topic. > > This would be nice: > <<Deprecated('symfony/blockchain', '8.9', 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin')>> > > The Deprecated attribute accepts a message that gets appended to the
default message, so you could do <<Deprecated("since symfony/blockchain 8.9, use fabcoin() instead")>>, which would turn the message into: Deprecated: Function bitcoin() is deprecated, since symfony/blockchain 8.9, use fabcoin() instead in /path/to/file:123 A free-form text is more flexible for use-cases where there are no package and versions, we can't expect to assume every deprecation will be triggered from a versioned composer package only.
> > It could also be quiteuseful to have this trigger_deprecation() function in core, but that's another topic :) > > Yes, a declarative statement like <<Deprecated>> would have the benefit of
keeping the implementation hidden, so details of it could potentially be changed.

Nicolas Grekas

6 years ago
> > https://wiki.php.net/rfc/deprecated_attribute
Thanks for the RFC. Why didn't you mention support for deprecating classes/interfaces/traits themselves? If there is a reason, it should be in the RFC at least to me.
> > It feels complicated to me, because there are so many entry points to a > class: 0. declaring/loading it 1. extending it, but not always because the > replacement could extend it 2. calling it statically 3. instantiating it 4. > i probably forget a bunch. I think this requires spreading the code to > handle this around into many places and is not easy to do without overhead > for classes that are *not* themselves deprecated. So I decided to keep this > out for now. >
It is. But the other cases are equally complex. That's the point I tried to make in my previous answer and that I will continue to make by sharing my experience on the topic. That's why I'm pretty sure that runtime side-effect will severely limit the usefulness of this attribute. I very much wish we would adopt the tag, but without any runtime side effect (which means in practice that php-internals doesn't need to define it, this could be left to userland.) - all other languages with Attributes/Annotations support have it as well
>>> >> >> Did you research what runtime side effects the other languages provide? >> > > They mostly warn or fail on compile and don't have runtime effects. The > same goes for any other deprecation that PHP triggers though, they are all > at runtime. So I don't think this can be compared, similar to how the error > stack of PHP cannot be compared to any other language. >
OK thanks.
> I'm concerned about runtime side-effects. Attributes should lead to none >> of them to me. The moment an attribute leads to runtime side effects, it >> should be turned into code instead, because it looses its "declarative" >> property. >> > > To me Symfony using Annotations at the moment is also all about side > effects based on declared annotations that are then automatically turned > into code. This is the same happening here. >
That's somewhat more subtle. Most annotations don't lead to any direct notice. We also double these annotations with calls to trigger_error() to notify end users about them. Both ways are required to fit several kinds of reporting tools (runtime collectors and static analyzers). This works well when calling a method or when instantiating a class. In other situations, annotations are not helpful, because the deprecations map to e.g. a specific value of an argument. That's something that no static analyzer can spot and only inline trigger_error() can report. When extending a class, in debug mode only, we have a special autoloader that uses reflection to check whether the parent class or one of its method are deprecated. It triggers a deprecation notice when it detects this situation. Before triggering the notice, it also checks the namespace of the parent class: if it matches the namespace of the child class, no deprecations are reported. This allows the very common pattern of ("new API" extends "legacy API"), which is required to make the "new API" pass the old type hints that we have to keep in place for BC. This aspect is unaddressed in your proposal (and doesn't need to be actually, pure declarations are fine, others can build or reuse the same logic in userland). We also have another strategy to report deprecations related to inheritance, which is purely local (no need for the autoloader I just described). By using reflection, we sometimes check inline whether the currently called method is being overridden, and we trigger a deprecation if yes (or e.g. if the signature of the child method misses an argument that we want to add in the future).
> From my experience, e.g. a method can be declared as deprecated but can >> *still* be called internally without any issue. This should not be reported >> to anyone as its internal. Preserving BC often requires this, and only >> runtime logic can decide when a deprecation should be reported to the >> end-user. Note that this is different from configuring the global error >> reporting level: deciding *locally* that some declaration should not be >> reported is the important part of this. >> > > Can I ask how you are handling this use-case in Symfony at the moment? If > you catch that in the error handler by looking at the backtrace, then this > is exactly possible with this declarative approach as well, in fact the > generated code is just calling trigger_error() and then triggering user > error handlers. >
There is no cooperation ever between the error handler and the decision to locally report a deprecation. All deprecation triggering sites in the code decide on their own. Also, the backtrace is never ever used to make such a decision. In order to *not* trigger a deprecation when calling a deprecated method, we call the method with an extra argument that we read via func_get_arg(). There is no other more accurate way. The variety of strategies that we use is quite high because the variety of situations that exists in real apps is also quite high, so we had to find a way for each of them. I invite you to check the source code of Symfony, branch 3.4 or 4.4, to have an example of each of them. I would be happy to guide you (or anyone reading) through this if you want to take the journey, that's quite useful for the discussion if we want to get deeper into it.
> That's also why I very much favor @trigger_error() and why static analysis >> works only for the simpler cases. >> > >> If one wants to save the duplicate "declaration" (attribute + explicit >> call inline), then this should be opt-in to me, e.g. <<Deprecated(true)>>. >> > > If you only want the static analysis effect, developers are still free to > use /** @deprecated */ instead, which I don't expect either static analysis > tools nor IDEs will drop support for. >
That'd be a quite unsatisfactory outcome...
> As an additional step, I very much wish that the attribute could accept >> two arguments: the package and the version of it. From my experience, >> making deprecations actionable is critical to help ppl to fix them. And >> making them actionable starts with allowing ppl to quickly identify which >> package cannot be upgraded to the next major yet. >> > >> This is what make us work on symfony/deprecation-contracts >> <https://github.com/symfony/deprecation-contracts>, which provide this >> single trigger_deprecation(string $package, string $version, string $ >> message, ...$args); function. From the example in the readme: >> >> trigger_deprecation('symfony/blockchain', '8.9', 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin'); >> >> Generates: >> Since symfony/blockchain 8.9: Using "bitcoin" is deprecated, use "fabcoin" instead. >> >> Joke aside, this is very useful and I wish the attribute could borrow from the experience we gathered on the topic. >> >> This would be nice: >> <<Deprecated('symfony/blockchain', '8.9', 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin')>> >> >> The Deprecated attribute accepts a message that gets appended to the > default message, so you could do <<Deprecated("since symfony/blockchain > 8.9, use fabcoin() instead")>>, which would turn the message into: > > Deprecated: Function bitcoin() is deprecated, since symfony/blockchain > 8.9, use fabcoin() instead in /path/to/file:123 > > A free-form text is more flexible for use-cases where there are no package > and versions, we can't expect to assume every deprecation will be triggered > from a versioned composer package only. >
A free-form text is what we're using since years yes. It works, but it makes it hard if not impossible to report structured info about which package block and cannot be updated to their next major. Providing this info in a structured way is useful and that'd what I'm trying to convey here. I think we *can* assume that every deprecation will be able to report some package+version info (it doesn't have to be a composer package, what matters is having the fields easily identifiable) Nicolas

=?utf-8?B?5rGf5rmW5aSn6Jm+5LuB?=

6 years ago
Hi Benjamin, Thanks for the RFC to bring real cases of attribute into the core. Here are some questions from my side: 1. what's the use case for deprecation against parameters? This feature is really not useful IMO. For example, ` function test2(<<Deprecated>> $value) {}` is not only deprecating the `$value` parameter, it's deprecating the first parameter. So actually it means we are not recommending to call `test2` with any parameter, and it also limits the ability to add more parameters in the future. The real cases I saw is more complex then just deprecate the parameter itself. usually what we deprecated are certain types/values of the parameter, which requires further check. 2. why you propose "for class constants/properties and parameters maybe 8.1" Is there any issue blocking us to get them in 8.0 as well? 3. default message for class functions. ` // Deprecated: Method Foo::test() is deprecated in %s` From my understanding, the %s here is where this method has been called/defined, while other message doesn't include this. I think we might remove it for consistency. 4. message for methods/functions For class property/constant, you clarify when the deprecated message will come up. But for the methods/functions this is missing. I think it would be better if you make it more clear. Thanks, CHU Zhaowei -----Original Message----- From: Benjamin Eberlei <kontakt@beberlei.de> Sent: Thursday, May 7, 2020 3:22 PM To: PHP Internals <internals@lists.php.net> Subject: [PHP-DEV] [RFC] <<Deprecated>> Attribute Hi everyone, The attributes RFC specifically looked into the use of attributes at the engine/compiler level. To have a showcase of this with the 8.0 release I want to propose this RFC for a <<Deprecated>> attribute: https://wiki.php.net/rfc/deprecated_attribute <<Deprecated>> is the most obvious engine attribute to add at the beginning in my opinion, as - all other languages with Attributes/Annotations support have it as well - its effects are quite obvious to understand, as such offer a good example and intro to attributes - with potential property/class constant deprecation it adds something that was not possible in userland before. Let me know what you think! greetings Benjamin