On Fri, Jul 4, 2025, at 17:21, Andreas Hennings wrote:
> On Fri, 4 Jul 2025 at 06:30, Stephen Reay <php-li...@koalephant.com> wrote:
> >
> >
> >
> > On 4 Jul 2025, at 00:54, Andreas Hennings <andr...@dqxtech.net> wrote:
> >
> > On Thu, 3 Jul 2025 at 19:17, Stephen Reay <php-li...@koalephant.com> wrote:
> >
> >
> >
> >
> >
> > Sent from my iPhone
> >
> > On 3 Jul 2025, at 23:40, Larry Garfield <la...@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 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 
> ->injectTargetReflector()?
> 
> >
> > 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)
> >
> 
> 
> --------
> 
> 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 =
> 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? 
https://3v4l.org/srdM6

You can do this in regular php, but it is even simpler in the engine.

— Rob

Reply via email to