Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108931
MIME-Version: 1.0
References: <CAEPJdunSr8PkBtSLO8g_JMLzM4cFVnXsQSEMFmu2RN25XT=grw@mail.gmail.com>
 <2461944e-bc66-4941-aef4-ed07aefa3858@www.fastmail.com>
In-Reply-To: <2461944e-bc66-4941-aef4-ed07aefa3858@www.fastmail.com>
Date: Tue, 10 Mar 2020 00:29:02 +0100
Message-ID: <CAEPJdumPxwwc9FV7FgbbtJLZ6DDJoie9iVfjYLDWLbNctKcEwQ@mail.gmail.com>
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000002047705a0745f0b"
Subject: Re: [PHP-DEV] [RFC] Attributes v2
From: kontakt@beberlei.de (Benjamin Eberlei)

--00000000000002047705a0745f0b
Content-Type: text/plain; charset="UTF-8"

On Mon, Mar 9, 2020 at 11:03 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> On Mon, Mar 9, 2020, at 9:42 AM, Benjamin Eberlei wrote:
> > Hi all,
> >
> > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > 2016 with a few changes, incorporating feedback from the mailing list
> back
> > then and from talking to previous no voters.
> >
> > The RFC is at https://wiki.php.net/rfc/attributes_v2
> >
> > A working patch is at https://github.com/beberlei/php-src/pull/2 though
> > work around the details is still necessary.
> >
> > The RFC contains a section with common criticism and objections to
> > attributes, and I hope to have collected and responded to a good amount
> > already from previous discussions.
> >
> > There is also a fair amount of implementation detail still up for debate,
> > which is noted in "Open Issues". I have pre-committed to one approach,
> but
> > listed alternatives there. On these issues I am looking for your
> feedback.
> >
> > greetings
> > Benjamin
>
> I am very excited to see this back, and overall I like the approach!
>
> Thoughts in no special order:
>
> * It's not entirely clear, but the values in an argument-carrying
> attribute are passed to its constructor variadically?  Viz:
>
> <<Foo(1, 2, 3)>>
> function stuff() {}
>
> class Foo implements Attribute {
>   public function __construct(int $first, int $second, int $third) { ... }
> }
>
> Yes?  (And then if you wanted to capture them all you could use a ...$args
> parameter in the constructor.)
>
> * It looks like the only way to support named parameters would be to pass
> an associative array and decompose it yourself.  That's... better than
> nothing I guess but also we're back to the same lack of self-documentation
> we always lament.  Is there any opportunity to do named parameters in some
> other way?
>

Improvements here are contingent on any named parameters support, which
could

>
> * Is it possible to inline a sub-object, or no?  Viz, is this legal, or
> would we want it to be legal (I'm assuming it's not at the moment):
>
> <<Foo('a', 'b', Bar('baz'))>>
> function stuff() {}
>

No thats not possible, because the constant expression syntax doesn't allow
this. See Tysons proposal lately to allow function calls in constant
expressions: https://externals.io/message/108630

>
> * I *really* dig how you can filter annotations to just those that match a
> certain parent class/interface.  That was one of the nicer parts of PSR-14
> so I'm really glad to see the same "make the type system earn its keep"
> approach here.  It encourages packages to interface-tag their annotations
> so they can easily grab "just theirs".  +1
>
> * The "more complex Doctrine ORM use-case" example includes inlining
> multiple annotations together: <<ORM\Id, ORM\Column, ORM\GeneratedValue>>
> .  Is that just an oops since that possibility was only just added to the
> Open Issues?
>
> Ah this is a mistake, I removed that use-case and settled on a single
syntax.

>
> Regarding open issues:
>
> * If constant expressions are easy enough to support, I don't see a reason
> not to support them.
>
> * I'm torn on the marker interface.  On the one hand, a marker interface
> would make it easier to audit a package for all of its attributes; just
> grep for "implements Attribute".  I'm sad that we didn't get a marker
> interface in PSR-14 for that reason.  On the other, if it doesn't do
> anything except be a marker then it doesn't really offer anything else; and
> it also potentially makes it harder to add some special casing of
> attributes in the future, say to offer "if you want this extra attribute
> behavior, use this interface/base class/whatever."  I could go either way
> here.
>
> * Letting attributes say where they're legal would be a good case for such
> a more-than-marker interface.
>

I am not sure. This is something PHP would validate at
Reflection::getAttribute() time, at which time it is also easy to validate
for userland implementations.

>
> * The aforementioned example of specifying where an attribute is legal
> implies that attribute classes can themselves have attributes.  True?  If
> so, that should be made explicit elsewhere in the RFC.  I'm not against
> attribute-ception, but that should be explicit.
>
> Attributes are just PHP classes, so they can indeed have their own
attributes. So a userland library building on top of attributes, could call
getAttributes() then validate that each attribute returned has itself an
attribute that "marks the target":

<<Target(["class"])>>
class Entity
{
}


> I think my only significant pushback at the moment is that attributes here
> suffer from the same redundancy problem as any other value object.  To wit:
>
> class Owner implements Attribute {
>
>   public readonly string $name;
>
>   public readonly string $title;
>
>   public readonly int $age;
>
>   public function __construct(string $name, string $title, int $age) {
>     $this->name = $name;
>     $this->title = $title;
>     $this->age = $age;
>   }
> }
>
> <<Owner('Larry', 'Manager', 99)>>
> function stuff() {}
>
> That necessitates the same quadrupal-repetition that other constructors
> have.  Since I anticipate the 99% use case to be exactly that, the pain of
> that redundancy (and the mandatory unnamed order) will be felt rather
> dramatically here and thus hurts ergonomics.  Is there no way that we
> address that?
>

This is not something to fix for this RFC, but any solution here and
attributes can benefit from it as they are just PHP classes.

>
> I realize the answer may be "no more than any other class, they all suck
> equally".  Are we at least certain then that if we can solve that in the
> future for classes generally that it will automatically work here?  (Eg, if
> new Owner({name: 'Larry', 'age' => 99, 'title' => 'Manager'}); became a
> legal shorthand for the above, it would automatically work for annotations
> as well.)
>
> Overall nice work, and I hope to get to use it.
>
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--00000000000002047705a0745f0b--