Re: [RFC] Attributes v2

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 > >