# [RFC] Attributes v2 - Thread: https://externals.io/message/108907 - Original archive: https://news-web.php.net/php.internals/108907 - Started by Benjamin Eberlei on 2020-03-09T14:42:33+00:00 - Messages: 66 - Page 1 of 3: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/108907.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #108907 — Benjamin Eberlei — 2020-03-09T14:42:33+00:00 URL: https://externals.io/message/108907#108907 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 --- ## #108908 — Benjamin Eberlei — 2020-03-09T14:45:19+00:00 URL: https://externals.io/message/108907#108908 · In reply to: #108907 > 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. > Obviously I am looking for feedback on the whole RFC, not just the open issues :-) > > greetings > Benjamin --- ## #108909 — Stephen Reay — 2020-03-09T16:18:46+00:00 URL: https://externals.io/message/108907#108909 · In reply to: #108907 > > > 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 Hi Benjamin, I can’t comment on the feasibility of this RFC or the patch, but I like the approach it takes to this ‘problem’. I can already imagine a number of ‘nicer’ ways to achieve similar/same results using this, so thanks for taking up the baton! My one small query/request is about the `Reflection*::getAttributes` method. Is there a philosophical and/or technological reason for why it can only ‘filter’ the attributes by a single given name, rather than being variadic and filtering against multiple names (i.e. returning those that match any of the given names)? I would imagine many uses (particularly within libraries) would end up wanting to retrieve multiple attribute types they’re aware of all at once, no? Cheers Stephen --- ## #108910 — Benjamin Eberlei — 2020-03-09T16:27:19+00:00 URL: https://externals.io/message/108907#108910 · In reply to: #108909 On Mon, Mar 9, 2020 at 5:18 PM Stephen Reay wrote: > > > > […] > back > > […] > but > > […] > feedback. > > […] > > Hi Benjamin, > > I can’t comment on the feasibility of this RFC or the patch, but I like > the approach it takes to this ‘problem’. I can already imagine a number of > ‘nicer’ ways to achieve similar/same results using this, so thanks for > taking up the baton! > > My one small query/request is about the `Reflection*::getAttributes` > method. Is there a philosophical and/or technological reason for why it can > only ‘filter’ the attributes by a single given name, rather than being > variadic and filtering against multiple names (i.e. returning those that > match any of the given names)? I would imagine many uses (particularly > within libraries) would end up wanting to retrieve multiple attribute types > they’re aware of all at once, no? > The name filtering works when using a common base class or interface, so if a library has a class "My\BaseAttribute" and all related attributes extend from it, then in the code you can do: $attributes = $reflectionClass->getAttributes(\My\BaseAttribute::class); This would return all attributes matching. > > > Cheers > > > Stephen --- ## #108911 — Stephen Reay — 2020-03-09T16:31:04+00:00 URL: https://externals.io/message/108907#108911 · In reply to: #108910 > > > > > > > > > […] > > Hi Benjamin, > > I can’t comment on the feasibility of this RFC or the patch, but I like the approach it takes to this ‘problem’. I can already imagine a number of ‘nicer’ ways to achieve similar/same results using this, so thanks for taking up the baton! > > My one small query/request is about the `Reflection*::getAttributes` method. Is there a philosophical and/or technological reason for why it can only ‘filter’ the attributes by a single given name, rather than being variadic and filtering against multiple names (i.e. returning those that match any of the given names)? I would imagine many uses (particularly within libraries) would end up wanting to retrieve multiple attribute types they’re aware of all at once, no? > > The name filtering works when using a common base class or interface, so if a library has a class "My\BaseAttribute" and all related attributes extend from it, then in the code you can do: > > $attributes = $reflectionClass->getAttributes(\My\BaseAttribute::class); > > This would return all attributes matching. > > > Cheers > > > Stephen Ah! I had wondered if that type of filtering would be possible (another benefit of using classes for the attributes) but I didn’t see it mentioned, and wondered if the intention was to leave it to userland. Cheers Stephen --- ## #108912 — Paul M. Jones — 2020-03-09T16:34:19+00:00 URL: https://externals.io/message/108907#108912 · In reply to: #108907 Hi Benjamin, From the examples, am I to understand that the RFC introduces a top-level namespace `\Php` ? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php --- ## #108914 — Rowan Tommins — 2020-03-09T17:17:31+00:00 URL: https://externals.io/message/108907#108914 · In reply to: #108907 Hi Benjamin > 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 > Thanks for picking this up again. It's amazing how deeply the integration of annotations has embedded into PHP without any support from the language, so this feels long overdue. One quick suggestion: the use cases currently appear inside the Proposal heading in the RFC, and some but not all are labelled "not part of the RFC". It might be useful to have a clearly separate "Possible Use Cases" header, and a summary somewhere of which interfaces and classes would actually be provided by the initial implementation. Regards, -- Rowan Tommins [IMSoP] --- ## #108916 — Benjamin Eberlei — 2020-03-09T18:52:56+00:00 URL: https://externals.io/message/108907#108916 · In reply to: #108914 On Mon, Mar 9, 2020 at 6:18 PM Rowan Tommins wrote: > Hi Benjamin > > > > > […] > back > > […] > > > Thanks for picking this up again. It's amazing how deeply the integration > of annotations has embedded into PHP without any support from the language, > so this feels long overdue. > > One quick suggestion: the use cases currently appear inside the Proposal > heading in the RFC, and some but not all are labelled "not part of the > RFC". It might be useful to have a clearly separate "Possible Use Cases" > header, and a summary somewhere of which interfaces and classes would > actually be provided by the initial implementation. > Thank you, I didn't notice that before in the ToC view, fixed! > > Regards, > -- > Rowan Tommins > [IMSoP] --- ## #108917 — Matthew Brown — 2020-03-09T19:16:41+00:00 URL: https://externals.io/message/108907#108917 · In reply to: #108907 For information that's needed at runtime (as opposed to documentation or static analysis) docblocks have an obvious overh I think the syntax here looks great, and I think this would be an exciting addition to the language. I want to build things with it! > 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 --- ## #108918 — Andrea Faulds — 2020-03-09T19:18:54+00:00 URL: https://externals.io/message/108907#108918 · In reply to: #108907 Benjamin Eberlei wrote: > 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 Hi, I have concerns about these two statements in the RFC: > The name of an attribute is resolved against the currently active namespace import scope during compilation. The resolved class names are then autoloaded to make sure they exist. > Consistent with PHP expressions in general, no validation is performed if the provided attribute arguments are fullfilling the contract of the attribute class constructor. This would happen only when accessing attributes as objects in the Reflection API (below). These two details are inconsistent with eachother: use of an annotation triggers an autoload, yet we aren't using the class that is autoloaded to validate it? This seems quite wasteful: if we have loaded the class, we might as well use it to check the arguments are correct. Also, why are we privileging the class existing over the arguments to the class being correct? If the arguments can be validated at Reflection time, surely the autoloading can be done then too? Both types of coding mistake are important. It also seems inconsistent with existing PHP behaviour, I think normally mentioning a class either triggers an immediate autoload and actual execution/validation (`new`) or it doesn't (a type declaration). This proposal is a strange half-way house. Is this being done to avoid paying the cost of creating the object at compilation time? Because I think triggering the autoload is going to be expensive anyway, possibly moreso. On a different note, the wording here is syntactically ambiguous. It can be read as both "if the provided attribute arguments are fullfilling the contract […], then no validation is performed" and "no validation is performed as to whether the provided attribute arguments are fullfilling the contract". I read it as the former the first time, which confused me for a moment. Another thing: > Thanks to class name resolving, IDEs or static analysis tools can perform this validation for the developer. Is this referencing the autoloading behaviour? I don't see why that would be required. (You could also be referring to the fact you use classes, which IDEs can look for, instead of arbitrary string attributes, which IDEs can not, which does make sense.) Thanks, Andrea --- ## #108919 — David Rodrigues — 2020-03-09T19:29:23+00:00 URL: https://externals.io/message/108907#108919 As I am not a good expert on parser (not to say that I don't do anything), could you tell me if I can write a note like that? <<[space]Annotation()[space]>> << MyAnnotation(1, 2, 3) >> It's just because I think the code is more "breathable". Until the PSR staff decides how best to write. Atenciosamente, David Rodrigues Em seg., 9 de mar. de 2020 às 16:19, Andrea Faulds escreveu: > Benjamin Eberlei wrote: > > […] > back > > […] > > Hi, > > I have concerns about these two statements in the RFC: > > > […] > namespace import scope during compilation. The resolved class names are > then autoloaded to make sure they exist. > > > […] > performed if the provided attribute arguments are fullfilling the > contract of the attribute class constructor. This would happen only when > accessing attributes as objects in the Reflection API (below). > > These two details are inconsistent with eachother: use of an annotation > triggers an autoload, yet we aren't using the class that is autoloaded > to validate it? This seems quite wasteful: if we have loaded the class, > we might as well use it to check the arguments are correct. Also, why > are we privileging the class existing over the arguments to the class > being correct? If the arguments can be validated at Reflection time, > surely the autoloading can be done then too? Both types of coding > mistake are important. > > It also seems inconsistent with existing PHP behaviour, I think normally > mentioning a class either triggers an immediate autoload and actual > execution/validation (`new`) or it doesn't (a type declaration). This > proposal is a strange half-way house. > > Is this being done to avoid paying the cost of creating the object at > compilation time? Because I think triggering the autoload is going to be > expensive anyway, possibly moreso. > > On a different note, the wording here is syntactically ambiguous. It can > be read as both "if the provided attribute arguments are fullfilling the > contract […], then no validation is performed" and "no validation is > performed as to whether the provided attribute arguments are fullfilling > the contract". I read it as the former the first time, which confused me > for a moment. > > Another thing: > > > […] > perform this validation for the developer. > > Is this referencing the autoloading behaviour? I don't see why that > would be required. (You could also be referring to the fact you use > classes, which IDEs can look for, instead of arbitrary string > attributes, which IDEs can not, which does make sense.) > > Thanks, > Andrea --- ## #108920 — Robert Hickman — 2020-03-09T19:29:44+00:00 URL: https://externals.io/message/108907#108920 · In reply to: #108909 > > […] Why do decorator names refer to classes, rather than any callable as in python? --- ## #108921 — Aleksander Machniak — 2020-03-09T19:35:10+00:00 URL: https://externals.io/message/108907#108921 · In reply to: #108907 > The RFC is at https://wiki.php.net/rfc/attributes_v2 Would it make sense to support this: <> <> <> function foo() {} in this form: << WithoutArgument; // comments allowed here SingleArgument(0); FewArguments('Hello', 'World'); > > […] function foo() {} or if there's not many attributes, in the same line: <> It may look better for the short version, but I'm not so sure about the long version. -- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net] ---------------------------------------------------- PGP: 19359DC1 # Blog: https://kolabian.wordpress.com --- ## #108922 — David Rodrigues — 2020-03-09T19:54:34+00:00 URL: https://externals.io/message/108907#108922 · In reply to: #108921 Another doubt: in relation to the PR ( https://github.com/beberlei/php-src/pull/2) it is called as "attributes", but seems better keep as "annotations". The Doctrine annotations is based in PHPDoc, are not? And it is basically the same thing, and annotations term is used in other languages. And too, could annotators have some additional features like Typescript decorators? Atenciosamente, David Rodrigues Em seg., 9 de mar. de 2020 às 16:35, Aleksander Machniak escreveu: > > > […] > > Would it make sense to support this: > > <> > <> > <> > function foo() {} > > in this form: > > << > WithoutArgument; // comments allowed here > SingleArgument(0); > FewArguments('Hello', 'World'); > > […] > function foo() {} > > or if there's not many attributes, in the same line: > > <> > > It may look better for the short version, but I'm not so sure about the > long version. > > -- > Aleksander Machniak > Kolab Groupware Developer [https://kolab.org] > Roundcube Webmail Developer [https://roundcube.net] > ---------------------------------------------------- > PGP: 19359DC1 # Blog: https://kolabian.wordpress.com --- ## #108923 — Benjamin Eberlei — 2020-03-09T20:24:21+00:00 URL: https://externals.io/message/108907#108923 > Benjamin Eberlei wrote: > > […] > back > > […] > > Hi, > > I have concerns about these two statements in the RFC: > > > […] > namespace import scope during compilation. The resolved class names are > then autoloaded to make sure they exist. > > > […] > performed if the provided attribute arguments are fullfilling the > contract of the attribute class constructor. This would happen only when > accessing attributes as objects in the Reflection API (below). > > These two details are inconsistent with eachother: use of an annotation > triggers an autoload, yet we aren't using the class that is autoloaded > to validate it? This seems quite wasteful: if we have loaded the class, > we might as well use it to check the arguments are correct. Also, why > are we privileging the class existing over the arguments to the class > being correct? If the arguments can be validated at Reflection time, > surely the autoloading can be done then too? Both types of coding > mistake are important. > > It also seems inconsistent with existing PHP behaviour, I think normally > mentioning a class either triggers an immediate autoload and actual > execution/validation (`new`) or it doesn't (a type declaration). This > proposal is a strange half-way house. > > Is this being done to avoid paying the cost of creating the object at > compilation time? Because I think triggering the autoload is going to be > expensive anyway, possibly moreso. > Thinking about how I got to my thought process here, I believe it was something like When compiling new Foo("abcdefg", 1234); then it resolves the class name (does it autoload?) but it wouldn't check if the arguments match until its actually executed. I believe I am wrong to propose autoloading directly at compiling the code is not how PHP does it. It should autoload only during Reflection*::getAttributes(). this is something I haven't implemented in the prototype PR yet, maybe I would have seen the problems. Going to mule this one over and adjust the RFC and this thread with an update. > On a different note, the wording here is syntactically ambiguous. It can > be read as both "if the provided attribute arguments are fullfilling the > contract […], then no validation is performed" and "no validation is > performed as to whether the provided attribute arguments are fullfilling > the contract". I read it as the former the first time, which confused me > for a moment. > Will update to clarify. > > Another thing: > > > […] > perform this validation for the developer. > > Is this referencing the autoloading behaviour? I don't see why that > would be required. (You could also be referring to the fact you use > classes, which IDEs can look for, instead of arbitrary string > attributes, which IDEs can not, which does make sense.) > That is right, an IDE can find this out without the autoloading behavior. This sentence means like you inferred, that because attributes are semantically mapping to class names, that is the reason why IDEs can better validate them in their own static analysis. > > Thanks, > Andrea --- ## #108924 — Benjamin Eberlei — 2020-03-09T20:30:40+00:00 URL: https://externals.io/message/108907#108924 · In reply to: #108919 On Mon, Mar 9, 2020 at 8:30 PM David Rodrigues wrote: > As I am not a good expert on parser (not to say that I don't do anything), > could you tell me if I can write a note like that? > > <<[space]Annotation()[space]>> > << MyAnnotation(1, 2, 3) >> > > It's just because I think the code is more "breathable". Until the PSR > staff decides how best to write. > Yes, you can add as many spaces as you want between << Attr >> On your other question: It is named Attributes in C#, C++, Rust and Hack, so this name is not something novell. The naming discussion is a coin flip issue in my opinion. You can't really argue to name it Annotations, just because Doctrine uses it this way, because historically it would have been docblock "tags" how php-doc named them. > > > Atenciosamente, > David Rodrigues > > > Em seg., 9 de mar. de 2020 às 16:19, Andrea Faulds escreveu: > > > […] --- ## #108925 — Benjamin Eberlei — 2020-03-09T20:34:32+00:00 URL: https://externals.io/message/108907#108925 · In reply to: #108921 > > > […] > > Would it make sense to support this: > > <> > <> > <> > function foo() {} > > in this form: > > << > WithoutArgument; // comments allowed here > SingleArgument(0); > FewArguments('Hello', 'World'); > > […] > function foo() {} > > or if there's not many attributes, in the same line: > > <> > > It may look better for the short version, but I'm not so sure about the > long version. > Its certainly possible, the original Attributes v1 RFC supported both syntaxes. I wanted to simplify it and opted for each attribute to have its own <<>> enclosing. I have added it to the list of open issues to determine what approach to take. > > -- > Aleksander Machniak > Kolab Groupware Developer [https://kolab.org] > Roundcube Webmail Developer [https://roundcube.net] > ---------------------------------------------------- > PGP: 19359DC1 # Blog: https://kolabian.wordpress.com --- ## #108926 — Benjamin Eberlei — 2020-03-09T20:36:46+00:00 URL: https://externals.io/message/108907#108926 · In reply to: #108912 > Hi Benjamin, > > From the examples, am I to understand that the RFC introduces a top-level > namespace `\Php` ? > > Honestly didn't realize this and thought PhpToken would already be in the PHP namespace. I am going to think this over and probably go for PhpAttribute in the global namespace instead. It wasn't my intention to create a precedent here :) > > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com > > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp > > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php --- ## #108927 — Benjamin Eberlei — 2020-03-09T20:43:04+00:00 URL: https://externals.io/message/108907#108927 · In reply to: #108920 > > […] > though > > […] > > Why do decorator names refer to classes, rather than any callable as in > python? > Decorators in Python work differently (to my understanding) that the following function declartion @foo def bar(): At runtime becomes foo(bar()); PHPs Attributes do not become runtime behavior, instead they allow the Reflection API to access the attributes as metadata. As such declaring a callable doesn't make sense, because an attribute is not a callable, it is (meta-)data. --- ## #108928 — Saif Eddin Gmati — 2020-03-09T21:01:52+00:00 URL: https://externals.io/message/108907#108928 · In reply to: #108907 Really excited to see the discussion about this feature being brought up again, thanks for working on this :) One thing I would suggest is to add a base interface `Attribute` and multiple sub-interfaces such as `ClassAttribute`, `MethodAttribute`, and `PropertyAttribute` just like hack lang ( https://docs.hhvm.com/search?term=HH%20Attribute ) This allows one to declare attributes that can only be used with classes ( e.g `ORM\Entity` ), so PHP and check against them at runtime and throws an error when an attribute is used in the wrong place. ```php namespace ORM { class Entity implements \ClassAttribute { ... } } namespace Foo { use ORM; <> // <-- Error function bar(): void {} } ``` I also question whether the following syntax would be supported: ```php <> class Use {...} ``` I'm really looking forward to this since it can help bring more features into PHP such as : - https://docs.hhvm.com/hack/attributes/predefined-attributes#__entrypoint - https://docs.hhvm.com/hack/attributes/predefined-attributes#__memoize - https://docs.hhvm.com/hack/attributes/predefined-attributes#__sealed Regards. --- ## #108929 — Larry Garfield — 2020-03-09T22:02:51+00:00 URL: https://externals.io/message/108907#108929 · In reply to: #108907 > 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: <> 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): <> 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: <> . 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; } } <> 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 --- ## #108930 — Larry Garfield — 2020-03-09T23:04:41+00:00 URL: https://externals.io/message/108907#108930 · In reply to: #108929 > > > […] > > 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: > > <> > 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): > > <> > 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\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; > } > } > > <> > 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. Just for kicks, I decided to see what attributes would look like in Tukio to figure out what using the API would look like. So here's a few possibilities as a test case. The original code is here: https://github.com/Crell/Tukio/blob/master/src/ProviderBuilder.php#L69 It's implementing basically the same idea as Symfony's EventSubscriberInterface, for those familiar with it. Only right now you can't set anything directly and have to use a separate callback to have non-default configuration. The intent here is to replace that separate callback with attributes. Code here: https://gist.github.com/Crell/22fd255efe389f2eaf01fc89ab1d85d9 In the single attribute version, it works pretty well but it's annoying that the various options have to be in order, and are not self-documenting. I left out the option of before/after ordering instead of priority. I'm not sure how I'd do it there other than making it 3 different annotation classes, but then I'd need to add code to enforce that you use only one of the three. In the multi-attribute version, it's decent amount more code and it has a type switch in the middle. Interestingly I did it a different way than the RFC's example. I first pulled out the attribute objects en masse, then did an if-elseif dance. The RFC example had a switch on the type property of the reflection object, and then pulled out the actual attribute object itself within each switch case. Neither one seems particularly nice to me. Finally I did a single attribute version that takes an array parameter to make it more self-documenting. That made it much easier to handle internally, and I was able to add the priority/before/after logic that would have been too annoying to do in the other examples. Overall thoughts: The reflection API is annoying, but that doesn't come from annotations. I'd love to be able to clean up those RTTI statements. Neither the one here nor the one in the RFC now is particularly pleasant. That's possibly out of scope (we are talking about an array of multiple objects of unknown concrete type, so RTTI is probably unavoidable), but I wonder if having Attributes have a base class with some built in functionality (eg, the pieces of the reflection object itself) would be useful? Essentially combining the reflection object and the attribute object so we can skip a step. Just brainstorming here. Visually as a user of the attribute, I somewhat prefer the multi-attribute version. Not completely, though. I'm still torn. The array param version seems the most flexible, but also has a lot of sigil-soup going on. And while it does give me a place to do some extra error handling, it has all the same problems that $options parameters always have. Hopefully this test case is useful and educational. I'd still love to see this get in. (If some of the examples aren't quite clear, let me know.) --Larry Garfield --- ## #108931 — Benjamin Eberlei — 2020-03-09T23:29:02+00:00 URL: https://externals.io/message/108907#108931 · In reply to: #108929 On Mon, Mar 9, 2020 at 11:03 PM Larry Garfield wrote: > > > […] > back > > […] > but > > […] > feedback. > > […] > > 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: > > <> > 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): > > <> > 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: <> > . 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": <> 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; > } > } > > <> > 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 --- ## #108932 — Mike Schinkel — 2020-03-10T01:01:57+00:00 URL: https://externals.io/message/108907#108932 · In reply to: #108907 > > > 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. I make heavy use of pseduo-attributes in both PhpDoc and using class constants, and having real attributes would be quite the boon. I do have a few concerns. 1. The syntax makes my eyes bleed. --------------- I find angle brackets extremely hard to read and fear — have trained many newbies in programming — that it will cause newbies who see PHP to think it is too complex for them to consider learning. You mention that the good symbols are taken, but why limit ourselves to symbols? Why not use words like PHP uses for other parts of the language? So instead of: <> <> <<\My\Attributes\FewArguments("foo", "bar")>> function foo() {} Why not use?: attribute SingleArgument("Hello") attribute Another\SingleArgument("World") attribute \My\Attributes\FewArguments("foo", "bar") function foo() {} If we don't want to make attribute a keyword, why not this?: function attribute SingleArgument("Hello") function attribute Another\SingleArgument("World") function attribute \My\Attributes\FewArguments("foo", "bar") function foo() {} Or?: function attributes SingleArgument("Hello"), Another\SingleArgument("World"), \My\Attributes\FewArguments("foo", "bar") function foo() {} Alternately, why not use this (which is probably the best option IMO)?: function foo() attributes SingleArgument("Hello"), Another\SingleArgument("World"), \My\Attributes\FewArguments("foo", "bar") {} 2. How do your attributes relate to traits and interfaces? --------------- I assume that anything you could do in a class you could do in a trait, but what about interfaces? Using my syntax, could we have attributes defines in an interface and then require them to be implemented? For example, interface Storable attributes StorageEngine(string $engine), DoNotTestMethodsPrefixedWith(string $prefix){ function store() attributes PermissionsRequired(string $role) {} } When implemented that might look like this: class AdminOnly() implements Storable attributes StorageEngine("mongodb"), DoNotTestMethodsPrefixedWith("_"){ function store() attributes PermissionsRequired("admin") {} ... } Other than those two concerns, I'm sold! -Mike --- ## #108933 — Matthew Brown — 2020-03-10T01:36:28+00:00 URL: https://externals.io/message/108907#108933 · In reply to: #108932 Saying "the syntax makes my eyes bleed" is slightly useless feedback. You could say "it's hard to scan", but I don't even think that's true – prepending everything << makes it easy to pick out attributes in plain text at a glance, and one can assume that IDEs will help even further. Additionally this syntax has effectively been battle-tested at scale already in PHP-like codebases – thousands of engineers at Facebook and Slack have used it over the last few years, and been productive with it. > > […] > wrote: > > […] > back > > […] > but > > […] > feedback. > > […] > > I am very excited to see this. I make heavy use of pseduo-attributes in > both PhpDoc and using class constants, and having real attributes would be > quite the boon. > > I do have a few concerns. > > 1. The syntax makes my eyes bleed. > --------------- > > I find angle brackets extremely hard to read and fear — have trained many > newbies in programming — that it will cause newbies who see PHP to think it > is too complex for them to consider learning. > > You mention that the good symbols are taken, but why limit ourselves to > symbols? Why not use words like PHP uses for other parts of the language? > > So instead of: > > <> > <> > <<\My\Attributes\FewArguments("foo", "bar")>> > function foo() {} > > Why not use?: > > attribute SingleArgument("Hello") > attribute Another\SingleArgument("World") > attribute \My\Attributes\FewArguments("foo", "bar") > function foo() {} > > If we don't want to make attribute a keyword, why not this?: > > function attribute SingleArgument("Hello") > function attribute Another\SingleArgument("World") > function attribute \My\Attributes\FewArguments("foo", "bar") > function foo() {} > > Or?: > > function attributes > SingleArgument("Hello"), > Another\SingleArgument("World"), > \My\Attributes\FewArguments("foo", "bar") > function foo() {} > > Alternately, why not use this (which is probably the best option IMO)?: > > function foo() attributes > SingleArgument("Hello"), > Another\SingleArgument("World"), > \My\Attributes\FewArguments("foo", "bar") {} > > > 2. How do your attributes relate to traits and interfaces? > --------------- > > I assume that anything you could do in a class you could do in a trait, > but what about interfaces? > > Using my syntax, could we have attributes defines in an interface and then > require them to be implemented? For example, > > > interface Storable attributes > StorageEngine(string $engine), > DoNotTestMethodsPrefixedWith(string $prefix){ > function store() attributes PermissionsRequired(string $role) {} > } > > When implemented that might look like this: > > class AdminOnly() implements Storable attributes > StorageEngine("mongodb"), > DoNotTestMethodsPrefixedWith("_"){ > function store() attributes PermissionsRequired("admin") {} > ... > } > > Other than those two concerns, I'm sold! > > -Mike --- Next page: https://externals.io/message/108907.md?page=2