Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120452
MIME-Version: 1.0
References: <CAB-BujaDnogpq7=BwwVbbT6XmBAAiDqQ3mDqgNccdzG1nW=WuQ@mail.gmail.com>
 <64EAB991-7082-47B8-B546-73CD08243C6E@koalephant.com>
In-Reply-To: <64EAB991-7082-47B8-B546-73CD08243C6E@koalephant.com>
Date: Tue, 30 May 2023 14:06:26 +0200
Message-ID: <CAB-BujYA+CG1GkS176eqsVkMrgQVqX=P8xNPz9zNgijE1uMC6w@mail.gmail.com>
To: Stephen Reay <php-lists@koalephant.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Declaration-aware attributes
From: andreas@dqxtech.net (Andreas Hennings)

On Tue, 30 May 2023 at 05:22, Stephen Reay <php-lists@koalephant.com> wrote=
:
>
>
>
> > On 30 May 2023, at 07:48, Andreas Hennings <andreas@dqxtech.net> wrote:
> >
> > Hello internals,
> > I am picking up an idea that was mentioned by Benjamin Eberlei in the p=
ast.
> > https://externals.io/message/110217#110395
> > (we probably had the idea independently, but Benjamin's is the first
> > post where I see it mentioned in the list)
> >
> > Quite often I found myself writing attribute classes that need to fill
> > some default values or do some validation based on the symbol the
> > attribute is attached to.
> > E.g. a parameter attribute might require a specific type on that
> > parameter, or it might fill a default value based on the parameter
> > name.
> >
> > Currently I see two ways to do this:
> > 1. Do the logic in the code that reads the attribute, instead of the
> > attribute class. This works ok for one-off attribute classes, but it
> > becomes quite unflexible with attribute interfaces, where 3rd parties
> > can provide their own attribute class implementations.
> > 2. Add additional methods to the attribute class that take the symbol
> > reflector as a parameter, like "setReflectionMethod()", or
> > "setReflectionClass()". Or the method in the attribute class that
> > returns the values can have a reflector as a parameter.
> >
> > Both of these are somewhat limited and unpleasant.
> >
> > I want to propose a new way to do this.
> > Get some feedback first, then maybe an RFC.
> >
> > The idea is to mark constructor parameters of the attribute class with
> > a special parameter attribute, to receive the reflector.
> > The other arguments are then shifted to skip the "special" parameter.
> >
> > #[Attribute]
> > class A {
> >  public function __construct(
> >    public readonly string $x,
> >    #[AttributeContextClass]
> >    public readonly \ReflectionClass $class,
> >    public readonly string $y,
> >  ) {}
> > }
> >
> > $a =3D (new ReflectionClass(C::class))->getAttributes()[0]->newInstance=
();
> > assert($a instanceof A);
> > assert($a->x =3D=3D=3D 'x');
> > assert($a->class->getName() =3D=3D=3D 'C');
> > assert($a->y =3D=3D=3D 'y');
> >
> > Note that for methods, we typically need to know the method reflector
> > _and_ the class reflector, because the method could be defined in a
> > base class.
> >
> > #[Attribute]
> > class AA {
> >  public function __construct(
> >    #[AttributeContextClass]
> >    public readonly \ReflectionClass $class,
> >    #[AttributeContextMethod]
> >    public readonly ReflectionMethod $method,
> >  ) {}
> > }
> >
> > class B {
> >  #[AA]
> >  public function f(): void {}
> > }
> >
> > class CC extends B {}
> >
> > $aa =3D (new ReflectionMethod(CC::class, 'f))->getAttributes()[0]->newI=
nstance();
> > assert($a->class->getName() =3D=3D=3D 'CC');
> > assert($a->method->getName() =3D=3D=3D 'f');
> >
> > ---
> >
> > Notice that the original proposal by Benjamin would use an interface
> > and a setter method, ReflectorAwareAttribute::setReflector().
> >
> > I prefer to use constructor parameters, because I generally prefer if
> > a constructor creates a complete and immutable object.
> >
> > ----
> >
> > Thoughts?
> >
> > -- Andreas
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: https://www.php.net/unsub.php
> >
>
> Hi Andreas,
>
> I too have wondered (and I think asked in room11?) about such a concept. =
From memory the general response was =E2=80=9Cjust do it in userland with a=
 wrapper=E2=80=9D so its good to see someone else is interested in this bei=
ng part of the language.
>
> While I agree that it=E2=80=99s most useful if the `Reflector` instance i=
s available in the constructor, I=E2=80=99m not keen on the proposed magic =
=E2=80=9Cskipping=E2=80=9D of arguments as you suggest. It seems way too ea=
sy to confuse someone (particularly if the attribute class itself has reaso=
n to be instantiated directly in code)

Good point! Almost made me change my mind completely. But I already
changed it back :)

When instantiating in code, the "real" signature would have to be
used, and the reflector argument passed explicitly. This would be
useful for unit tests that want to replicate the realistic behavior.
Also it guarantees that the code of the attribute class can really
count on this value to not be null, no matter how the class is
instantiated.


>
> I think a better approach would be to suggest authors put the parameter a=
t the *end* of the parameter list, so that no =E2=80=98skipping' is require=
d when passing arguments without names (or put it where you like if you=E2=
=80=99re always using named arguments)

If I understand correctly, the proposal would technically not change,
we just add a recommendation.

The only problem I see is with variadic parameters, which naturally
need to be last.

----

This brings me to another idea: What if instead of the parameter
attribute we use a pseudo constant default value?
Or some other kind of expression that is only valid in the right context.

#[Attribute]
class A {
  function __construct(
    ..
    ReflectionClass $class =3D __REFLECTOR__,
    ..
  ) {..

The "constant" would be available only as a default value, not inside
the constructor body.

But I already don't like this idea, because
ReflectionParameter->getDefaultValue() would have to throw an
exception, or ->isOptional would have to return false.

Better to have some kind of placeholder in the attribute declaration,
if we dont like the skipping.

#[A('x', ?, 'y')]
or
#[A('x', __REFLECTOR__, 'y')]

The '?' would be the same as proposed for first class callable partial
function application.

This could be interpreted as that we create a callable with only one
free parameter, which php then invokes with the reflector.

But even the skipping without a placeholder can be justified, because
an attribute declaration is already not regular code, e.g. we don't
write "new ".


>
>
> Cheers
>
> Stephen
>