[PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3

[Benjamin_Aussenhofer]

Hi internals,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute

PR: https://github.com/php/php-src/pull/11293

Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

greetings
Benjamin

[Levi_Morrison]

On Tue, Apr 23, 2024 at 7:30 AM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

Hi internals,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: PHP: rfc:deprecated_attribute
PR: RFC: Add `#[\Deprecated]` Attribute by beberlei · Pull Request #11293 · php/php-src · GitHub
Old discussion: #[Deprecated] Attribute - Externals

Let me know about your questions and feedback.

greetings
Benjamin

I skimmed through the previous discussion and didn't see anything
about adding a `since` property. This is occasionally useful, at least
in my limited usage of it in Rust. The names below are modelled after
the names in [Rust's deprecated attribute][1], but "note" is the same
as the proposed "message":

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
class Deprecated
{
    public function __construct(
        public readonly ?string $note = null,
        public readonly ?string $since = null
    ) {
    }
}

#[Deprecated(since: "1.3", note: "this is not good, use good_pls_use")]
function bad_dont_use() {}

#[Deprecated("this wasn't meant to be public, use good_pls_use instead")
function oops_dont_use() {}

function good_pls_use() {}

In Rust, you get a message for each of ["since" and "note"][2]. In
PHP, this might look something like:

Deprecated: Function bad_dont_use() is deprecated since 1.3,
this wasn't meant to be public, use good_pls_use instead in %s
on line %d

  [1]: Diagnostics - The Rust Reference
  [2]: rfcs/text/1270-deprecation.md at master · rust-lang/rfcs · GitHub

[Benjamin_Aussenhofer]

On Tue, Apr 23, 2024 at 7:27 PM Levi Morrison <levi.morrison@datadoghq.com> wrote:

On Tue, Apr 23, 2024 at 7:30 AM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

Hi internals,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute
PR: https://github.com/php/php-src/pull/11293
Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

greetings
Benjamin

I skimmed through the previous discussion and didn’t see anything
about adding a since property. This is occasionally useful, at least
in my limited usage of it in Rust. The names below are modelled after
the names in Rust’s deprecated attribute, but “note” is the same
as the proposed “message”:

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
class Deprecated
{
public function __construct(
public readonly ?string $note = null,
public readonly ?string $since = null
) {
}
}

#[Deprecated(since: "1.3", note: "this is not good, use good_pls_use")]
function bad_dont_use() {}

#[Deprecated("this wasn't meant to be public, use good_pls_use instead")
function oops_dont_use() {}

function good_pls_use() {}

In Rust, you get a message for each of “since” and “note”. In
PHP, this might look something like:

Deprecated: Function bad_dont_use() is deprecated since 1.3,
this wasn’t meant to be public, use good_pls_use instead in %s
on line %d

This request is similar to Roman’s question about a replacement parameter elsewhere. We are unsure about these, because from an engine POV they do not add value, and from a user messaging perspective they could be put into the message with #[\Deprecated(“since 1.3, this is not good, use good_pls_use”)].

The only reason this might make sense is to allow third party tooling to work on this, but there are no conventions ala php-doc here in place already. If tools need more infos, they could just introduce their own attributes. example:

#[\Deprecated, Since(“1.3”), Replacement(“good_pls_use()”)].

It feels arbitrary if we add parameters that the engine does not use and where no tooling conventions exist on how they are being used, so we left them out for now.

[Crell]

On Wed, Apr 24, 2024, at 4:01 AM, Benjamin Außenhofer wrote:

On Tue, Apr 23, 2024 at 7:27 PM Levi Morrison
<levi.morrison@datadoghq.com> wrote:

On Tue, Apr 23, 2024 at 7:30 AM Benjamin Außenhofer <kontakt@beberlei.de> wrote:
>
> Hi internals,
>
> My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.
>
> Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.
>
> While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:
>
> RFC: PHP: rfc:deprecated_attribute
> PR: RFC: Add `#[\Deprecated]` Attribute by beberlei · Pull Request #11293 · php/php-src · GitHub
> Old discussion: #[Deprecated] Attribute - Externals
>
> Let me know about your questions and feedback.
>
> greetings
> Benjamin

I skimmed through the previous discussion and didn't see anything
about adding a `since` property. This is occasionally useful, at least
in my limited usage of it in Rust. The names below are modelled after
the names in [Rust's deprecated attribute][1], but "note" is the same
as the proposed "message":

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
class Deprecated
{
    public function __construct(
        public readonly ?string $note = null,
        public readonly ?string $since = null
    ) {
    }
}

#[Deprecated(since: "1.3", note: "this is not good, use good_pls_use")]
function bad_dont_use() {}

#[Deprecated("this wasn't meant to be public, use good_pls_use instead")
function oops_dont_use() {}

function good_pls_use() {}

In Rust, you get a message for each of ["since" and "note"][2]. In
PHP, this might look something like:

> Deprecated: Function bad_dont_use() is deprecated since 1.3,
> this wasn't meant to be public, use good_pls_use instead in %s
> on line %d

This request is similar to Roman's question about a replacement
parameter elsewhere. We are unsure about these, because from an engine
POV they do not add value, and from a user messaging perspective they
could be put into the message with #[\Deprecated("since 1.3, this is
not good, use good_pls_use")].

The only reason this might make sense is to allow third party tooling
to work on this, but there are no conventions ala php-doc here in place
already. If tools need more infos, they could just introduce their own
attributes. example:

#[\Deprecated, Since("1.3"), Replacement("good_pls_use()")].

It feels arbitrary if we add parameters that the engine does not use
and where no tooling conventions exist on how they are being used, so
we left them out for now.

I support this attribute, but would also like to see it fleshed out more. A since parameter or a separate attribute I'm flexible on and open to discussing; I don't have a strong preference right now. We should definitely see what other languages have found useful here beyond just Rust, which could give us a better idea of what is conventional/helpful.

My other question is whether E_USER_DEPRECATED is still the right error to throw, rather than E_DEPRECATED. I'm not saying it isn't, just that I'm unsure and would like to dig into that a bit more.

--Larry Garfield

[Lynn]

On Tue, Apr 23, 2024 at 3:30 PM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

Hi internals,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute

PR: https://github.com/php/php-src/pull/11293

Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

greetings
Benjamin

JetBrains was mentioned in the previous discussion but not sure if this was considered in the RFC: https://github.com/JetBrains/phpstorm-attributes/blob/master/src/Deprecated.php

[Benjamin_Aussenhofer]

On Wed, Apr 24, 2024 at 3:57 PM Nicolas Grekas <nicolas.grekas+php@gmail.com> wrote:

Hi Benjamin,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute

PR: https://github.com/php/php-src/pull/11293

Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

Thanks for the RFC.

While I’d like to be able to replace as many annotations as possible with attributes, the devil is in the details and here, I’m not sold about the details:

• I concur with others about the “since” property being missing

• We’d also need a “package” property so that it’s trivial to know which composer package is deprecating.
• The attribute class should not be final, so that packages could subclass, e.g. to define the “since” or “package” property once for all - or to define a custom constructor, etc.

As the RFC mentions, this is technically not possible, because the attribute is processed during compilation and an instanceof check at that point is not legal (cant autoload more during compiling).

Even considering we can solve this technically, it would also otherwise secretly mean that all method/function attributes are getting autoloaded and this would break the core assumption of attributes being backed by classes on Reflection access only.

• We should be able to add the attribute to a class.

This is in the future scope as it requires a very different (orthogonal) implementation.

Yes, the “package” + “since” info can be put in the message itself, but having a structured way to declare them is critical to 1. be sure that authors don’t forget to give that info and 2. build tools around that.

Are there tools around them? I can’t find a use case in my head what the “since” property can be used for.

Both since and package would be optional attributes, so tooling that checks they are not forgotten is needed anyways. It could just as well check for missing additional attributes next to the internal Deprecated attribute.

You’re saying that these are not useful because the engine wouldn’t make anything useful out of e.g. “since”.
That’s true, although I’d suggest using them to prefix the final message. If that’s the policy around attributes for the engine, then I’m wondering if the attribute shouldn’t live elsewhere. Or if we shouldn’t discuss this policy.

The attribute is there to close the gap with internal functions ZEND_ACC_DEPRECATED flag and ReflectionFunction/Method::isDeprecated already existing. This cannot live elsewhere.

The since, package (and replacement) requirements however could live elsewhere.

Given the many different requirements, we could think about adding an extra variadic argument, so that you could add whatever information you pass to the attributa via a key value array returned from Deprecated::getExtraInformation() ;

I also anticipate a problem with the adoption period of the attribute: in order to be sure that a call to a deprecated method will trigger a deprecation, authors will have to 1. add the attribute and 2. still call trigger_error when running on PHP < 8.next. That’s a lot of boilerplate.

This is a problem of any new feature that covers a use-case previously solved differently, for example attributes themselves, where libraries/Frameworks shipped both at the same time for a while. Taking this argument at face value would mean we stop evolving PHP.

Personally, I’m not convinced that there should be any runtime side-effects to the attribute. I’d prefer having it be just reported by reflection.

This RFC does not introduce the runtime side effects of ZEND_ACC_DEPRECATED flag on methods or functions. They already exist with internal functions. Having this attribute not trigger the deprecation where they are already triggered for internal functions introduces an inconsistency.

We should talk about how PHP implements deprecation tracking and logging as a separate issue, I have a lot of ideas here how to change that.

Nicolas

[Benjamin_Aussenhofer]

On Wed, Apr 24, 2024 at 2:55 PM Lynn <kjarli@gmail.com> wrote:

On Tue, Apr 23, 2024 at 3:30 PM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

Hi internals,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute

PR: https://github.com/php/php-src/pull/11293

Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

greetings
Benjamin

JetBrains was mentioned in the previous discussion but not sure if this was considered in the RFC: https://github.com/JetBrains/phpstorm-attributes/blob/master/src/Deprecated.php

Yes we considered it, but this is one implementation of a vendor, it might be conflicting with requirements of other projects and vendors. For the implementation and the engine itself this information is just “weight” that is carried around, so we decided not to add them, because its harder to decide on what to include than recommending tools to add their own supplemental attributes.

[Nicolas_Grekas]

Hi Benjamin,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute

PR: https://github.com/php/php-src/pull/11293

Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

Thanks for the RFC.

While I’d like to be able to replace as many annotations as possible with attributes, the devil is in the details and here, I’m not sold about the details:

• I concur with others about the “since” property being missing.
• We’d also need a “package” property so that it’s trivial to know which composer package is deprecating.
• The attribute class should not be final, so that packages could subclass, e.g. to define the “since” or “package” property once for all - or to define a custom constructor, etc.
• We should be able to add the attribute to a class.

Yes, the “package” + “since” info can be put in the message itself, but having a structured way to declare them is critical to 1. be sure that authors don’t forget to give that info and 2. build tools around that.

You’re saying that these are not useful because the engine wouldn’t make anything useful out of e.g. “since”.
That’s true, although I’d suggest using them to prefix the final message. If that’s the policy around attributes for the engine, then I’m wondering if the attribute shouldn’t live elsewhere. Or if we shouldn’t discuss this policy.

I also anticipate a problem with the adoption period of the attribute: in order to be sure that a call to a deprecated method will trigger a deprecation, authors will have to 1. add the attribute and 2. still call trigger_error when running on PHP < 8.next. That’s a lot of boilerplate.

Personally, I’m not convinced that there should be any runtime side-effects to the attribute. I’d prefer having it be just reported by reflection.

Nicolas

[Jorg_Sowa]

I like the proposition and I like the idea of $since parameter, however, this option is too ambiguous about what should store. Should it store the PHP version, package version, or the date?

What about setting this parameter vaguely as the boolean we can pass?

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
class Deprecated
{
public function __construct(
public readonly ?string $note = null,
public readonly ?bool $since = null
) {
}
}

#[Deprecated(since: $packageVersion > 5.5)]
#[Deprecated(since: PHP_VERSION_ID > 80100)]
#[Deprecated(since: date(“Y-m-d”) > “2024-01-21”)]

Kind regards,

Jorg

[Benjamin_Aussenhofer]

On Wed, Apr 24, 2024 at 7:18 PM Jorg Sowa <jorg.sowa@gmail.com> wrote:

I like the proposition and I like the idea of $since parameter, however, this option is too ambiguous about what should store. Should it store the PHP version, package version, or the date?

What about setting this parameter vaguely as the boolean we can pass?

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
class Deprecated
{
public function __construct(
public readonly ?string $note = null,
public readonly ?bool $since = null
) {
}
}

#[Deprecated(since: $packageVersion > 5.5)]
#[Deprecated(since: PHP_VERSION_ID > 80100)]
#[Deprecated(since: date(“Y-m-d”) > “2024-01-21”)]

These are not valid uses of attributes, the expressions for since parameter are illegal in the language.

PHP Fatal error: Constant expression contains invalid operations in /private/tmp/dep.php on line 6

And this proposal would only make the condition human readable, the machine would see $since as either true or false, and that is not really something that tools will find valuable enough.

But I agree on $since being less useful, users could put a date, a string, or anyhing in there.

The way I see it vendors should add their own attributes, example:

#[\Deprecated, Since(package: “symfony/http-request”, version: “7.0.2”)]

or for PHPStorms replacement functinoality (potentially usable by Rector):

#Deprecated, Replacement(‘%class%->setPublic(!%parameter0%)’)]

This is
a.) composable, which increases the potential use-cases for different vendor and communities and doesn’t restrict them to what core “designed in the ivory tower once”.
b.) allows vendors to have very strict validation on their attributes arguments. I.e. Symfony could enforce package to be a valid composer package string and version to follow composers version parser. This is something we cannot enforce in the engine.

Kind regards,

Jorg

[Rowan_Tommins_IMSoP]

On 24 April 2024 18:18:28 BST, Jorg Sowa <jorg.sowa@gmail.com> wrote:

What about setting this parameter vaguely as the boolean we can pass?
...
#[Deprecated(since: $packageVersion > 5.5)]
#[Deprecated(since: PHP_VERSION_ID > 80100)]
#[Deprecated(since: date("Y-m-d") > "2024-01-21")]

Even if these expressions were legal, as far as I know, standard reflection doesn't give any access to the source code or AST of how the attribute was written, so this would just end up with a meaningless "$since = true", and some source code that might as well be a comment.

To be honest, I'm not really sure what I'd do with the information in a "since" field even if it was there. If you were running PHP 7.4, what difference would it make to know that create_function was deprecated in 7.2, rather than in 7.1 or 7.3? The two relevant facts are when the suggested replacement was introduced (in case you need to support multiple versions); and what is the soonest that the deprecated feature will be removed. The second in particular is something I would like every deprecation message to include, rather than the vague "may be removed in a future version".

I found this discussion of "since" in Rust's implementation, but don't find the arguments in favour particularly compelling: RFC: #[deprecated] for Everyone by llogiq · Pull Request #1270 · rust-lang/rfcs · GitHub

Of interest, that discussion also linked to a related feature in Java, which could perhaps be added to a list in the RFC alongside the Rust and JetBrains ones already mentioned: JEP 277: Enhanced Deprecation

It's interesting to note, for instance, that both Java and Rust designers considered a specific "replacement" field, but decided that it was unlikely to be useful in practice. The Java proposal states this nicely:

In practice, there is never a drop-in replacement API for
any deprecated API; there are always tradeoffs and
design considerations, or choices to be made among
several possible replacements. All of these topics require
discussion and are thus better suited for textual
documentation.

The JetBrains attribute *does* include a "replacement" argument, but it's heavily tied into a specific use case: it contains a template used for code transformation in the IDE. Both it and "since" are explicitly marked "applicable only for PhpStorm stubs".

Regards,
Rowan Tommins
[IMSoP]

[Stephen_Reay]

Sent from my iPhone

On 25 Apr 2024, at 16:26, Rowan Tommins [IMSoP] imsop.php@rwec.co.uk wrote:

On 24 April 2024 18:18:28 BST, Jorg Sowa jorg.sowa@gmail.com wrote:

What about setting this parameter vaguely as the boolean we can pass?

…

#[Deprecated(since: $packageVersion > 5.5)]

#[Deprecated(since: PHP_VERSION_ID > 80100)]

#[Deprecated(since: date(“Y-m-d”) > “2024-01-21”)]

Even if these expressions were legal, as far as I know, standard reflection doesn’t give any access to the source code or AST of how the attribute was written, so this would just end up with a meaningless “$since = true”, and some source code that might as well be a comment.

To be honest, I’m not really sure what I’d do with the information in a “since” field even if it was there. If you were running PHP 7.4, what difference would it make to know that create_function was deprecated in 7.2, rather than in 7.1 or 7.3? The two relevant facts are when the suggested replacement was introduced (in case you need to support multiple versions); and what is the soonest that the deprecated feature will be removed. The second in particular is something I would like every deprecation message to include, rather than the vague “may be removed in a future version”.

I found this discussion of “since” in Rust’s implementation, but don’t find the arguments in favour particularly compelling: RFC: #[deprecated] for Everyone by llogiq · Pull Request #1270 · rust-lang/rfcs · GitHub

Of interest, that discussion also linked to a related feature in Java, which could perhaps be added to a list in the RFC alongside the Rust and JetBrains ones already mentioned: JEP 277: Enhanced Deprecation

It’s interesting to note, for instance, that both Java and Rust designers considered a specific “replacement” field, but decided that it was unlikely to be useful in practice. The Java proposal states this nicely:

In practice, there is never a drop-in replacement API for

any deprecated API; there are always tradeoffs and

design considerations, or choices to be made among

several possible replacements. All of these topics require

discussion and are thus better suited for textual

documentation.

The JetBrains attribute does include a “replacement” argument, but it’s heavily tied into a specific use case: it contains a template used for code transformation in the IDE. Both it and “since” are explicitly marked “applicable only for PhpStorm stubs”.

Regards,
Rowan Tommins
[IMSoP]

I think it’s worth pointing out that phpdoc has had a @deprecated [version] [description] attribute for a long time - since is, IMO a different name for “version” and indicates the version that element was deprecated. If you’re on X.y and it says it was deprecated in X.w you know you don’t need to worry about it being removed until at least Y.a.

Also, phpdoc also has a separate @since attribute, which documents the introduction or modification of an element.

I think suggestions of a “Since” attribute “beside” the deprecated attribute defeat the purpose of it being an attribute, on top of being confusing when compared to existing phpdoc attributes.

#[Deprecated, Since(1.9)] to me, means “currently deprecated; introduced (or significantly changed) in 1.9”

To mean what you’re suggesting it would need to be
#[Deprecated, Since(1.9, ‘Deprecated in favour of foo…’)] which is just needless duplication.

For those who don’t see the point of since, I think the obvious answer is: documentation. Rather than needing a separate docblock to detail the deprecation, it can be all in one, just as parameter types and return types are. This reduces duplication, and reduces possibility of mismatched values.

If you wanted it to be clearer I’d suggest maybe rename “since” to “version”, but that’s more to give a hint at intended use than anything.

[Rowan_Tommins_IMSoP]

On 25/04/2024 08:40, Stephen Reay wrote:

If you're on X.y and it says it was deprecated in X.w you know you don't need to worry about it being removed until at least Y.a.

Yeah, that's the reasoning given in the Rust discussion, but I don't find it convincing.

If the project's deprecation policy is that deprecations will be removed in the next major version, the information is redundant: if you get the deprecation message in 2.x, you know it will be removed in 3.0

If the project has some other deprecation policy, like "after 1 full major version cycle", then you can work out that "since: 2.3" means removal in 4.0; but the person adding the attribute also knows that, and could save the reader some effort by writing "planned removal: 4.0"

If the project has no clear deprecation policy, the information is useless anyway.

If you wanted it to be clearer I'd suggest maybe rename "since" to "version", but that's more to give a hint at intended use than anything.

I don't think there's anything *unclear* about "since", I just don't think it's very *useful*. But apparently it's common to write it, so I guess I'm in the minority.

Naming it "version" would just make it less clear, and not resolve anything from my point of view.

Regards,

--
Rowan Tommins
[IMSoP]

[Mike_Schinkel]

On Apr 25, 2024, at 11:28 AM, Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:

If the project has no clear deprecation policy, the information is useless anyway.

Not true.

Having standardized notation for deprecation would allow tooling to analyze a codebase and determine if it contains deprecated code that needs to be remediated without having to run the code with full coverage.

Actually, since: doesn’t go far enough because it doesn’t indicate if the deprecation throws warnings, throws errors, or just no longer works. The latter two would be especially helpful when deciding if code modifications are necessary before upgrading to a new version of PHP.

-Mike

[Rowan_Tommins_IMSoP]

On 25 April 2024 22:01:35 BST, Mike Schinkel <mike@newclarity.net> wrote:

On Apr 25, 2024, at 11:28 AM, Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:
If the project has no clear deprecation policy, the information is useless anyway.

Not true.

Having standardized notation for deprecation would allow tooling to analyze a codebase and determine if it contains deprecated code that needs to be remediated without having to run the code with full coverage.

I think you missed the context of that sentence - or I'm missing something in yours. I meant specifically that the "deprecated since" information is useless if there's no published policy on how long something will stay deprecated.

I think the "deprecated" attribute itself is definitely useful.

Regards,
Rowan Tommins
[IMSoP]

[Benjamin_Aussenhofer]

On Tue, Apr 23, 2024 at 7:27 PM Levi Morrison <levi.morrison@datadoghq.com> wrote:

On Tue, Apr 23, 2024 at 7:30 AM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

Hi internals,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute
PR: https://github.com/php/php-src/pull/11293
Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

greetings
Benjamin

I skimmed through the previous discussion and didn’t see anything
about adding a since property. This is occasionally useful, at least
in my limited usage of it in Rust. The names below are modelled after
the names in Rust’s deprecated attribute, but “note” is the same
as the proposed “message”:

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
class Deprecated
{
public function __construct(
public readonly ?string $note = null,
public readonly ?string $since = null
) {
}
}

#[Deprecated(since: "1.3", note: "this is not good, use good_pls_use")]
function bad_dont_use() {}

#[Deprecated("this wasn't meant to be public, use good_pls_use instead")
function oops_dont_use() {}

function good_pls_use() {}

In Rust, you get a message for each of “since” and “note”. In
PHP, this might look something like:

Deprecated: Function bad_dont_use() is deprecated since 1.3,
this wasn’t meant to be public, use good_pls_use instead in %s
on line %d

After discussing with Mate shortly one reason for adding $since from a PHP project POV is that we do show the $since information in the generated documentation output.

Integrating with the work in progress to auto generate parts of the function docs based on the stub files, having the $since attribute on the stubs would allow to use this as the central information in code. Therefore we would reconsider and add the $since argument to the Deprecated class.

I am still partly on the side on Rowan (https://externals.io/message/123184#123206) that i don’t find the arguments for this parameter very convincing but at least there is a use-case internally now that warrants adding it.

[Mike_Schinkel]

On Apr 25, 2024, at 5:47 PM, Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:

I think you missed the context of that sentence - or I’m missing something in yours. I meant specifically that the “deprecated since” information is useless if there’s no published policy on how long something will stay deprecated.

To paraphrase a former US President, “Depends on what the meaning of ‘since’ is.”

Given a lack of agreed definition for ‘since’ it appears you are using narrow assumptions about the meaning of ‘since’ that led you to view ‘since’ as useless.

Conversely, I try not to make limiting assumptions and look for how a feature could be useful IF defined appropriately. And to me there is benefit to have a standardized way for tooling to recognize deprecation based on versions and other criteria, even if the specifics may need to be explored further.

Or at least that is how I see it. But then, I have no vote, so ¯_(ツ)_/¯.

-Mike

[Rowan_Tommins_IMSoP]

On 26 April 2024 09:40:57 BST, Mike Schinkel <mike@newclarity.net> wrote:

Given a lack of agreed definition for 'since' it appears you are using narrow assumptions about the meaning of 'since' that led you to view 'since' as useless.

I can't see any ambiguity in the definition: "This function has been deprecated since version 7.2" seems a straightforward English sentence, meaning that before 7.2 it wasn't deprecated, and from that version onward it is.

If there's some alternative reading of it, it's not that I'm assuming it doesn't apply, it's that I'm completely unaware of what it might be.

Regards,
Rowan Tommins
[IMSoP]

[Alex_Wells]

On Fri, Apr 26, 2024 at 11:01 AM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

After discussing with Mate shortly one reason for adding $since from a PHP project POV is that we do show the $since information in the generated documentation output.

Integrating with the work in progress to auto generate parts of the function docs based on the stub files, having the $since attribute on the stubs would allow to use this as the central information in code. Therefore we would reconsider and add the $since argument to the Deprecated class.

I am still partly on the side on Rowan (https://externals.io/message/123184#123206) that i don’t find the arguments for this parameter very convincing but at least there is a use-case internally now that warrants adding it.

Hey. Before including the since parameter, please consider this:

Let’s assume the usual (or at least, the most common) development flow includes creating a separate branch where development happens before it’s merged into the default branch. With that in mind, let’s go through the flow:

• tag 2.1.1 exists at some ref
• the master branch is ahead of tag 2.1.1 and contains some unreleased, untagged changes
• a new branch deprecate-something is created from master
• a function is deprecated with the attribute: #[Deprecated(since: '2.1.1')]
• a pull request is created from deprecate-something into master, but isn’t merged yet
• a new version is tagged and released - either 2.1.2, 2.2.0 or 3.0.0

At this point deprecate-something contains an incorrectly specified since: '2.1.1' which needs to be updated by hand, or require additional tooling on top to update it automatically.

Updating it by hand is tedious and may be needed to be done multiple times before the PR is merged. It’s also not error-prone, as you may make a typo or just forget about the deprecation. This is also why Composer parses package version from the Git tags and advocates against specifying it manually: https://getcomposer.org/doc/04-schema.md#version

[Benjamin_Aussenhofer]

On Tue, Apr 23, 2024 at 3:27 PM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

Hi internals,

My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work.

Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation.

While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4:

RFC: https://wiki.php.net/rfc/deprecated_attribute

PR: https://github.com/php/php-src/pull/11293

Old discussion: https://externals.io/message/112554#112554

Let me know about your questions and feedback.

greetings
Benjamin

We have updated the RFC and PR to include #[Deprecated] on class constants and enum cases as this is something the engine already supports for internal class constants.

This does not include support for non-class based constants, because they don’t have support for attributes at the moment and also only recently got reflection support (for 8.4).

We are planning to work on adding $since next and get back to the list once that is finished.