Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127891
Precedence: bulk
MIME-Version: 1.0
References: <1cfc477a-781f-40e9-9e37-dd748ef261be@app.fastmail.com>
 <20DACB49-3C84-4B23-98E2-35050D9EDAC9@koalephant.com> <CAB-BujY3vLBp7SVNXgUGC8_TWUKiCWnbyUJ7yXSjtK=yFdLGow@mail.gmail.com>
 <3C4ED1AE-3E25-461D-8C3C-AF954499F905@koalephant.com> <CAB-BujY_omX9HWoXv+H0UxOpvvWdtOjwvgV2YV5g4iun7VqpRA@mail.gmail.com>
 <4839a478-9b65-4ac5-8444-ca286ddc72fd@app.fastmail.com>
In-Reply-To: <4839a478-9b65-4ac5-8444-ca286ddc72fd@app.fastmail.com>
Date: Sat, 5 Jul 2025 00:47:50 +0200
Message-ID: <CAB-BujaBWS3GYuWRo9HUQUuE6LorzfY0OoQxRSYpM0p92j0qqw@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC idea] Target-aware attributes
To: Rob Landers <rob@bottled.codes>
Cc: Stephen Reay <php-lists@koalephant.com>, Larry Garfield <larry@garfieldtech.com>, 
	php internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: andreas@dqxtech.net (Andreas Hennings)

On Sat, 5 Jul 2025 at 00:11, Rob Landers <rob@bottled.codes> wrote:
>
>
>
> On Fri, Jul 4, 2025, at 17:21, Andreas Hennings wrote:
>
> On Fri, 4 Jul 2025 at 06:30, Stephen Reay <php-lists@koalephant.com> wrot=
e:
> >
> >
> >
> > On 4 Jul 2025, at 00:54, Andreas Hennings <andreas@dqxtech.net> wrote:
> >
> > On Thu, 3 Jul 2025 at 19:17, Stephen Reay <php-lists@koalephant.com> wr=
ote:
> >
> >
> >
> >
> >
> > Sent from my iPhone
> >
> > On 3 Jul 2025, at 23:40, Larry Garfield <larry@garfieldtech.com> wrote:
> >
> > =EF=BB=BFOn Wed, Jul 2, 2025, at 5:26 PM, Andreas Hennings wrote:
> >
> > This topic was discussed in the past as "Declaration-aware
> > attributes", and mentioned in the discussion to "Amendments to
> > Attributes".
> > I now want to propose a close-to-RFC iteration of this.
> > (I don't have RFC Karma, my wiki account is "Andreas Hennings (donquixo=
te)")
> >
> > -----
> >
> > Primary proposal
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >
> > I propose to introduce 3 new methods on ReflectionAttribute.
> >
> > static ReflectionAttribute::getCurrentTargetReflector(): ?Reflector
> > Most of the time, this will return NULL.
> > During the execution of ReflectionAttribute->newInstance(), it will
> > return the reflector of the symbol on which the attribute is found.
> > (in other words, during
> > $reflector->getAttributes()[$i]->newInstance(), it will return
> > $reflector.)
> > During the execution of
> > ReflectionAttribute::invokeWithTargetAttribute($target, $callback), it
> > will return $target.
> > If the call stack contains multiple calls to the above mentioned
> > methods, only the closest/deepest one counts.
> > (This means that php needs to maintain a stack of reflectors.)
> >
> >
> > *snip*
> >
> > Other alternatives
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >
> > In older discussions, it was suggested to provide the target reflector
> > as a special constructor parameter.
> > This is problematic because an attribute expression #[MyAttribute('a',
> > 'b', 'c')] expects to pass values to all the parameters.
> >
> > Another idea was to provide the target reflector through a kind of
> > setter method on the attribute class.
> > This can work, but it makes attribute classes harder to write, because
> > the constructor does not have all the information.
> > It may also prevent attribute classes from being stateless (depending
> > how we define stateless).
> >
> >
> > Userland implementations
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D
> >
> > One userland implementation that was mentioned in this list in the
> > past is in the 'crell/attributeutils' package.
> > This one uses a kind of setter injection for the target reflector.
> > See
> > https://github.com/Crell/AttributeUtils/blob/master/src/FromReflectionC=
lass.php
> >
> >
> > Hey, I know that guy! :-)
> >
> > Another userland implementation is in the
> > 'ock/reflector-aware-attributes' package.
> > https://github.com/ock-php/reflector-aware-attributes (I created that o=
ne)
> > This supports both a setter method and getting the target reflector
> > from the attribute constructor.
> >
> > The problem with any userland implementation is that it only works if
> > the attribute is instantiated (or processed) using that userland
> > library.
> > Simply calling $reflector->getAttributes()[0]->newInstance() would
> > either return an instance that is incomplete, or it would break, if
> > the attribute class expects access to its target.
> >
> >
> > I am unsurprisingly in favor of finding a solution here, as there are i=
nnumerable cases where you need the reflectable that the attribute is on; t=
he most common for me is using the name/type of a property as defaults for =
the attribute.
> >
> > However, I am very skeptical about a stateful global value as the solut=
ion.  We've tried very hard to remove those from PHP, mostly successfully. =
 Adding another one back in feels like a major step backwards, and a great =
place for weird bugs to hide.
> >
> > A setter method injection is what I did in AttributeUtils, because it w=
as the only real option.  Alternatively, I suppose core could use property =
setter injection (either a magically named property like $__reflector, or a=
 property that itself has an attribute on it, etc.).  That would allow it t=
o be set before the constructor is called, and with property hooks would al=
low processing either immediately or later in the constructor.  The downsid=
e here is that Attribute are, generally, serializable, but a Reflection obj=
ect is not.  So if someone wanted a serializable attribute they would have =
to accept the property, use it, and then remember to unset it at some point=
.  That's clumsy.
> >
> > --Larry Garfield
> >
> >
> > As someone that's written yet another userland "solution" for this prob=
lem, I have an alternative solution, based somewhat on an internalised  con=
cept of "never store Reflectors".
> >
> > Introduce an interface "ReflectorAttribute" (bike shedding to come); wh=
ich accepts a single Reflector argument.
> >
> > If the attribute implements the interface, the method is called immedia=
tely following instantiation.
> >
> >
> > Yep, this is the "method injection" mentioned by Larry, or what I
> > referred to as "setter injection".
> > I have not seen your library, but I assume that's where it is going.
> >
> >
> > Hi Andreas,
> >
> > I guess the key difference I wanted to highlight is that the existing d=
iscussion keeps referencing it as a "setter" - which I think from user land=
 at least will generally be understood to mean setting a property on an obj=
ect - which then adapted into Larry's mention of specifically setting a pro=
perty before the constructor is run.
>
> I guess the main reason it "keeps" doing that is that an email is not
> a wiki page that could be updated :)
> I am happy to use different terminology in an RFC.
> (with the current plan, it would only be mentioned under "alternatives
> that were considered")
> "method injection" seems fine.
>
> But then how would we name such a method?
> In Larry's library it is ->fromReflection(..), but this is something I
> would typically use for static factories.
> To me, ->setReflector() is still ok even if internally it is not a
> setter. The method describes the contract, not what happens inside.
> Another idea would be ->tellAboutReflector() or maybe ->injectTargetRefle=
ctor()?
>
> >
> > I think it's a bad idea to reference this concept as "setting a propert=
y" - my understanding is that it's never a good idea to hang onto Reflector=
 objects longer than absolutely necessary, so I don't think this feature sh=
ould then result in people doing that due to the impression it gives (i.e. =
if it was referred to as "setReflection<subtype>()" or if the description f=
or it is "allows an Attribute to store the Reflector target it was declared=
 on" etc)
> >
>
>
> --------
>
> Now, to resolve the controversial part of this discussion.
>
> We could reduce the RFC to the uncontroversial part:
> Provide a ReflectionAttribute->getTargetReflector().
>
> With this, the rest of the proposal can be implemented in userland
> using debug_backtrace().
> https://3v4l.org/Ilrqm#vnull
>
> #[Attribute]
> class MyAttribute {
>     public function __construct() {
>         $target_reflector =3D
> debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS |
> DEBUG_BACKTRACE_PROVIDE_OBJECT, 3)[1]['object'];
>     }
> }
>
> Yes it feels dirty, but now it is now longer something we have to
> argue about in this list :)
>
>
> Why do we have to call a constructor before we initialize a property? htt=
ps://3v4l.org/srdM6
>
> You can do this in regular php, but it is even simpler in the engine.
>
> =E2=80=94 Rob

This is interesting, but I still add some logistical clutter to the
attribute class.
It does bring some advantage over post-construction method injection,
because you can do the logistics in a trait, and then in the
constructor you have all the information available.
But, even if that part is hidden away in the engine, now we have a
property that we probably want to unset later.

-- Andreas