Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127879
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>
In-Reply-To: <3C4ED1AE-3E25-461D-8C3C-AF954499F905@koalephant.com>
Date: Fri, 4 Jul 2025 17:21:43 +0200
Message-ID: <CAB-BujY_omX9HWoXv+H0UxOpvvWdtOjwvgV2YV5g4iun7VqpRA@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC idea] Target-aware attributes
To: Stephen Reay <php-lists@koalephant.com>
Cc: 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 Fri, 4 Jul 2025 at 06:30, Stephen Reay <php-lists@koalephant.com> wrote:
>
>
>
> 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> wrot=
e:
>
>
>
>
>
> 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 (donquixote=
)")
>
> -----
>
> 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/FromReflectionCla=
ss.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 one=
)
> 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 inn=
umerable cases where you need the reflectable that the attribute is on; the=
 most common for me is using the name/type of a property as defaults for th=
e attribute.
>
> However, I am very skeptical about a stateful global value as the solutio=
n.  We've tried very hard to remove those from PHP, mostly successfully.  A=
dding another one back in feels like a major step backwards, and a great pl=
ace for weird bugs to hide.
>
> A setter method injection is what I did in AttributeUtils, because it was=
 the only real option.  Alternatively, I suppose core could use property se=
tter injection (either a magically named property like $__reflector, or a p=
roperty that itself has an attribute on it, etc.).  That would allow it to =
be set before the constructor is called, and with property hooks would allo=
w processing either immediately or later in the constructor.  The downside =
here is that Attribute are, generally, serializable, but a Reflection objec=
t 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 proble=
m, I have an alternative solution, based somewhat on an internalised  conce=
pt of "never store Reflectors".
>
> Introduce an interface "ReflectorAttribute" (bike shedding to come); whic=
h accepts a single Reflector argument.
>
> If the attribute implements the interface, the method is called immediate=
ly 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 dis=
cussion keeps referencing it as a "setter" - which I think from user land a=
t least will generally be understood to mean setting a property on an objec=
t - which then adapted into Larry's mention of specifically setting a prope=
rty 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 ->injectTargetReflect=
or()?

>
> I think it's a bad idea to reference this concept as "setting a property"=
 - my understanding is that it's never a good idea to hang onto Reflector o=
bjects longer than absolutely necessary, so I don't think this feature shou=
ld 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 for=
 it is "allows an Attribute to store the Reflector target it was declared o=
n" 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 :)