On Tue, Sep 29, 2020 at 3:45 PM Larry Garfield <la...@garfieldtech.com>
wrote:
> On Mon, Sep 28, 2020, at 12:06 PM, Benjamin Eberlei wrote:
> > On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel <benjamin.mo...@gmail.com
> >
> > wrote:
> >
> > > On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas <
> nicolas.grekas+...@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.
>
The problem with this approach is that it would require autoloading the
attributes when they are assigned to either the internal property or
parameter struct, but we have the design goal *not* to trigger autoloading
unless newInstance() or getArguments() is called. What you could do in
userland code is handle this case yourself and never newInstance()
attributes that don't apply to the right "thing" (parameter vs property).
but that would defer the problem to userland with some annoying piece of
code.
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
