Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123200
Precedence: bulk
MIME-Version: 1.0
References: <CAEPJdumvJNQ+JZ3wkLP-Q2O0y57g=W_hHFkuvOKMVJqqraKCzw@mail.gmail.com>
In-Reply-To: <CAEPJdumvJNQ+JZ3wkLP-Q2O0y57g=W_hHFkuvOKMVJqqraKCzw@mail.gmail.com>
Date: Wed, 24 Apr 2024 15:57:32 +0200
Message-ID: <CAOWwgpkQ9XkBO-W_BL4P1fSBJdi44cCxjMhLWBp7ROsVZGa4=A@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3
To: =?UTF-8?Q?Benjamin_Au=C3=9Fenhofer?= <kontakt@beberlei.de>
Cc: PHP Internals <internals@lists.php.net>, 
	"tim@tideways-gmbh.com" <tim@tideways-gmbh.com>
Content-Type: multipart/alternative; boundary="000000000000001a240616d80e7c"
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--000000000000001a240616d80e7c
Content-Type: text/plain; charset="UTF-8"

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

--000000000000001a240616d80e7c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Benjamin,<br><div class=3D"gmail_quote=
"><div dir=3D"ltr" class=3D"gmail_attr"><br></div><blockquote class=3D"gmai=
l_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,20=
4,204);padding-left:1ex"><div dir=3D"ltr"><div></div><div>My PR for #[\Depr=
ecated] attribute was in hibernation for a long while now and after some of=
f-list discussion a few weeks ago I have decided to revisit it and asked Ti=
m to help me out with the work.</div><div><br></div><div>Tim has cleaned up=
 the PR quite a bit and also worked in additional features such as #[Deprec=
ated] support in stub generation.</div><div><br></div><div>While there are =
still some small todos, at this point we want to restart the discussion abo=
ut the RFC for inclusion in 8.4:</div><div><br></div><div>RFC: <a href=3D"h=
ttps://wiki.php.net/rfc/deprecated_attribute" target=3D"_blank">https://wik=
i.php.net/rfc/deprecated_attribute</a><br></div><div>PR: <a href=3D"https:/=
/github.com/php/php-src/pull/11293" target=3D"_blank">https://github.com/ph=
p/php-src/pull/11293</a><br></div><div>Old discussion:=C2=A0<a href=3D"http=
s://externals.io/message/112554#112554" target=3D"_blank">https://externals=
.io/message/112554#112554</a></div><div><br></div><div>Let me know about yo=
ur questions and feedback.</div></div></blockquote><div><br></div><div>Than=
ks for the RFC.</div><div><br></div><div>While I&#39;d like to be able to r=
eplace as many annotations as possible with attributes, the devil is in the=
 details and here, I&#39;m not sold about the details:</div><div><ul><li>I =
concur with others about the &quot;since&quot; property being missing.</li>=
<li>We&#39;d also need a &quot;package&quot; property so that it&#39;s triv=
ial to know which composer package is deprecating.</li><li>The attribute cl=
ass should not be final, so that packages could subclass, e.g. to define th=
e &quot;since&quot; or &quot;package&quot; property once for all - or to de=
fine a custom constructor, etc.</li><li>We should be able to add the attrib=
ute to a class.<br></li></ul><div>Yes,  the &quot;package&quot; + &quot;sin=
ce&quot; 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&#39;t forget to=
 give that info and 2. build tools around that.</div><div><br></div><div>Yo=
u&#39;re saying that these are not useful because the engine wouldn&#39;t m=
ake anything useful out of e.g. &quot;since&quot;.</div><div>That&#39;s tru=
e, although I&#39;d suggest using them to prefix the final message. If that=
&#39;s the policy around attributes for the engine, then I&#39;m wondering =
if the attribute shouldn&#39;t live elsewhere. Or if we shouldn&#39;t discu=
ss this policy.</div><div><br></div><div>I also anticipate a problem with t=
he adoption period of the attribute: in order to be sure that a call to a d=
eprecated method will trigger a deprecation, authors will have to 1. add th=
e attribute and 2. still call trigger_error when running on PHP &lt; 8.next=
. That&#39;s a lot of boilerplate.</div><div><br></div><div>Personally, I&#=
39;m not convinced that there should be any runtime side-effects to the att=
ribute. I&#39;d prefer having it be just reported by reflection.</div><div>=
<br></div><div>Nicolas<br></div><div><br></div><div><br></div><div><br></di=
v></div><div>=C2=A0</div></div></div>
</div>

--000000000000001a240616d80e7c--