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?

* 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() {}

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


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.

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



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?

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

Reply via email to