Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:123201 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by qa.php.net (Postfix) with ESMTPS id C12AF1A009C for ; Wed, 24 Apr 2024 14:53:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1713970439; bh=78gA8myH2IYt4BLfOn1+JXl8bikjcNBOqvxipZWovmo=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=n6rWluRsTpaC7MrQc0Uj8Qg48Kf3/QjgUk4K8+WDSS9O13urWJ50W7XSPP31o4JWP 8fzJnUORWHXADPnvgYhmGXCiLYz8F/vA1Sqk/OWxH4xtGzt8bb3nHBjRjhBsfv46U6 GL/cCzULRHjaYozLP8sKyMtMbSIe//bxUSLTrzzdT824XQNtAJTQocoTZ7JLtlnGiv wHhdaH01rIe1WUgHgdVTIR5zzltyBB0/LDZU6ZcJEyEtm5ezvkF7krRhg5oLgtWzp6 9vg97QFiEVHrnKBRcDW3Vnne3aw5Br0p7L73kM0UBFRF/kjvpO7N49yxsaDSmtUBfo IxzXstY1JP4Ig== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id C51FC180079 for ; Wed, 24 Apr 2024 14:53:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DMARC_MISSING,HTML_MESSAGE,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from mail-ua1-f46.google.com (mail-ua1-f46.google.com [209.85.222.46]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 24 Apr 2024 14:53:57 +0000 (UTC) Received: by mail-ua1-f46.google.com with SMTP id a1e0cc1a2514c-7e618c51802so1848812241.0 for ; Wed, 24 Apr 2024 07:53:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=beberlei-de.20230601.gappssmtp.com; s=20230601; t=1713970396; x=1714575196; darn=lists.php.net; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=/fL/l1HBCr8BU9uCR5vaX//lsA+g83hL124ca+6e7Vc=; b=s1aeZr9DE/KWmXTxFF9PZSqSLMMUVE3QbYJxHMZN3f8mep879bdGDjSo4TkOnhi35Z aQOmHRjkbRXmiO8u4wrUYhoKABdsuLIr1D25ttYMumyzQyfiRLwg+LmUecgMZj0Bxa++ 2FgwcttVejKMLsdqCoPRVw0q5qsxx0BYO+VgXlymJmnqTeHezgdYD+IOnUHXIOugA4XW hvrip4bhgEZA599lom6gzhIYjiSLjMiYO3idybzbhW68b6STWfEUen033O7i1gumD3lj LO7Cvqyc6+zw+xDmCoER6Lz4dHTgyNhUN9PxmWzvv+f4aHVSU0YBMhKAR2OUFMxwLeBz fWgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713970396; x=1714575196; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=/fL/l1HBCr8BU9uCR5vaX//lsA+g83hL124ca+6e7Vc=; b=A5WqjcxakzZQlOqFVuXdDFGImjFIusXaRYoayyYk/MHDDEeTdMgyv6nC6JBISLniOg LXA6s3/rOWpdGuFimAum92tGitmCzvB4i9DEtdHn42ysA7pIkjr5JY6jHA4XTkz7tWw1 +29WJ4hAlUow38+MGFAlHqSO4/+4hy/BzVgsBq0MRcP3/nR3mQwE34iyTHf2VJpyZJjC IKioZ61Mrgq5JPLYG597S3osjavD7Miq/l6Hx47A+1Ps7vL0IrZeBJ7K9tdh7fjH0NLW yrZI9W2MABNm74TA/dDJkMNqPTvmeFlWOQz6FG+u9qDeb76bodowtk1wFt8NdrCns1Rq uEmA== X-Gm-Message-State: AOJu0YymYgYQfXg7yd+hx2wxbGRW0QLvZvI1Zm1cD3rOihGufOe5szTl jubK6UKFM7jDOTM9Np3T4ejeowlbzqGLF4TkrMuiHjm+Bqa2HRziAZhVFjugEytDI5xPSEIMMSp wF+jbT8baA/h2+LFQXYxgSngy/i1pGQpyzYPF9g== X-Google-Smtp-Source: AGHT+IF0OacuPu6CYqX0ORiHhChHM14DclagT0hjbIlVce+oRzTrvj8e/aU/d4PaBbicGS/YgQYQ3iYKooG63Ce6Z84= X-Received: by 2002:a05:6102:34cd:b0:47a:2ff9:d059 with SMTP id a13-20020a05610234cd00b0047a2ff9d059mr3127389vst.21.1713970394550; Wed, 24 Apr 2024 07:53:14 -0700 (PDT) Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net MIME-Version: 1.0 References: In-Reply-To: Date: Wed, 24 Apr 2024 16:53:02 +0200 Message-ID: Subject: Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3 To: Nicolas Grekas Cc: PHP Internals , "tim@tideways-gmbh.com" Content-Type: multipart/alternative; boundary="00000000000085f61c0616d8d481" From: kontakt@beberlei.de (=?UTF-8?Q?Benjamin_Au=C3=9Fenhofer?=) --00000000000085f61c0616d8d481 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Apr 24, 2024 at 3:57=E2=80=AFPM Nicolas Grekas wrote: > Hi Benjamin, > > My PR for #[\Deprecated] attribute was in hibernation for a long while no= w >> 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 al= l - > 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 thi= s > 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 > > > > > --00000000000085f61c0616d8d481 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Wed, Apr 24, 2024 at 3:57=E2=80=AF= PM Nicolas Grekas <nic= olas.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 ad= ditional 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:

<= br>
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 wi= th attributes, the devil is in the details and here, I'm not sold about= the details:
  • I concur with others about the "since&= quot; property being missing
=
  • We'd also need a "package" property so that i= t'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 al= l - 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 com= piling).=C2=A0

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.
=C2=A0
  • We should be able to add the attribute to a c= lass.
This is in the= future scope as it requires a very different (orthogonal) implementation.<= /div>
Yes, the "package" + "s= ince" info can be put in the message itself, but having a structured w= ay to declare them is critical to 1. be sure that authors don't forget = to give that info and 2. build tools around that.
=C2=A0
Are there tools around them? I can&#= 39;t find a use case in my head what the "since" property can be = used for.=C2=A0

Both since and package would be op= tional 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 ma= ke anything useful out of e.g. "since".
That's true= , although I'd suggest using them to prefix the final message. If that&= #39;s the policy around attributes for the engine, then I'm wondering i= f the attribute shouldn't live elsewhere. Or if we shouldn't discus= s this policy.

The attribute is there to close the gap with internal functions ZEND_ACC_= DEPRECATED flag and ReflectionFunction/Method::isDeprecated already existin= g. This cannot live elsewhere.=C2=A0

The since, pa= ckage (and replacement) requirements however could live elsewhere.

Given the many different requirements, we could think abou= t adding an extra variadic argument, so that you could add whatever informa= tion you pass to the attributa via a key value array returned from Deprecat= ed::getExtraInformation() ;


I also anticipate a problem with the adoption period of the attribute: i= n order to be sure that a call to a deprecated method will trigger a deprec= ation, 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 proble= m 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.
=C2=A0

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 refl= ection.

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

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


Nicolas



=C2=A0
--00000000000085f61c0616d8d481--