Re: Re: Attributes and constructor property promotion

On Mon, Sep 28, 2020, at 12:06 PM, Benjamin Eberlei wrote: > On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel <benjamin.morel@gmail.com> > wrote: > > > On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas <nicolas.grekas+php@gmail.com> > > wrote: > > > >> I assume the 80% case is properties, because attributes did not have > >>> docblock annotations yet, that means this use-case isn't even possible at > >>> the moment. Yet annotations on properties are widespread (Doctrine ORM, > >>> symfony validator, ...). > >>> > >> > >> I'm 100% with Benjamin here, this is what will be the most useful to me > >> also. > >> > > > > To be clear, I don't have a strong opinion against yours, I'm just > > pointing out the fact that even though it might be useful, it might also be > > confusing and create yet another WTF moment in PHP for developers. Sure, it > > might make more sense to apply to the property. Sure, so far annotations > > weren't possible on parameters. But is that obvious to the average > > developer writing the attribute? A few years down the road, DI containers > > may have broad support for annotating parameters for injection. Will it > > still be obvious then that an attribute on a promoted property applies to > > the property only? > > > > I do agree that applying the attribute to both the property and the > > parameter will probably never be useful, though. So, throwing an exception > > and forcing the de-sugaring feels like the most sensible thing to do for me > > in this case! > > > > I haven't checked if this is possible in the code doing the "desugering", > but what if we had an attribute on the constructor that could specify where > the attributes should apply to? > > #[AttributePromotion(Attribute::TARGET_PROPERTY)] > public function __construct(#[Foo] public string $bar) {} > > Then we could apply it to both by default, which is what is probably the > expected approach, and users could change it to apply only to properties, > which is what is the use-case that makes most sense. > > > > > — Benjamin From a user experience POV, I'd almost expect the opposite. If I mark the attribute as being for properties, it gets applied to the property. If I mark the attribute as being for parameters, it gets applied to the parameter. If I mark it for both, or don't restrict it at all, it applies to both. That may not be how the logic is currently implemented but that's what as a user I'd find least-surprising. --Larry Garfield