Hi internals,
I have updated the RFC for a #[Deprecated] attribute that wasn't completed
for PHP 8.0 due to time constraints and I am able to restart the discussion
now.
https://wiki.php.net/rfc/deprecated_attribute
The following updates have been made:
- focus on only method and function deprecations for now, removed
class/property/constant deprecations. - a section on explaining the runtime effects of deprecations in PHP, and a
note that this RFC is about completing deprecation support within the
existing model, while changes to deprecations in general are out of scope /
a disjunct concern for a different RFC.
Sara proposed a much improved implementation over my initial patch, by
using the already existing ZEND_ACC_DEPRECATED constant on userland
functions. The resulting implementation is therefore much simpler and
really just extending existing function deprecation support from internal
to userland functions. You can find the PR here:
https://github.com/php/php-src/pull/6521
Let me know what you think.
greetings
Benjamin
Hi Benjamin,
I have updated the RFC for a #[Deprecated] attribute that wasn't completed
for PHP 8.0 due to time constraints and I am able to restart the discussion
now.https://wiki.php.net/rfc/deprecated_attribute
The following updates have been made:
- focus on only method and function deprecations for now, removed
class/property/constant deprecations.- a section on explaining the runtime effects of deprecations in PHP, and a
note that this RFC is about completing deprecation support within the
existing model, while changes to deprecations in general are out of scope /
a disjunct concern for a different RFC.Sara proposed a much improved implementation over my initial patch, by
using the already existing ZEND_ACC_DEPRECATED constant on userland
functions. The resulting implementation is therefore much simpler and
really just extending existing function deprecation support from internal
to userland functions. You can find the PR here:
Thanks for the RFC.
It would be great to allow adding this attribute on classes. What about
allowing it right now and not bind it to any runtime side-effect? That
would allow static analyzers to do their job. Same for consts and
properties by the way.
Also, it would be very useful to add named parameters to the attribute,
namely: "package" (the name of the package that declares the deprecation)
and "version" (the version of that package that introduced the
deprecation), next to the message.
This is critical info when building reports of deprecations.
Thanks for considering,
Nicolas
On Tue, Dec 22, 2020 at 12:35 PM Nicolas Grekas <
nicolas.grekas+php@gmail.com> wrote:
It would be great to allow adding this attribute on classes. What about
allowing it right now and not bind it to any runtime side-effect? That
would allow static analyzers to do their job. Same for consts and
properties by the way.Also, it would be very useful to add named parameters to the attribute,
namely: "package" (the name of the package that declares the deprecation)
and "version" (the version of that package that introduced the
deprecation), next to the message.This is critical info when building reports of deprecations.
You could do that now with a polyfill from userspace. If the annotation
need not have an effect, then it's just any other userspace implementation.
-Sara
You could do that now with a polyfill from userspace. If the annotation
need not have an effect, then it's just any other userspace implementation.
It's possible now, while the attribute is not provided by PHP. But once it
gets introduced and if it forbids usage on some elements it wouldn't be
possible to fix this from userspace.
--
Best regards,
Bruce Weirdan mailto:
weirdan@gmail.com
Hi Sara
On Tue, Dec 22, 2020 at 12:35 PM Nicolas Grekas <
nicolas.grekas+php@gmail.com> wrote:It would be great to allow adding this attribute on classes. What about
allowing it right now and not bind it to any runtime side-effect? That
would allow static analyzers to do their job. Same for consts and
properties by the way.Also, it would be very useful to add named parameters to the attribute,
namely: "package" (the name of the package that declares the deprecation)
and "version" (the version of that package that introduced the
deprecation), next to the message.This is critical info when building reports of deprecations.
You could do that now with a polyfill from userspace. If the annotation
need not have an effect, then it's just any other userspace implementation.
The difference is that PHP core has the ability to force standarization. There's already JetBrains' implementation of #[Deprecated], which Psalm and PhpStan also support, but it's not a real standard. Maybe the FIG would one day step in to decide these kinds of things, but the reality is that many major frameworks don't follow FIG as closely as they used to. I think there's value in adding attributes in the core, with the goal only being static analysis. It'll allow for consistency and that's a valuable thing.
-Sara
Kind regards
Brent
Hi Sara
On Tue, Dec 22, 2020 at 12:35 PM Nicolas Grekas <
nicolas.grekas+php@gmail.com> wrote:It would be great to allow adding this attribute on classes. What about
allowing it right now and not bind it to any runtime side-effect? That
would allow static analyzers to do their job. Same for consts and
properties by the way.Also, it would be very useful to add named parameters to the attribute,
namely: "package" (the name of the package that declares the
deprecation)
and "version" (the version of that package that introduced the
deprecation), next to the message.This is critical info when building reports of deprecations.
You could do that now with a polyfill from userspace. If the annotation
need not have an effect, then it's just any other userspace
implementation.The difference is that PHP core has the ability to force standarization.
There's already JetBrains' implementation of #[Deprecated], which Psalm and
PhpStan also support, but it's not a real standard. Maybe the FIG would one
day step in to decide these kinds of things, but the reality is that many
major frameworks don't follow FIG as closely as they used to. I think
there's value in adding attributes in the core, with the goal only being
static analysis. It'll allow for consistency and that's a valuable thing.
I want to keep #[Deprecated] on other elements than functions / methods out
of this RFC, because they require entirely different implementation
approaches.
We identified in the PR already that this should at some point be
standardized in core, because internal attributes will currently not be
able to support extension through inheritance by userland for
implementation reasons.
My next RFC update will reflect this future scope.
-Sara
Kind regards
BrentTo unsubscribe, visit: https://www.php.net/unsub.php
Hi internals,
I have updated the RFC for a #[Deprecated] attribute that wasn't completed
for PHP 8.0 due to time constraints and I am able to restart the discussion
now.https://wiki.php.net/rfc/deprecated_attribute
The following updates have been made:
- focus on only method and function deprecations for now, removed
class/property/constant deprecations.- a section on explaining the runtime effects of deprecations in PHP, and a
note that this RFC is about completing deprecation support within the
existing model, while changes to deprecations in general are out of scope /
a disjunct concern for a different RFC.Sara proposed a much improved implementation over my initial patch, by
using the already existing ZEND_ACC_DEPRECATED constant on userland
functions. The resulting implementation is therefore much simpler and
really just extending existing function deprecation support from internal
to userland functions. You can find the PR here:https://github.com/php/php-src/pull/6521
Let me know what you think.
greetings
Benjamin
Just to clarify this raises an E_DEPRECATED right?
Could it make sense to raise E_USER_DEPRECATED instead?
Best regards,
George P. Banyard
Just to clarify this raises an
E_DEPRECATEDright?
Could it make sense to raiseE_USER_DEPRECATEDinstead?
I hadn't checked this before, but as per George's message, this is worrying.
I've been quite loud about it in the past, but static metadata definitions
should really not lead to runtime side-effects, especially if not pure
(error handling system tripped here).
I'd be totally for a built-in #[Deprecated] attribute if it did NOT
have a runtime behavior, which is an absolute no-go from my PoV.
This stuff is easily introspectible via tools like phpstan, psalm, phan,
and does not need to lead to more problematic runtime issues.
Marco Pivetta
Just to clarify this raises an
E_DEPRECATEDright?
Could it make sense to raiseE_USER_DEPRECATEDinstead?I hadn't checked this before, but as per George's message, this is
worrying.I've been quite loud about it in the past, but static metadata definitions
should really not lead to runtime side-effects, especially if not pure
(error handling system tripped here).I'd be totally for a built-in
#[Deprecated]attribute if it did NOT
have a runtime behavior, which is an absolute no-go from my PoV.This stuff is easily introspectible via tools like phpstan, psalm, phan,
and does not need to lead to more problematic runtime issues.
I get where you are coming from, but side-effect based notices/deprecations
is just the way PHP works at the moment and as such this existing mechanism
should be used and extended.
I do have (vague) plans to tackle alternative ways to process
notices/deprecations in the future, but this is independent from that and
#[Deprecated] would automatically tie into a change of this kind.
Marco Pivetta
Hey Benjamin
On Wed, Jan 13, 2021 at 1:47 PM Benjamin Eberlei kontakt@beberlei.de
wrote:
I get where you are coming from, but side-effect based
notices/deprecations is just the way PHP works at the moment and as such
this existing mechanism should be used and extended.I do have (vague) plans to tackle alternative ways to process
notices/deprecations in the future, but this is independent from that and
#[Deprecated] would automatically tie into a change of this kind.
If you have vague plans, perhaps introducing #[Deprecated] first, then
proposing a separate RFC for the runtime behavior?
For me, runtime behavior is:
- a big problem with existing tools that started doing this
- a production outage risk
- something that attributes shouldn't do anyway, at least not out of the
box (AOP OOTB is quite ugly and unexpected)
As usual, I'm fighting for pushing things into compile-time rather than
runtime, as runtime in PHP is already quite the nightmare of things to keep
in mind.
Marco Pivetta
For me, runtime behavior is:
[...]
- a production outage risk
If your production code can cause outages based on E_DEPRECATED notices,
then that's a bug in your code. I can't think of any justification for a
production system to abort because of a notification about future changes.
If we have to consider every introduction of a deprecation notice as a
breaking change, then we're stuck in a Catch-22, because the whole point
of such notices is to warn of upcoming breaking changes.
As usual, I'm fighting for pushing things into compile-time rather than
runtime
In principle, I do agree with this, but as Benjamin says, it's quite a
meaty topic in its own right. It's also unlikely to ever eliminate 100%
of runtime checks, because you can't statically analyse completely
dynamic code.
Defining #[Deprecated] as "do what all the other deprecations do", and
later changing that for all existing deprecations seems reasonable.
Regards,
--
Rowan Tommins
[IMSoP]