Re: [RFC idea] Target-aware attributes

> 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 <NZÎo¬ > ”¢´0:KÞmailto:php-lists@koalephant.com>> wrote: >> >> >> >> >> Sent from my iPhone >>> On 3 Jul 2025, at 23:40, Larry Garfield <larry@garfieldtech.com> wrote: >>> >>> On 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 >>>> ============= >>>> >>>> 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 >>>> ====================== >>>> >>>> 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 >>>> ========================= >>>> >>>> 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/FromReflectionClass.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 innumerable 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 the attribute. >>> >>> However, I am very skeptical about a stateful global value as the solution. 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 was 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 to be set before the constructor is called, and with property hooks would allow processing >>> either immediately or later in the constructor. The downside here is that Attribute are, generally, >>> serializable, but a Reflection object 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 problem, >> I have an alternative solution, based somewhat on an internalised concept of "never store >> Reflectors". >> >> Introduce an interface "ReflectorAttribute" (bike shedding to come); which >> accepts a single Reflector argument. >> >> If the attribute implements the interface, the method is called immediately 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 discussion 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 object - which then adapted into Larry's mention of specifically setting a property before the constructor is run. 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 objects longer than absolutely necessary, so I don't think this feature should 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 on" etc) Cheers Stephen >> >> Yes this means logic dependant on the reflector has to be delayed until the method is >> called. I think this is an acceptable payoff for the solution: it only applies to attributes that >> explicitly opt in to receive the Reflector, and it helps to not encourage storing the reflector in a >> property. > > yep, same tradeoff I mentioned in the other mail. > >> >> In theory I guess it could call a static named constructor, but the other arguments would >> have to be an array, which would end up being quite clunky if the goal is to derive default argument >> values from the reflector. >> >> I'm really looking forward to this feature, thanks for introducing this >> discussion/RFC! >> >> >> >> Cheers >> >> Stephen