Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127900
Precedence: bulk
MIME-Version: 1.0
References: <CAB-Buja3vt9ho3Hxf5FqFB-1vW7+11_2O3KpXdzFbiRBTviQVA@mail.gmail.com>
 <1cfc477a-781f-40e9-9e37-dd748ef261be@app.fastmail.com> <CAB-BujaAXVp1OPs0XLtVix_NBLjWjyx=TRtbTAnWw-Rzg91UZg@mail.gmail.com>
 <926ca0e2-13f4-47fc-aec3-f7fa265cfdb5@app.fastmail.com>
In-Reply-To: <926ca0e2-13f4-47fc-aec3-f7fa265cfdb5@app.fastmail.com>
Date: Sat, 5 Jul 2025 01:50:40 +0200
Message-ID: <CAB-BujbJrvAmK+V49f9JUhR_g_XTrTBUtpB0QG6jeRyqh308Xg@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC idea] Target-aware attributes
To: Larry Garfield <larry@garfieldtech.com>
Cc: 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 Thu, 3 Jul 2025 at 23:44, Larry Garfield <larry@garfieldtech.com> wrote:
>
> On Thu, Jul 3, 2025, at 12:49 PM, Andreas Hennings wrote:
>
> >> A setter method injection is what I did in AttributeUtils, because it =
was the only real option.
> >
> > In my experience, this alternative leads to more complexity in
> > attribute classes.
> > The constructor needs to populate the attribute with temporary values,
> > which are then replaced in that separate method.
> > (I call that a "setter", but only because it behaves like one from the
> > outside, internally we usually don't want to store the reflector).
> >
> > Also, we can never be fully sure whether an attribute instance is
> > "complete" or not.
> > If the attribute instance is coming from
> > $reflection_attribute->newInstance(), we can be confident that the
> > ->injectReflector() or has been called.
> > But if the attribute was instantiated manually with "new ...", it
> > could as well be incomplete.
> > With ReflectionAttribute::getCurrentTargetReflector(), we can fail
> > early in the attribute constructor, by throwing on NULL.
>

I somehow missed this reply earlier...

> I ran into the "completeness problem" in AttributeUtils as well.  In my c=
ase, it's not just reflection; there's a whole bunch of other things that c=
an be passed back to the attribute to give it more context.

There is a trade-off how much logic should live in the attribute vs
the code that parses the attribute.
In symfony DI there is a pattern where you register a callback for an
attribute class, which would allow to make the attribute agnostic of
the application or framework details.
E.g. you could have the same attribute for different DI systems.
(Perhaps this is not the reason why symfony does it in this way.)
On the other hand, keeping logic in the attribute class also has its benefi=
ts.

I did play around with dedicated interfaces for attributes per application.
I am not sure I like the overly generic interfaces I see in
AttributeUtils. But I would have to look deeper into it.

>  My eventual solution was to also have a `Finalizable` interface that is =
guaranteed to be the last thing called, so the attribute can handle any las=
t-minute defaults.  I don't love it, but given the severe limitations of `r=
eadonly` it was the best I could do.  (With aviz today, I could likely do b=
etter.)

The attribute object does not have to be your final "collectable".
You can have an attribute with a method that returns something else.
In fact I strongly prefer to _not_ pass attribute instances around
everywhere, because they often contain clutter that becomes useless
after the initial discovery.

E.g.
$reflection_attributes =3D
$reflection_class->getAttributes(ServiceDefinition::class)->getAttributes()=
;
$attribute_instance =3D $reflection_attributes[0]->newInstance();
$service_definitions =3D
$attribute_instance->getServiceDefinitions(...$more_args);

Or
$reflection_attributes =3D
$reflection_method->getAttributes(RouteModifier::class)->getAttributes();
$route =3D new Route();
foreach ($reflection_attributes as $reflection_attribute) {
  $reflection_attribute->newInstance()->modifyRoute($route, ...$more_args);
}
return $route;

This way, you have a complete object in each step, but only the final
object is the one you want to "collect".

>
> Basically, any time you have multi-step construction you will have a peri=
od where the state is potentially incomplete.

It is not just about possible incompleteness, it is about being
deterministic as early as possible.
E.g. if two people enter a room, it could make _some_ things much
easier if you know whether they are brother and sister, or a married
couple.

> While passing the reflection target in is one such multi-step case, there=
 are lots of others, so in advanced cases (most of what I do)...

When looking at existing attributes in e.g. symfony or Drupal, or my
own experiments, I don't see the huge need to inject other things into
the attribute.
The reflector seems really the most commonly useful to me.

E.g. an attribute on a method/function might have a parameter that is
optional or required depending on the method's return type.
Checking that in the constructor allows us to fail early, and makes
everything more deterministic.

> ... the object will always have an incomplete phase, and that's unavoidab=
le.  So I'm not *that* worried about there being a brief incomplete-gap in =
time in core, because I'll always have one anyway, and I've already figured=
 out how to handle it.

I find that in most cases it can actually be avoided.
But I have not really looked into your code much. I have seen some
packages in packagist that use attributesutil, but I would need more
time to study these.

-- Andreas

>
> --Larry Garfield