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 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
pon., 9 mar 2020 o 15:45 Benjamin Eberlei kontakt@beberlei.de napisał(a):
On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei kontakt@beberlei.de
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.Obviously I am looking for feedback on the whole RFC, not just the open
issues :-)greetings
Benjamin
Hi Benjamin,
I'm really glad you're taking up the baton!
IMHO a bunch of straw polls could provide a way more beneficial knowledge
about feelings
people who can vote up for this feature. What do you think?
It could be good to discover the negative or positive feelings as well as
"I'm not opposite" feelings.
From what I can see now and I didn't think of it earlier are some details
but we should ask the question
how far we wanna go with this feature for its an initial shape.
Are we gonna provide complete attributes feature like C#, Java etc. which
allows for
annotating annotations (whatever it's called for me annotations/attributes
has no difference for now)
meaning they allow to put metadata which puts restrictions on:
- Usage of the attribute should be allowed once or multiple times
regarding the same target? - Targetting attributes should be annotated - no matter if through an
interface or annotation? - Allow for inheritance should be annotated - meaning when reading
attributes for class Bar
which extends Foo on which attributes are set, should I get attributes or
not? - Should attributes be resolvable all the time, what if the class won't
exist on runtime?
In some languages like Java there are for eg. documentary annotations
which live with code but are not stored on runtime.
Or we simply want to provide a simple way of retrieving the metadata from
class/trait/interface/properties/methods/functions without checking their
repeateness, without inheritance,
without validating the targets where they appear, and just allow to move
any kind of attributes into
the language and allow the userland to decide what to do with them and how
to handle them properly?
The thing which I haven't consider earlier and I find it nice features is
the ability
to filter annotations against some abstract/base annotation - it may be
described as a better cause
I do feel like it should be done through extending of the interface and not
by an abstract class.
Like from the RFC itself the \Validator\Attributes\Attribute::class
it
wouldn't make sense for it to be
a class, not even an abstract class, there's probably no method which this
abstract will provide.
Attributes should be forbidden for instantiation and this if funny cause
they do have a public __construct
so they look like classes but are not actually classes which you should be
allowed to instantiate wherever
you want, right? So we agree it looks a little weird here but dunno if
there's something we can do.
cheers,
Michał Brzuchalski
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
On Mon, Mar 9, 2020 at 5:18 PM Stephen Reay php-lists@koalephant.com
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
BenjaminHi 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
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 https://wiki.php.net/rfc/attributes_v2
A working patch is at https://github.com/beberlei/php-src/pull/2 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
BenjaminHi 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
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.
Why do decorator names refer to classes, rather than any callable as in python?
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.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.
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
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.comModernizing Legacy Applications in PHP
https://leanpub.com/mlaphpSolving the N+1 Problem in PHP
https://leanpub.com/sn1php
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]
On Mon, Mar 9, 2020 at 6:18 PM Rowan Tommins rowan.collins@gmail.com
wrote:
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.
Thank you, I didn't notice that before in the ToC view, fixed!
Regards,
Rowan Tommins
[IMSoP]
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
Am 09.03.2020 um 20:16 schrieb Matthew Brown:
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!
I could not have written better words :)
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
The RFC is at https://wiki.php.net/rfc/attributes_v2
Would it make sense to support this:
<<WithoutArgument>>
<<SingleArgument(0)>>
<<FewArguments('Hello', 'World')>>
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:
<<WithoutArgument; SingleArgument(0)>>
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
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 alec@alec.pl
escreveu:
The RFC is at https://wiki.php.net/rfc/attributes_v2
Would it make sense to support this:
<<WithoutArgument>>
<<SingleArgument(0)>>
<<FewArguments('Hello', 'World')>>
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:
<<WithoutArgument; SingleArgument(0)>>
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
The RFC is at https://wiki.php.net/rfc/attributes_v2
Would it make sense to support this:
<<WithoutArgument>>
<<SingleArgument(0)>>
<<FewArguments('Hello', 'World')>>
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:
<<WithoutArgument; SingleArgument(0)>>
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
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.
namespace ORM {
class Entity implements \ClassAttribute { ... }
}
namespace Foo {
use ORM;
<<ORM\Entity(...)>> // <-- Error
function bar(): void {}
}
I also question whether the following syntax would be supported:
<<ORM\Entity(...), ORM\Table(...)>>
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.
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
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
BenjaminI 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".
+1The "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.
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
On Mon, Mar 9, 2020 at 11:03 PM Larry Garfield larry@garfieldtech.com
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
BenjaminI 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". +1The "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
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.
- 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:
<<SingleArgument("Hello")>>
<<Another\SingleArgument("World")>>
<<\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") {}
- 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
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.
On Mar 9, 2020, at 10:42 AM, Benjamin Eberlei kontakt@beberlei.de
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
BenjaminI 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.
- 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:
<<SingleArgument("Hello")>>
<<Another\SingleArgument("World")>>
<<\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") {}
- 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
Saying "the syntax makes my eyes bleed" is slightly useless feedback.
That was just a header. There was elaboration right below it:
"I find angle brackets extremely hard to read and fear — having trained many newbies in programming — that it will cause newbies who see PHP to think it is too complex for them to consider learning."
And there was a proposed alternative.
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,
Not for me. Seeing the word "attributes" is far easier for me than seeing "<<" and ">>."
I do of course understand YMMV.
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.
Yeah, developers who have been vetted by two very successful tech companies.
Something tells me neither of them hire moderately-to-lower skilled developers who work in the trenches at many non high-flying tech companies, nor do they hire newbies who are trying to decide if they should learn PHP, or learn Javascript/Typescript instead (for example.)
Just registering my objection to the syntax and proposing a readable alternative. I would be highly surprised if there were not many others in userland who would feel the same. But of course I unfortunately have no way of validating that belief.
-Mike
I find angle brackets extremely hard to read
You're definitely not alone in disliking the syntax, but I think it
actually looks quite neat. It reminds me firstly of templating languages
which use {{foo}} for placeholders and/or keywords, and secondly of French
quote marks, «guillemets». It looks ugly in e-mails using variable-width
fonts, but that's not we're going to be writing code in every day.
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.
I think the concept of attributes carries a much higher risk of that than
any particular syntax. The same is true of things like generators, or
traits, which use a more verbose syntax, but take a while to grasp.
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?
I guess the same question can be asked of any sigils and operators - why do
we write "$foo->bar($a)" rather than "on foo call $bar with $a"? In the
end, there's a trade-off: too many symbols become hard to remember, and
hard to read close together; too many keywords become hard to read at a
glance, and tedious to write.
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") {}
This particular example leads to complications with how different keywords
stack up; would the return statement come before the "attributes" keyword?
if attributes were allowed on anonymous functions, would they come before
or after the use () clause? The traditional placement above the declaration
line keeps attributes out of the way of the main information about the
function.
It's also worth noting that all the other languages referenced in the RFC
use punctuation, rather than keywords, for their equivalent functionality:
C#: [Foo]
Rust: #![Foo] or #[Foo]
C++: [[Foo]]
Java: @Foo
ECMAScript (proposed): @Foo
Go: Foo
or "Foo"
Doctrine et al: /** @Foo */
Hack: <<Foo>>
That doesn't mean PHP couldn't buck the trend and use a keyword instead,
but we'd need a strong reason to do so.
Regards,
Rowan Tommins
[IMSoP]
C#: [Foo]
Rust: #![Foo] or #[Foo]
C++: [[Foo]]
Java: @Foo
ECMAScript (proposed): @Foo
Go:Foo
or "Foo"
Doctrine et al: /** @Foo */
Hack: <<Foo>>
To add to that list, Python decorators, which serve a similar purpose, also
use @Foo, and in confirming that I came upon some interesting discussions
of how they came to that syntax in https://www.python.org/dev/peps/pep-0318/
and a long list of alternative proposals at
https://wiki.python.org/moin/PythonDecorators
There was apparently an alternative proposal for a "using" keyword, but it
was ultimately rejected in favour of the punctuation form. Not all of the
considerations apply to the PHP case, but this quote from Guido is rather
interesting:
The keyword starting the line that heads a block draws a lot of attention
to it.
This is true for "if", "while", "for", "try", "def" and "class". But the
"using" keyword (or any other keyword in its place)
doesn't deserve that attention; the emphasis should be on the decorator
or decorators inside the suite,
since those are the important modifiers to the function definition that
follows.
When a function definition carries one or more decorators, the most
important information
is not the fact that it has decorators, but the specific decorators used.
I think that applies to our case equally: any punctuation or keyword is
just a separator between the main function declaration and the specific
attribute being applied. Having to write "attribute Sealed" would be like
having to write "visibility public"; as much as possible, we want "Sealed"
to be the keyword, and the rest of the syntax to just be formatting.
Regards,
Rowan Tommins
[IMSoP]
I think that applies to our case equally: any punctuation or keyword is
just a separator between the main function declaration and the specific
attribute being applied. Having to write "attribute Sealed" would be like
having to write "visibility public"; as much as possible, we want "Sealed"
to be the keyword, and the rest of the syntax to just be formatting.
No, you are making a flawed comparison.
There are only three (3) visibility modifiers in PHP whereas there are practically infinite potential attribute classes.
-Mike
On Mar 10, 2020, at 7:36 AM, Rowan Tommins rowan.collins@gmail.com
wrote:
I think that applies to our case equally: any punctuation or keyword is
just a separator between the main function declaration and the specific
attribute being applied. Having to write "attribute Sealed" would be like
having to write "visibility public"; as much as possible, we want
"Sealed"
to be the keyword, and the rest of the syntax to just be formatting.No, you are making a flawed comparison.
There are only three (3) visibility modifiers in PHP whereas there are
practically infinite potential attribute classes.
Yes, it's a slight exaggeration, I was just trying to expand on the quote
from Guido van Rossum; you can read his full response here:
https://mail.python.org/pipermail/python-dev/2004-September/048518.html
His example actually uses the infinite variety of decorators as a reason to
minimise the syntax:
A classmethod or staticmethod decorator adds a completely different
flavor than a decorator that provides an
external linkage hint for ObjC, or one that adds synchronization, or one
that declares deprecation.
Anyway, the main reason I linked the Python discussion was to see what
talking points were raised, and that was pulled out in the PEP and I
thought was interesting.
That doesn't mean PHP couldn't buck the trend and use a keyword instead,
but we'd need a strong reason to do so.
Clarity? Readability? Ease of learning?
Sure, if people agree it provides those things. I personally don't, but
that's fine, not everyone's going to agree.
Consistency with other declarations in PHP?
I think this is generally a weak argument, because we have a mixture of
punctuation and keywords already. We use { and } to denote blocks, not
begin and end keywords; we keep on adding operators because people prefer
them over function syntax; and the biggest complaint with the lambda syntax
was that people wanted to remove the "fn" keyword.
You mention properties as not having sigils, but we don't have a "property"
keyword either; we have an optional "var", but I'm not clear why that was
un-deprecated, because the syntax "visibility $name" is always sufficient.
And although we write "class Foo extends Bar" where other languages would
have "class Foo: Bar", we write "function foo(): Bar" rather than "function
foo() returns Bar", and there are people who would prefer to write "public
foo() {}" rather than "public function foo() {}".
I think it makes more sense to evaluate the syntax for this specific
case, rather than trying to compare it to other decisions we've made in
the past.
Just because other languages did it that way? That seems like it is just
a bandwagon justification, not a justification based on evaluation of
available options.
We should learn from other languages rather than blindly copying them, but
it's interesting that I've yet to see one which adopted a keyword-based
approach. That means either:
- They didn't even consider a keyword approach
- They considered a keyword approach, but rejected it for reasons that
don't apply to PHP - They considered a keyword approach, but rejected it for reasons that we
should at least consider
Before we go down the path of being the one language which does it
differently, we should at least examine which of those is the case.
Regards,
Rowan Tommins
[IMSoP]
You make it really hard to just let the discussion just stand.
Consistency with other declarations in PHP?
I think this is generally a weak argument, because we have a mixture of
punctuation and keywords already.
Saying it is a weak argument is merely opinion. You are grasping for reasons to claim that.
Yes we are talking opinion on both sides, but you are presenting your opinion as somehow more authoritative with the "weak argument" claim. To wit...
We use { and } to denote blocks, not begin and end keywords;
Begin and end are not declarations in any language. Those are statements to delineate blocks of code.
we keep on adding operators because people prefer them over function syntax; and the biggest complaint with the lambda syntax
was that people wanted to remove the "fn" keyword.
Those are closures for anonymous functions. They represent a callable value, they are not declarations of named symbols nor references to named symbols.
You mention properties as not having sigils, but we don't have a "property" keyword either; we have an optional "var", but I'm not clear why that was
un-deprecated, because the syntax "visibility $name" is always sufficient.
Pedantry notwithstanding, clearly I was speaking of "public", "private", and "protected" (as well as "var", but I know that most PHP prefer the visibility modifiers instead.).
All of those are keywords.
And although we write "class Foo extends Bar" where other languages would have "class Foo: Bar", we write "function foo(): Bar" rather than "function
foo() returns Bar"
Minimally because we need the terseness in type hints for parameter lists.
It would be inconsistent to use colon in parameters lists and then returns
in function declarations.
and there are people who would prefer to write "public foo() {}" rather than "public function foo() {}".
Makes sense where it can be unambiguous. Note that there is still a keyword there, not a sigil.
think it makes more sense to evaluate the syntax for this specific case, rather than trying to compare it to other decisions we've made in the past.
Sorry, but that feels really ironic. You point me to past decisions when it support your argument, but when it does not you suggest that we evaluate "for the specific case."
Just because other languages did it that way? That seems like it is just
a bandwagon justification, not a justification based on evaluation of
available options.We should learn from other languages rather than blindly copying them, but
it's interesting that I've yet to see one which adopted a keyword-based
approach.
This one is true. All other C-based languages appear to use @Foo, or [Foo] for C#.
But we can't use those. And since we can't it is not clear why we could not consider keywords just because all other languages had the @sign or the [] available.
That means either:
- They didn't even consider a keyword approach
- They considered a keyword approach, but rejected it for reasons that
don't apply to PHP- They considered a keyword approach, but rejected it for reasons that we
should at least considerBefore we go down the path of being the one language which does it
differently, we should at least examine which of those is the case.
Most likely because Java chose "@" so most everyone copied it. And the languages that did not use @ are generally for the more advanced developers.
Whatever the case it does not seem to me that this is the type of case where we could really go wrong no matter which we pick. Either sigils or keywords would work, it does not seem like something that needs exhaustive research.
That said, as it seems I am the only one on the list who has commented that really dislikes this syntax I will let this die if you can just let it die too because at this point we are both bikeshedding.
-Mike
P.S. Another approach would be to use at-sign+colon. e.g. "@:Foo"
Hi Mike,
You're right that we're bikeshedding rather, but to clarify a couple of
small points, and apologise on some others...
Saying it is a weak argument is merely opinion.
You are right, that was an unhelpful comment, and I apologise.
Pedantry notwithstanding, clearly I was speaking of "public", "private",
and "protected" (as well as "var", but I know that most PHP prefer the
visibility modifiers instead.).
My argument - or rather, my interpretation of Guido's argument in the
quoted Python discussion - is that those keywords are individual species of
a particular thing; they're not "empty keywords" (Guido's term) saying
"here comes an access modifier", or even "here comes a property". Indeed,
we might imagine a world where attributes had been added before we had
those keywords, and the language ended up looking like this:
<< Abstract >>
class Foo {
<< Private, Static >> int $foo = 42;
<< Public, Static >> getFoo(): int {
return self::$foo;
}
}
Note I've omitted the "function" keyword, because unlike "public" and
"static", it doesn't modify the declaration in any way, it's just mandatory
syntax which we could do without if we wanted to.
think it makes more sense to evaluate the syntax for this specific
case, rather than trying to compare it to other decisions we've made in
the past.Sorry, but that feels really ironic. You point me to past decisions when
it support your argument, but when it does not you suggest that we evaluate
"for the specific case."
I'm sorry it came across that way. What I should probably have said is that
I see the current language as inconsistent enough that it could be used to
argue for either position; my examples were intended to demonstrate
variety, rather than any particular pattern.
That said, I didn't give enough thought to your point about declarations
being different from statements; I don't find it fully convincing, but
you're right that it makes a lot of my examples irrelevant.
Before we go down the path of being the one language which does it
differently, we should at least examine which of those is the case.Most likely because Java chose "@" so most everyone copied it.
Possibly; if so, that would fall into the "didn't even consider it" case.
That clearly wasn't the case for Python, though, so there's at least one
discussion we can learn from.
And the languages that did not use @ are generally for the more advanced
developers.
I'm not sure that makes sense. C# is sometimes mocked as "Microsoft Java",
and very much targets the same use cases, so it's interesting that they
didn't copy the syntax here.
That said, as it seems I am the only one on the list who has commented
that really dislikes this syntax
You could equally say I'm the only on the list who is actively defending
it. If you look at discussions of the RFC off-list, "nice idea, horrible
syntax" is a recurring theme, so exploring alternatives is definitely
worthwhile.
Regards,
Rowan Tommins
[IMSoP]
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.I think the concept of attributes carries a much higher risk of that than
any particular syntax. The same is true of things like generators, or
traits, which use a more verbose syntax, but take a while to grasp.
But if we include attributes, then that point is moot. And orthogonal to the syntax concern.
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?I guess the same question can be asked of any sigils and operators - why do
we write "$foo->bar($a)" rather than "on foo call $bar with $a"? In the
end, there's a trade-off: too many symbols become hard to remember, and
hard to read close together; too many keywords become hard to read at a
glance, and tedious to write.
Easy. Attributes are declarations. $foo->bar($a) is a runtime statement.
Developer write at least an order of magnitude more runtime statements than declarations, so it makes sense that terseness would be a virtue for runtime statements. The same is not true for declarations.
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") {}This particular example leads to complications with how different keywords
stack up; would the return statement come before the "attributes" keyword?
How does the return statement affect this example? The return statement would be inside the braces, the attributes would be before the braces.
Are you getting confused because I wrote an empty function body? I did so because I was mirroring the RFC which used the same convention.
if attributes were allowed on anonymous functions, would they come before
or after the use () clause?
Why would we need to force an order? Let them come in either order.
(One of my biggest pet peeves about SQL is that the keywords have a defined order that you cannot change, and there is no reason they need to do that anymore with the power of today's computers and parsers.)
The traditional placement above the declaration
line keeps attributes out of the way of the main information about the
function.
Yes, and I provided several examples of using keywords that keep attributes "out of the way of the main information" too.
It's also worth noting that all the other languages referenced in the RFC
use punctuation, rather than keywords, for their equivalent functionality:C#: [Foo]
Rust: #![Foo] or #[Foo]cont
C++: [[Foo]]
Java: @Foo
ECMAScript (proposed): @Foo
Go:Foo
or "Foo"
Doctrine et al: /** @Foo */
Hack: <<Foo>>
@Foo is the only one of those that does not come across as syntax salad. And [Foo] is not horrible. But as the RFC said, those are not possible.
That leaves C++, Rust, and Hack. Not exactly languages that somebody can master without a ton of development skill.
Speaking of consistency, we don't use sigils for named classes. We don't use sigils for properties. We don't use sigils for traits. We don't use sigils for interfaces. Why should we use sigils for attributes? Just because other languages did it that way? That seems like it is just a bandwagon justification, not a justification based on evaluation of available options.
So I assert again, <<Foo>> is not a great syntax for userland PHP when we could use a keyword-based syntax.
Point of note, Go does not actually have attributes/annotations. At best they have the equivalent of structured PHPDoc:
https://stackoverflow.com/a/35205657/102699
That doesn't mean PHP couldn't buck the trend and use a keyword instead,
but we'd need a strong reason to do so.
Clarity? Readability? Ease of learning? Consistency with other declarations in PHP?
Anyway, that's about all I think I'm going to say on this subject. But if someone else has opinions on the topic, go for it.
-Mike
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") {}This particular example leads to complications with how different keywords
stack up; would the return statement come before the "attributes" keyword?How does the return statement affect this example? The return statement would be inside the braces, the attributes would be before the braces.
I think he meant return type declaration. That's why the question about
the use
clause is as well relevant.
--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com
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") {}This particular example leads to complications with how different keywords
stack up; would the return statement come before the "attributes" keyword?How does the return statement affect this example? The return statement would be inside the braces, the attributes would be before the braces.
I think he meant return type declaration. That's why the question about
theuse
clause is as well relevant.
If that is the case that makes a little more sense.
But even so, the question is surprising because we have a well established existing pattern with extends and implements clauses, for example:
function foo():returntype
extends Parent
implements Interface1, Interface2, Interface2
attributes Attribute1, Attribute2, Attribute3 {}
-Mike
I think he meant return type declaration. That's why the question about
theuse
clause is as well relevant.
Yes, that was a typo on my part, sorry.
But even so, the question is surprising because we have a well established
existing pattern with extends and implements clauses, for example:function foo():returntype
extends Parent
implements Interface1, Interface2, Interface2
attributes Attribute1, Attribute2, Attribute3 {}
I'm not sure where you're going with that - as far as I know, extends and
implements are only valid on classes, and return types are only valid on
functions, so there'd never be a mixture in one declaration.
Still, you're right that it's perfectly resolvable, it's just an extra
consideration to throw into the mix.
I do think that particular syntax would get a bit messy with more involved
declarations, though:
public static function doSomething(Action $what, int $howManyTimes):
boolean attributes Memoize, Jit, Log(LogLevel::DEBUG), Autowire(1) {
// ...
}
Maybe rules for whitespace would evolve to let that flow onto multiple
lines, but it's not quite clear how the indenting should look; like this
maybe?
public static function doSomething(Action $what, int $howManyTimes):
boolean
attributes Memoize, Jit, Log(LogLevel::DEBUG), Autowire(1) {
// ...
}
The current proposed syntax leaves the main declaration as it is now, and
visually separates the attributes at the expense of vertical whitespace:
<< Memoize>>
<<Jit>>
<<Log(LogLevel::DEBUG)>>
<<Autowire(1)>>
public static function doSomething(Action $what, int $howManyTimes):
boolean {
// ...
}
A suggestion that's come up a couple of times is to allow grouping of
attributes, so you could flatten it onto one line:
<<Memoize, Jit, Log(LogLevel::DEBUG), Autowire(1)>>
public static function doSomething(Action $what, int $howManyTimes):
boolean {
// ...
}
Or the attributes could be grouped according to some coding standard:
<<Memoize, Jit>>
<<Log(LogLevel::DEBUG), Autowire(1)>>
public static function doSomething(Action $what, int $howManyTimes):
boolean {
// ...
}
There's plenty of other possibilities we could consider, though - again,
see the Python wiki link for a whole range of proposals they had for
decorators, and we may be able to find similar lists to learn from for
other languages.
Regards,
Rowan Tommins
[IMSoP]
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") {}This particular example leads to complications with how different
keywords
stack up; would the return statement come before the "attributes"
keyword?How does the return statement affect this example? The return
statement would be inside the braces, the attributes would be before the
braces.I think he meant return type declaration. That's why the question about
theuse
clause is as well relevant.If that is the case that makes a little more sense.
But even so, the question is surprising because we have a well established
existing pattern with extends and implements clauses, for example:function foo():returntype
extends Parent
implements Interface1, Interface2, Interface2
attributes Attribute1, Attribute2, Attribute3 {}Just to make sure you don't run in circles in this discussion thread here,
even when syntax is not fixed yet, it's not going to be a syntax where the
attributes are suffixed after the declaration. It would maybe some other
characters like %[Attr].
-Mike
Just to make sure you don't run in circles in this discussion thread here, even when syntax is not fixed yet, it's not going to be a syntax where the attributes are suffixed after the declaration. It would maybe some other characters like %[Attr].
It is your RFC so you are the arbiter.
One final on syntax: Can I suggest you consider @:Attr?
-Mike
P. S. Did you see the question on interfaces?
On Mar 10, 2020, at 11:28 AM, Benjamin Eberlei kontakt@beberlei.de
wrote:Just to make sure you don't run in circles in this discussion thread
here, even when syntax is not fixed yet, it's not going to be a syntax
where the attributes are suffixed after the declaration. It would maybe
some other characters like %[Attr].It is your RFC so you are the arbiter.
One final on syntax: Can I suggest you consider @:Attr?
-Mike
P. S. Did you see the question on interfaces?
No I missed it. Attributes work on interfaces, but they are not inherited
to parent classes. This is similar to how a class implementing an interface
may have different docblocks. You can use the reflection api to get to
interfaces from a class and check for their attributes there. For traits
the attributes get copied into the using class.
I find angle brackets extremely hard to read
You're definitely not alone in disliking the syntax, but I think it
actually looks quite neat. It reminds me firstly of templating languages
which use {{foo}} for placeholders and/or keywords, and secondly of French
quote marks, «guillemets». It looks ugly in e-mails using variable-width
fonts, but that's not we're going to be writing code in every day.
I quite like the relationship to Hack. Hack borrowed so much from PHP, and PHP, in turn, borrows back from Hack.
Are spaces allowed in the brackets? If so, maybe that helps the readability?
<< A1(123), A2 >>
class C
{
<< A3(true, 100) >>
public function __construct()
{
/* ... */
}
<< A4 >>
public function doIt(): void
{
/* ... */
}
}
If spaces are allowed, I imagine that folks will adopt coding standards to aid in readability.
Cheers,
Ben
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.
namespace ORM {
class Entity implements \ClassAttribute { ... }
}
namespace Foo {
use ORM;
<<ORM\Entity(...)>> // <-- Error
function bar(): void {}
}
I also question whether the following syntax would be supported:
<<ORM\Entity(...), ORM\Table(...)>>
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.
Hi Benjamin,
Two small issues/questions:
-
It is not allowed to use the same attribute name more than once on the
same declaration and a compile error is thrown when this detected.
Is there any reason for this? IMO, the attribute can represents something
that can have multiple options. Like anAround
attribute for aspect
oriented programming for example. -
for reflection classes extended with the getAttributes() methods, I
think we need to add ReflectionMethod or change ReflectionFunction with
ReflectionFunctionAbstract.
Alex
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 everyone,
I have updated the RFC with much of the feedback received here, on Twitter
and Reddit.
https://wiki.php.net/rfc/attributes_v2
The following changes were made:
- Changed to support the same attribute multiple times on the same
declaration - Added support for attributes on method and function parameters
- Replaced PhpAttribute interface with an attribute instead
- Distinction between userland and compiler attributes and description
when each of them gets evaluated/validated - Reduce number of examples to shorten RFC a bit and expand the other
examples instead - Introduced validation of compiler attributes at compile time using a C
callback - Offer alternative syntax “@:” using new token T_ATTRIBUTE which will
be included with a secondary vote
You may have seen me mentioning that I don't want to deviate from the <<>>
syntax, a topic of heated debate. As Martin helped me tremendously with the
RFC and patches he earned to propose an alternative (including patch with
prototype). So we will have a secondary vote on syntax being either
<<Attribute>> or @:Attribute.
Let us know what you think about the changes.
greetings
Benjamin
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 everyone,
I have updated the RFC with much of the feedback received here, on Twitter
and Reddit.
Hi Benjamin,
Thanks a lot for you effort, same for Martin. I really hope to see this in PHP8.
I have a simple question. From the RFC:
Attributes are added before the declaration they belong to, similar to do-block comments. They can be declared before or after a doc-block comment that documents a declaration.
Could annotations be declared before AND after the doc-block?
What would be the behavior in that case?
Regards,
Iván Arias.
Hi everyone,
I have updated the RFC with much of the feedback received here, on
and Reddit.Hi Benjamin,
Thanks a lot for you effort, same for Martin. I really hope to see this in
PHP8.I have a simple question. From the RFC:
Attributes are added before the declaration they belong to, similar to
do-block comments. They can be declared before or after a doc-block
comment that documents a declaration.Could annotations be declared before AND after the doc-block?
What would be the behavior in that case?
Attributes are handled as a list, so you always append to them. So it is
sort of like merging the list of attributes before and after the doc
comment. See here:
https://gist.github.com/beberlei/6cf033a3d2cd1a8d991fcabdecce0626
Regards,
Iván Arias.
Hi Benjamin,
I have updated the RFC with much of the feedback received here, on Twitter
and Reddit.
Thanks for the update and for giving this a try!
I'm wondering about nested annotations: as you know, they're quite common
in apps that use doctrine/annotation.
How do you think these should be migrated to the new system?
Nicolas
On Tue, Apr 14, 2020 at 3:04 PM Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:
Hi Benjamin,
I have updated the RFC with much of the feedback received here, on Twitter
and Reddit.
Thanks for the update and for giving this a try!
I'm wondering about nested annotations: as you know, they're quite common
in apps that use doctrine/annotation.
How do you think these should be migrated to the new system?
Nested attributes are not supported in this step, because they might
interfere with potential changes to constant AST. Tyson dipped into that
topic a few weeks ago.
If constants were to allow something like const $foo = new Foo(); - then
this would automatically be available for attributes (they use the same
concept).
For now I suppose the approach would be to flatten the attributes, for
example:
@JoinTable(name="users_posts", joinColumns={@JoinColumn(name="id",
referencedColumnName="user_id"}, inverseJoinColumns={@JoinColumn(name="id",
"referencedColumnName="post_id"})
becomes:
<<JoinTable("users_posts")>>
<<JoinColumn("id", "user_id")>>
<<InverseJoinColumn("id", "post_id")>>
As you can see with this example, named arguments is another under
construction area in the language that would automatically spill its
benefits to attributes.
Think of this like scalar types in 7.0 that missed nullable, which was
added in 7.1.
Nicolas
Hi everyone,
I have updated the RFC with much of the feedback received here, on Twitter
and Reddit.https://wiki.php.net/rfc/attributes_v2
The following changes were made:
- Changed to support the same attribute multiple times on the same
declaration- Added support for attributes on method and function parameters
- Replaced PhpAttribute interface with an attribute instead
- Distinction between userland and compiler attributes and description
when each of them gets evaluated/validated- Reduce number of examples to shorten RFC a bit and expand the other
examples instead- Introduced validation of compiler attributes at compile time using a C
callback- Offer alternative syntax “@:” using new token T_ATTRIBUTE which will
be included with a secondary voteYou may have seen me mentioning that I don't want to deviate from the <<>>
syntax, a topic of heated debate. As Martin helped me tremendously with the
RFC and patches he earned to propose an alternative (including patch with
prototype). So we will have a secondary vote on syntax being either
<<Attribute>> or @:Attribute.Let us know what you think about the changes.
greetings
Benjamin
This looks lovely and I look forward to being able to use it!
Questions:
-
Why is exact-match the default for getAttributes(), and "instanceof" an extra flag? I would expect it to the other way around. The whole point of LSP is that any subclass is a viable replacement for its parent; if not, You're Doing It Wrong(tm). It also means that requesting by interface mandates adding the second parameter or else it will always return nothing. What is the reason for not making instanceof the default match and offering an EXACT opt-in mode?
-
Regarding sub-annotations, can you still do classes as parameters even if not as an annotation marker? Eg:
<<Foo(1, "B", Bar('blah'))>>
function foo()
Or is that also a no-go?
- I see the most common case for attributes being getting the object version. With the reflection API as currently described, I see two shortcomings.
A) I can't tell if an attribute has a valid object or not before trying to access it, which would presumably fail spectacularly. I believe we need a way to know if getObject() is going to return a valid value before trying to call it. I think this is a hard-requirement.
B) Related, as is getting all attributes as objects looks to be rather clunky.
$attribute_objectgs = array_filter(array_map(function(ReflectionAttribute $r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));
That's gross. :-) Can "get all the attributes that can be formed into objects" be its own operation? $obj->getAttributeObjects() or some such, that skips over non-instantiable attributes and instantiates the rest?
This isn't a requirement, but without it I predict virtually everyone using attributes is going to have to recreate the knot of code above.
Thanks again!
--Larry Garfield
On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield larry@garfieldtech.com
wrote:
Hi everyone,
I have updated the RFC with much of the feedback received here, on
and Reddit.https://wiki.php.net/rfc/attributes_v2
The following changes were made:
- Changed to support the same attribute multiple times on the same
declaration- Added support for attributes on method and function parameters
- Replaced PhpAttribute interface with an attribute instead
- Distinction between userland and compiler attributes and description
when each of them gets evaluated/validated- Reduce number of examples to shorten RFC a bit and expand the other
examples instead- Introduced validation of compiler attributes at compile time using
a C
callback- Offer alternative syntax “@:” using new token T_ATTRIBUTE which will
be included with a secondary voteYou may have seen me mentioning that I don't want to deviate from the
<<>>
syntax, a topic of heated debate. As Martin helped me tremendously with
the
RFC and patches he earned to propose an alternative (including patch with
prototype). So we will have a secondary vote on syntax being either
<<Attribute>> or @:Attribute.Let us know what you think about the changes.
greetings
BenjaminThis looks lovely and I look forward to being able to use it!
Questions:
- Why is exact-match the default for getAttributes(), and "instanceof" an
extra flag? I would expect it to the other way around. The whole point of
LSP is that any subclass is a viable replacement for its parent; if not,
You're Doing It Wrong(tm). It also means that requesting by interface
mandates adding the second parameter or else it will always return
nothing. What is the reason for not making instanceof the default match
and offering an EXACT opt-in mode?
Yes, you make a good point here.
- Regarding sub-annotations, can you still do classes as parameters even
if not as an annotation marker? Eg:<<Foo(1, "B", Bar('blah'))>>
function foo()Or is that also a no-go?
This is a no go because it would require reimplementing constant ASTs,
which is as of now 300 lines of tricky code evaluating ASTs and allowing
this would also clash with Bar("Blah") reading like a function call, which
is confusing and would prevent reconciliation with constant ASTs in the
future.
- I see the most common case for attributes being getting the object
version. With the reflection API as currently described, I see two
shortcomings.A) I can't tell if an attribute has a valid object or not before trying to
access it, which would presumably fail spectacularly. I believe we need a
way to know if getObject() is going to return a valid value before trying
to call it. I think this is a hard-requirement.B) Related, as is getting all attributes as objects looks to be rather
clunky.$attribute_objectgs = array_filter(array_map(function(ReflectionAttribute
$r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));That's gross. :-) Can "get all the attributes that can be formed into
objects" be its own operation? $obj->getAttributeObjects() or some such,
that skips over non-instantiable attributes and instantiates the rest?
I don't see A.) what would you do when the object instantiation fails? You
would throw an exception I presume, let the engine throw the regular
TypeError, ArgumentError, Error if class not exists that everyone is
already familiar with.
For B.) I believe you are extrapolating based on your own use case. Working
with Reflection is usually a lot of boilerplate, I don't believe we need to
have a one liner here.
This isn't a requirement, but without it I predict virtually everyone
using attributes is going to have to recreate the knot of code above.Thanks again!
--Larry Garfield
On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield larry@garfieldtech.com
wrote:
- Regarding sub-annotations, can you still do classes as parameters even
if not as an annotation marker? Eg:<<Foo(1, "B", Bar('blah'))>>
function foo()Or is that also a no-go?
This is a no go because it would require reimplementing constant ASTs,
which is as of now 300 lines of tricky code evaluating ASTs and allowing
this would also clash with Bar("Blah") reading like a function call, which
is confusing and would prevent reconciliation with constant ASTs in the
future.
Sad panda.
- I see the most common case for attributes being getting the object
version. With the reflection API as currently described, I see two
shortcomings.A) I can't tell if an attribute has a valid object or not before trying to
access it, which would presumably fail spectacularly. I believe we need a
way to know if getObject() is going to return a valid value before trying
to call it. I think this is a hard-requirement.B) Related, as is getting all attributes as objects looks to be rather
clunky.$attribute_objectgs = array_filter(array_map(function(ReflectionAttribute
$r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));That's gross. :-) Can "get all the attributes that can be formed into
objects" be its own operation? $obj->getAttributeObjects() or some such,
that skips over non-instantiable attributes and instantiates the rest?I don't see A.) what would you do when the object instantiation fails? You
would throw an exception I presume, let the engine throw the regular
TypeError, ArgumentError, Error if class not exists that everyone is
already familiar with.For B.) I believe you are extrapolating based on your own use case. Working
with Reflection is usually a lot of boilerplate, I don't believe we need to
have a one liner here.
It depends on the annotation, I suppose. If I'm requesting a specific annotation by name, presumably I know if it is supposed to have an associated class. If it's supposed to but it's missing, that's a legit class-not-found exception/error.
However, I'm thinking of cases where code is integrating with a 3rd party optionally, through an annotation. In that case it's a fair question of whether the class will be defined or not based on whether some other library is present.
Similarly, if a bit of code is requesting all attributes (as above) rather than just specific ones by name, it wouldn't know if a given attribute is supposed to be defined or not; as written, class-less attributes are supported.
I suppose the workaround would be class_exists($r->getName()). Weird but I guess works? It would have to be documented as a thing you should do, though, which implies to me that it could be made cleaner.
That reflection is usually clunky today (true) is to me not a compelling argument that it shouldn't be made less clunky. :-)
--Larry Garfield
On Tue, Apr 14, 2020 at 6:14 PM Larry Garfield larry@garfieldtech.com
wrote:
On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield larry@garfieldtech.com
wrote:
- Regarding sub-annotations, can you still do classes as parameters
even
if not as an annotation marker? Eg:<<Foo(1, "B", Bar('blah'))>>
function foo()Or is that also a no-go?
This is a no go because it would require reimplementing constant ASTs,
which is as of now 300 lines of tricky code evaluating ASTs and allowing
this would also clash with Bar("Blah") reading like a function call,
which
is confusing and would prevent reconciliation with constant ASTs in the
future.Sad panda.
- I see the most common case for attributes being getting the object
version. With the reflection API as currently described, I see two
shortcomings.A) I can't tell if an attribute has a valid object or not before
trying to
access it, which would presumably fail spectacularly. I believe we
need a
way to know if getObject() is going to return a valid value before
trying
to call it. I think this is a hard-requirement.B) Related, as is getting all attributes as objects looks to be rather
clunky.$attribute_objectgs =
array_filter(array_map(function(ReflectionAttribute
$r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));That's gross. :-) Can "get all the attributes that can be formed into
objects" be its own operation? $obj->getAttributeObjects() or some
such,
that skips over non-instantiable attributes and instantiates the rest?I don't see A.) what would you do when the object instantiation fails?
You
would throw an exception I presume, let the engine throw the regular
TypeError, ArgumentError, Error if class not exists that everyone is
already familiar with.For B.) I believe you are extrapolating based on your own use case.
Working
with Reflection is usually a lot of boilerplate, I don't believe we need
to
have a one liner here.It depends on the annotation, I suppose. If I'm requesting a specific
annotation by name, presumably I know if it is supposed to have an
associated class. If it's supposed to but it's missing, that's a legit
class-not-found exception/error.However, I'm thinking of cases where code is integrating with a 3rd party
optionally, through an annotation. In that case it's a fair question of
whether the class will be defined or not based on whether some other
library is present.Similarly, if a bit of code is requesting all attributes (as above) rather
than just specific ones by name, it wouldn't know if a given attribute is
supposed to be defined or not; as written, class-less attributes are
supported.I suppose the workaround would be class_exists($r->getName()). Weird but
I guess works? It would have to be documented as a thing you should do,
though, which implies to me that it could be made cleaner.That reflection is usually clunky today (true) is to me not a compelling
argument that it shouldn't be made less clunky. :-)
You are not safe from these problems when using Doctrine Annotations either
(missing library or class does not exist) and it fails exactly the same way
as trying to instantiate something that doesn't exist.
I also realized why IS_INSTANCEOF is not the default, because it needs to
resolve all attributes to classes to perform the check. This triggers
autoloading all attributes of the reflected declaration (even the ones
not requested), so we felt it should not be the default.
--Larry Garfield
- I see the most common case for attributes being getting the object
version. With the reflection API as currently described, I see two
shortcomings.A) I can't tell if an attribute has a valid object or not before
trying to
access it, which would presumably fail spectacularly. I believe we
need a
way to know if getObject() is going to return a valid value before
trying
to call it. I think this is a hard-requirement.B) Related, as is getting all attributes as objects looks to be rather
clunky.$attribute_objectgs =
array_filter(array_map(function(ReflectionAttribute
$r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));That's gross. :-) Can "get all the attributes that can be formed into
objects" be its own operation? $obj->getAttributeObjects() or some
such,
that skips over non-instantiable attributes and instantiates the rest?I don't see A.) what would you do when the object instantiation fails?
You
would throw an exception I presume, let the engine throw the regular
TypeError, ArgumentError, Error if class not exists that everyone is
already familiar with.For B.) I believe you are extrapolating based on your own use case.
Working
with Reflection is usually a lot of boilerplate, I don't believe we need
to
have a one liner here.It depends on the annotation, I suppose. If I'm requesting a specific
annotation by name, presumably I know if it is supposed to have an
associated class. If it's supposed to but it's missing, that's a legit
class-not-found exception/error.However, I'm thinking of cases where code is integrating with a 3rd party
optionally, through an annotation. In that case it's a fair question of
whether the class will be defined or not based on whether some other
library is present.Similarly, if a bit of code is requesting all attributes (as above) rather
than just specific ones by name, it wouldn't know if a given attribute is
supposed to be defined or not; as written, class-less attributes are
supported.I suppose the workaround would be class_exists($r->getName()). Weird but
I guess works? It would have to be documented as a thing you should do,
though, which implies to me that it could be made cleaner.That reflection is usually clunky today (true) is to me not a compelling
argument that it shouldn't be made less clunky. :-)You are not safe from these problems when using Doctrine Annotations either
(missing library or class does not exist) and it fails exactly the same way
as trying to instantiate something that doesn't exist.I also realized why IS_INSTANCEOF is not the default, because it needs to
resolve all attributes to classes to perform the check. This triggers
autoloading all attributes of the reflected declaration (even the ones
not requested), so we felt it should not be the default.
Ah, valid. I suppose that's an unavoidable result of allowing non-class-mapped attributes, which means anyone building on it is stuck doing their own class_exists()
check for everything.
Sad panda. (Still very +1 on the RFC, just sad panda about these details.)
--Larry Garfield
Le jeu. 16 avr. 2020 à 16:29, Larry Garfield larry@garfieldtech.com a
écrit :
- I see the most common case for attributes being getting the
object
version. With the reflection API as currently described, I see two
shortcomings.A) I can't tell if an attribute has a valid object or not before
trying to
access it, which would presumably fail spectacularly. I believe we
need a
way to know if getObject() is going to return a valid value before
trying
to call it. I think this is a hard-requirement.B) Related, as is getting all attributes as objects looks to be
rather
clunky.$attribute_objectgs =
array_filter(array_map(function(ReflectionAttribute
$r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));That's gross. :-) Can "get all the attributes that can be formed
into
objects" be its own operation? $obj->getAttributeObjects() or some
such,
that skips over non-instantiable attributes and instantiates the
rest?I don't see A.) what would you do when the object instantiation
fails?
You
would throw an exception I presume, let the engine throw the regular
TypeError, ArgumentError, Error if class not exists that everyone is
already familiar with.For B.) I believe you are extrapolating based on your own use case.
Working
with Reflection is usually a lot of boilerplate, I don't believe we
need
to
have a one liner here.It depends on the annotation, I suppose. If I'm requesting a specific
annotation by name, presumably I know if it is supposed to have an
associated class. If it's supposed to but it's missing, that's a legit
class-not-found exception/error.However, I'm thinking of cases where code is integrating with a 3rd
party
optionally, through an annotation. In that case it's a fair question
of
whether the class will be defined or not based on whether some other
library is present.Similarly, if a bit of code is requesting all attributes (as above)
rather
than just specific ones by name, it wouldn't know if a given attribute
is
supposed to be defined or not; as written, class-less attributes are
supported.I suppose the workaround would be class_exists($r->getName()). Weird
but
I guess works? It would have to be documented as a thing you should
do,
though, which implies to me that it could be made cleaner.That reflection is usually clunky today (true) is to me not a
compelling
argument that it shouldn't be made less clunky. :-)You are not safe from these problems when using Doctrine Annotations
either
(missing library or class does not exist) and it fails exactly the same
way
as trying to instantiate something that doesn't exist.I also realized why IS_INSTANCEOF is not the default, because it needs to
resolve all attributes to classes to perform the check. This triggers
autoloading all attributes of the reflected declaration (even the ones
not requested), so we felt it should not be the default.Ah, valid. I suppose that's an unavoidable result of allowing
non-class-mapped attributes, which means anyone building on it is stuck
doing their ownclass_exists()
check for everything.Sad panda. (Still very +1 on the RFC, just sad panda about these details.)
Same here, sad panda:
- we're going to miss nested structure instantly - I wish we could try
harder here. I'm actually wondering if using the syntax for object literals
would be the solution here? (we don't have any yet, but some proposals have
been mentioned recently. - having declarative statements possibly turn into failures is also
unfortunate. This makes me wonder if the attributes shouldn't be turned
into plain arrays instead? The logic to hydrate objects would then be moved
to some userland libs. Would that make sense? (I know, that conflicts with
my previous proposal, I'm just trying to explore :))
Nicolas
On Thu, Apr 16, 2020 at 4:41 PM Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:
Le jeu. 16 avr. 2020 à 16:29, Larry Garfield larry@garfieldtech.com a
écrit :
- I see the most common case for attributes being getting the
object
version. With the reflection API as currently described, I see
two
shortcomings.A) I can't tell if an attribute has a valid object or not before
trying to
access it, which would presumably fail spectacularly. I believe
we
need a
way to know if getObject() is going to return a valid value before
trying
to call it. I think this is a hard-requirement.B) Related, as is getting all attributes as objects looks to be
rather
clunky.$attribute_objectgs =
array_filter(array_map(function(ReflectionAttribute
$r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));That's gross. :-) Can "get all the attributes that can be formed
into
objects" be its own operation? $obj->getAttributeObjects() or
some
such,
that skips over non-instantiable attributes and instantiates the
rest?I don't see A.) what would you do when the object instantiation
fails?
You
would throw an exception I presume, let the engine throw the regular
TypeError, ArgumentError, Error if class not exists that everyone is
already familiar with.For B.) I believe you are extrapolating based on your own use case.
Working
with Reflection is usually a lot of boilerplate, I don't believe we
need
to
have a one liner here.It depends on the annotation, I suppose. If I'm requesting a specific
annotation by name, presumably I know if it is supposed to have an
associated class. If it's supposed to but it's missing, that's a
legit
class-not-found exception/error.However, I'm thinking of cases where code is integrating with a 3rd
party
optionally, through an annotation. In that case it's a fair question
of
whether the class will be defined or not based on whether some other
library is present.Similarly, if a bit of code is requesting all attributes (as above)
rather
than just specific ones by name, it wouldn't know if a given
attribute is
supposed to be defined or not; as written, class-less attributes are
supported.I suppose the workaround would be class_exists($r->getName()). Weird
but
I guess works? It would have to be documented as a thing you should
do,
though, which implies to me that it could be made cleaner.That reflection is usually clunky today (true) is to me not a
compelling
argument that it shouldn't be made less clunky. :-)You are not safe from these problems when using Doctrine Annotations
either
(missing library or class does not exist) and it fails exactly the same
way
as trying to instantiate something that doesn't exist.I also realized why IS_INSTANCEOF is not the default, because it needs
to
resolve all attributes to classes to perform the check. This triggers
autoloading all attributes of the reflected declaration (even the ones
not requested), so we felt it should not be the default.Ah, valid. I suppose that's an unavoidable result of allowing
non-class-mapped attributes, which means anyone building on it is stuck
doing their ownclass_exists()
check for everything.Sad panda. (Still very +1 on the RFC, just sad panda about these
details.)Same here, sad panda:
- we're going to miss nested structure instantly - I wish we could try
harder here. I'm actually wondering if using the syntax for object literals
would be the solution here? (we don't have any yet, but some proposals have
been mentioned recently.
Nested attributes are intentionally not part of this RFC, because I believe
it is better to attempt to integrate it with as many existing concepts as
possible, with the potential to benefit from future additions (named
function params, constant AST/expression improvements). Inventing a
completely new syntax will cause future problems for the language, and the
extensibility of attributes themselves.
I can't guarantee you a timeframe for any of that, as mentioned before, I
see it much like scalar types missing nullable in PHP 7. Nikita mentioned
potential improvements to constant AST in a reply to his ctor promotion
RFC, Tyson also discussed it a few weeks ago, and I plan to look into this
myself if this RFC is accepted. And then there is also the potential of the
named params RFC being revived as discussed a few days ago.
Your mention of object literal syntax is a very good example why nested is
not in this RFC. if we invent our own nested attributes syntax now,
deviating from PHPs expressions, we might never be able to integrate
general expression improvements into attributes and be left with two
syntaxes that can't benefit from each other. In addition we will be left to
maintaining both.
Hypothetically, if we had named params and object initializer in PHP (which
makes no sense, because they somewhat do the same thing), plus the ability
to evaluate more expressions, then the following would automatically be
possible in an attribute:
<<ORM\JoinTable(name => "users", joinColumns => [new ORM\JoinColumn{ name =
"id, referencedColumnName = "user_id" }])>>
Then ReflectionAttribute::getArguments() returns: ["name" => "users",
"joinColumns" => [object(ORM\JoinColumn) { "name" => "id,
"referencedColumnName" => "user_id" }]].
Hope this clarifies a bit on the RFCs approach.
- having declarative statements possibly turn into failures is also
unfortunate. This makes me wonder if the attributes shouldn't be turned
into plain arrays instead? The logic to hydrate objects would then be moved
to some userland libs. Would that make sense? (I know, that conflicts with
my previous proposal, I'm just trying to explore :))
The attributes can be turned into arrays as well, see the
ReflectionAttribute object in the RFC getName and getArguments functions.
Only ReflectionAttribute::getAsObject can turn into a failure. This is the
reason Reflection*::getAttribute does not immediately convert to objects.
This gives the most flexibility to userland, but also establishes the
convention how attributes are "mapped" to objects.
Nicolas
As there has only been minimal new discussion after the last changes to the
RFC I wanted to give a heads up that I will open the vote on Monday
afternoon.
If you have further remarks or questions about the RFC, please let me know.
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
As there has only been minimal new discussion after the last changes to the
RFC I wanted to give a heads up that I will open the vote on Monday
afternoon.If you have further remarks or questions about the RFC, please let me know.
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
Hi Benjamin,
Thanks for working on this RFC. I have a couple questions and
thoughts about the proposed syntax options.
First, the RFC says that the alternate T_ATTRIBUTE @:
token has the
downside "that it does not permit whitespace in attribute names to
allow detecting the ending of the declaration." Can you provide an
example of an attribute name containing whitespace that would be allowed
with the shift left/right tokens but not with the attribute token?
The RFC says that "Semantically the attribute declaration should be
read as instantiating a class with the attribute name and passing
arguments to the constructor." But class names can't contain spaces,
so how is it a downside for attribute names to not permit them either?
It seems like it would be a massive footgun to allow attribute names
that can't resolve to a class name, since there would be no way to
migrate them to a declared attribute class without a BC break. For this
reason, regardless of the final syntax choice I think whitespace should
not be permitted in attribute names.
Secondly, given that attribute arguments are evaluated as constant
expressions, it's far easier for me to read the T_ATTRIBUTE @:
syntax
at a glance than the syntax reusing shift left/right tokens. With the
latter my eyes tend to confuse shift left/right tokens in a constant
expression with the open/close token of an attribute (especially since
the syntax using bit shift tokens allows multiple attribute declarations
on the same line).
Consider this example using the shift left/right token syntax:
<<BitShiftExample(4 >> 1, 4 << 1)>><<OtherAttribute(4 >> 1, 4 << 1)>>
function foo() {}
To me the same thing using the T_ATTRIBUTE syntax is far more readable:
@:BitShiftExample(4 >> 1, 4 << 1)
@:OtherAttribute(4 >> 1, 4 << 1)
function foo() {}
Best regards,
Theodore
Consider this example using the shift left/right token syntax:
<<BitShiftExample(4 >> 1, 4 << 1)>><<OtherAttribute(4 >> 1, 4 << 1)>> function foo() {}
To me the same thing using the T_ATTRIBUTE syntax is far more readable:
@:BitShiftExample(4 >> 1, 4 << 1) @:OtherAttribute(4 >> 1, 4 << 1) function foo() {}
This is an interesting point, but how often are users likely to write
anything like that?
Perhaps part of the reason I'm less bothered by the angle-bracket syntax
than others is that I can count the number of times I've used bit shifts on
the fingers of one hand, so I don't read it as "reusing the left/right
shift token", I read it as "enclosed in double angle brackets".
Regards,
Rowan Tommins
[IMSoP]
On Fri, Apr 17, 2020 at 6:11 PM Theodore Brown theodorejb@outlook.com
wrote:
On Fri, Apr 17, 2020 at 5:49 AM Benjamin Eberlei kontakt@beberlei.de
wrote:As there has only been minimal new discussion after the last changes to
the
RFC I wanted to give a heads up that I will open the vote on Monday
afternoon.If you have further remarks or questions about the RFC, please let me
know.On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei kontakt@beberlei.de
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
Hi Benjamin,
Thanks for working on this RFC. I have a couple questions and
thoughts about the proposed syntax options.First, the RFC says that the alternate T_ATTRIBUTE
@:
token has the
downside "that it does not permit whitespace in attribute names to
allow detecting the ending of the declaration." Can you provide an
example of an attribute name containing whitespace that would be allowed
with the shift left/right tokens but not with the attribute token?
This is about whitespaes between token and attribute name, so "@:Foo" is
allowed but "@: Foo" is not.
Whereas with the hugging As, <<Foo>> and << Foo >> is allowed. This might
come in handy if potentially we allow grouping attributes in the future
such that:
<<
Foo("bar", 1+1),
Bar("baz", 2+2)
The RFC says that "Semantically the attribute declaration should be
read as instantiating a class with the attribute name and passing
arguments to the constructor." But class names can't contain spaces,
so how is it a downside for attribute names to not permit them either?
It seems like it would be a massive footgun to allow attribute names
that can't resolve to a class name, since there would be no way to
migrate them to a declared attribute class without a BC break. For this
reason, regardless of the final syntax choice I think whitespace should
not be permitted in attribute names.
This paragraph becomes a non-issue then, the attribute names themselves
don't have spaces in them, so they are always valid.
Secondly, given that attribute arguments are evaluated as constant
expressions, it's far easier for me to read the T_ATTRIBUTE@:
syntax
at a glance than the syntax reusing shift left/right tokens. With the
latter my eyes tend to confuse shift left/right tokens in a constant
expression with the open/close token of an attribute (especially since
the syntax using bit shift tokens allows multiple attribute declarations
on the same line).Consider this example using the shift left/right token syntax:
<<BitShiftExample(4 >> 1, 4 << 1)>><<OtherAttribute(4 >> 1, 4 << 1)>> function foo() {}
To me the same thing using the T_ATTRIBUTE syntax is far more readable:
@:BitShiftExample(4 >> 1, 4 << 1) @:OtherAttribute(4 >> 1, 4 << 1) function foo() {}
This is a personal assumption here, but I would assume 95% of developers
have never used >> or << before or for a long time and maybe 80% don't even
know what bit shift means and how it works. I would think nobody needs this
in attributes.
The syntax is a secondary vote though, so you can just vote your preference
:-)
Best regards,
Theodore
Can you provide an example of an attribute name containing whitespace
that would be allowed with the shift left/right tokens but not with the
attribute token?This is about [whitespace] between token and attribute name, so
@:Foo
is allowed but@: Foo
is not. Whereas with the hugging As,<<Foo>>
and<< Foo >>
is allowed.
Ah, that makes a lot more sense. Thanks for the clarification. I guess
allowing whitespace could also be considered a downside, since it will
lead to style guide wars about which spacing convention to use.
This is a personal assumption here, but I would assume 95% of developers
have never used >> or << before or for a long time and maybe 80% don't
even know what bit shift means and how it works. I would think nobody
needs this in attributes.
Even if bit shifts are rarely used, the fact that such syntax is valid
means there's a higher cognitive load when reading attributes to
understand whether a shift token is part of a parameter value or
delineating the start/end of an attribute declaration.
Quick, does this function have two attributes with one parameter each,
or one attribute with two parameters?
<<SomeAttribute(2 * 3 + 3)>><<Foo(4 + 5 * 2)>>
function foo() {}
How about this one?
const Foo = 2;
<<SomeAttribute(2 * (3 + 3)>>Foo, (4 + 5) * 2)>>
function foo() {}
Even if the syntax is technically unambiguous, reusing shift tokens as
attribute delineators results in symbol-heavy code which is harder to
quickly and correctly understand.
FWIW Hack is apparently moving away from the shift token attribute
syntax to one using @
(see https://github.com/facebook/hhvm/commit/3983bd2ca6b252a93d98f2bb2d7e8e89f6f004d1);
Sincerely,
Theodore
the RFC says that the alternate T_ATTRIBUTE
@:
token has the
downside "that it does not permit whitespace in attribute names to
allow detecting the ending of the declaration." Can you provide an
example of an attribute name containing whitespace that would be allowed
with the shift left/right tokens but not with the attribute token?
From what I saw in the related PRs (esp.
https://github.com/beberlei/php-src/pull/2#issuecomment-609466083),
the main reason is that if @:
were defined as a standalone token,
then the following sequence (with whitespace everywhere possible, like
in https://3v4l.org/NRYbp):
function ( @: A \ B $x )
would be ambiguous between two possible interpretations:
function ( @:A \B $x ) // A attribute, \B type declaration
function ( @:A\B $x ) // A\B attribute, untyped
equivalent to respectively:
function ( <<A>> \B $x )
function ( <<A\B>> $x )
which permit whitespace (unambiguous because delimited):
function ( << A >> \ B $x )
function ( << A \ B >> $x )
(although I've never seen code with spaces around namespace separators).
--
Guilliam Xavier
Hey Benjamin,
I haven't really followed the discussion, so sorry if anything I'm
writing has already been discussed.
Attributes / annotations are one of the two things I currently miss
most in PHP (the other being generics), so thanks for the work on
that!
There are a few points that seem odd to me, I'll start with the "Php" prefix:
<?php
namespace My\Attributes;
use PhpAttribute;
<<PhpAttribute>>
class SingleArgument
{
public $value;
public function __construct(string $value)
{
$this->value = $value;
}
}
I think this should be <<Attribute>> instead, if we go with that
syntax. However, I'd even propose another syntax, as attributes aren't
ordinary classes, I'm not sure whether they should be instantiatable
from userland and / or be able to use inheritance, especially as
constructors in PHP aren't subject to variance rules. I guess the
INSTANCE_OF filter also changes whether the attributes are autoloaded
or not?
I expect annotations to be readonly, which classes as outlined in the
RFC cannot currently enforce in PHP. A syntax similar to interfaces
might be appropriate, I'm not sure whether that has been discussed:
<?php
namespace My\Attributes;
attribute SingleArgument
{
public function value(): string;
}
Such a Java-like syntax would unfortunately only work with some kind
of named parameters.
Finally, the naming of "getAsObject" should IMO be improved,
especially if I read the RFC correctly and autoloading and constructor
invocation is performed by this method. I think "createObject" or
"toObject" might be better names.
In summary, I'd probably vote "no" on the current proposal, even if
it's one of the most missed features, because I think we can do
better, and there's only one chance.
Best,
Niklas
Am Fr., 17. Apr. 2020 um 12:49 Uhr schrieb Benjamin Eberlei
kontakt@beberlei.de:
As there has only been minimal new discussion after the last changes to the
RFC I wanted to give a heads up that I will open the vote on Monday
afternoon.If you have further remarks or questions about the RFC, please let me know.
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
Hey Benjamin,
I haven't really followed the discussion, so sorry if anything I'm
writing has already been discussed.Attributes / annotations are one of the two things I currently miss
most in PHP (the other being generics), so thanks for the work on
that!There are a few points that seem odd to me, I'll start with the "Php"
prefix:<?php namespace My\Attributes; use PhpAttribute; <<PhpAttribute>> class SingleArgument { public $value; public function __construct(string $value) { $this->value = $value; } }
I think this should be <<Attribute>> instead, if we go with that
syntax. However, I'd even propose another syntax, as attributes aren't
ordinary classes, I'm not sure whether they should be instantiatable
from userland and / or be able to use inheritance, especially as
constructors in PHP aren't subject to variance rules. I guess the
INSTANCE_OF filter also changes whether the attributes are autoloaded
or not?
<<Attribute>> would best require a namespace (PHP\Attributes) as I feel
claminig "Attribute" class in the main namespace might cause problems for
users. But there is no PHP namespace yet and proposals about this have just
gotten to the list. I have therefore looked to PhpToken from nikitas recent
RFC as an example for naming, because several contributors mentioned that
the PHP\Attributes namespace I suggested in an earlier version of the RFC
would be an instant "no" vote, because of the lack of previous decision on
this topic.
The reason these are plain PHP objects and not some special case is that
you can re-use them for configuration from different sources. Symfony
Validator is a good example where the "attributes" could map to the
validator configuration classes that are also used from XML, YAML, ...
Yes, INSTANCE_OF attempts to load each attributes class, but if an
attribute class cannot be looked up (not autoloadable) it gets skipped
without error (subject to error handling of autoloader implementation, but
for Composer it skips).
I expect annotations to be readonly, which classes as outlined in the
RFC cannot currently enforce in PHP. A syntax similar to interfaces
might be appropriate, I'm not sure whether that has been discussed:
The write-once / readonly RFC was rejected and only internal classes can
implement this behavior (see ext/dom). But userland attributes also map to
userland classes, so as you say this is not possible. However given this
RFC maps to existing concepts.
I don't see how preventing instantiation or requiring readonly in userland
produces any benefits over this described use-case in the RFC. This would
make attributes much stricter than everything else in the language, I don't
see how its fair to impose this on a new feature like Attributes, when the
language doesn't even have concepts for this strictness for regular classes
(containing more important code). Mapping to "normal" classes is the way C#
implements attributes, I don't think this should be more complex than that.
Extensions, and Third Party library authors can easily guard their
attribute classes against writes from the outside with the usual OOP
abstractions and if application authors deem their attributes to be
writable that is their business.
<?php namespace My\Attributes; attribute SingleArgument { public function value(): string; }
Such a Java-like syntax would unfortunately only work with some kind
of named parameters.
Named parameters might some day come to PHP in the future, and this is
precisely the argument for treating an attribute like a regular php class
with a constructor, because the named params syntax would look exactly the
same in attribute declarations then, increasing language consistency.
The reason I went with the C# way of mapping to a "plain class" is
simplicity. The concept of attributes is already not the easiest nut to
crack, inventing another keyword and a structure that looks like a class,
but has very different implications requires a lot of words in the
documentation and doesn't provide the easiest access to beginners.
Finally, the naming of "getAsObject" should IMO be improved,
especially if I read the RFC correctly and autoloading and constructor
invocation is performed by this method. I think "createObject" or
"toObject" might be better names.
The name getAsObject is an implementation detail in my perspective. I am
open for a better name.
In summary, I'd probably vote "no" on the current proposal, even if
it's one of the most missed features, because I think we can do
better, and there's only one chance.
Sorry to hear and I hope you reconsider after reading my reply, as I built
this RFC around extensibility in the future primarily to address a lot of
voter feedback from the last RFCs.
For example things you mentioned: named parameters, readonly properties,
constructor property promotion, PHP namespace. These topics are all
currently or recently discussed and would all automatically lead to
improvements for attributes, when they "just" map to classes and their
constructor. If we would go with special cases for Attributes, we have to
solve all of these problems a second time, and this RFC would explode in
complexity.
Best,
NiklasAm Fr., 17. Apr. 2020 um 12:49 Uhr schrieb Benjamin Eberlei
kontakt@beberlei.de:As there has only been minimal new discussion after the last changes to
the
RFC I wanted to give a heads up that I will open the vote on Monday
afternoon.If you have further remarks or questions about the RFC, please let me
know.On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei kontakt@beberlei.de
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
Hey Benjamin,
thanks for your answers!
<<Attribute>> would best require a namespace (PHP\Attributes) as I feel
claminig "Attribute" class in the main namespace might cause problems for
users.
While this is true, I don't see how Attribute is different to any
classes introduced in the "recent" versions like \Generator or \Error.
Maybe Nikita can run his analyzer to see how widespread its usage is?
But there is no PHP namespace yet and proposals about this have just
gotten to the list. I have therefore looked to PhpToken from nikitas recent
RFC as an example for naming, because several contributors mentioned that
the PHP\Attributes namespace I suggested in an earlier version of the RFC
would be an instant "no" vote, because of the lack of previous decision on
this topic.
Some prefix is pretty similar to a namespace, and I don't like a new
naming convention being part of this RFC. For PhpToken the naming
might make sense, as "Token" alone might be seen as a too generic
term.
The reason these are plain PHP objects and not some special case is that
you can re-use them for configuration from different sources. Symfony
Validator is a good example where the "attributes" could map to the
validator configuration classes that are also used from XML, YAML, ...
Sounds fine!
Yes, INSTANCE_OF attempts to load each attributes class, but if an
attribute class cannot be looked up (not autoloadable) it gets skipped
without error (subject to error handling of autoloader implementation, but
for Composer it skips).
This seems rather strange to me, but I can definitely see the value if
you're using no-dev deployments, for example. I think I'd be better to
disallow inheritance for attributes and skip the autoloading and
INSTANCE_OF parameter here.
I expect annotations to be readonly, which classes as outlined in the
RFC cannot currently enforce in PHP. A syntax similar to interfaces
might be appropriate, I'm not sure whether that has been discussed:The write-once / readonly RFC was rejected and only internal classes can
implement this behavior (see ext/dom). But userland attributes also map to
userland classes, so as you say this is not possible. However given this
RFC maps to existing concepts.
I'd be possible if the actual objects are created in core and userland
only provides the contracts (interfaces) for these attributes.
I don't see how preventing instantiation or requiring readonly in userland
produces any benefits over this described use-case in the RFC. This would
make attributes much stricter than everything else in the language, I don't
see how its fair to impose this on a new feature like Attributes, when the
language doesn't even have concepts for this strictness for regular classes
(containing more important code). Mapping to "normal" classes is the way C#
implements attributes, I don't think this should be more complex than that.
Readonly is just something I'd like to see, it's not a requirement and
I'll still vote yes if the more important points are solved.
What's more important here IMO is the restriction of inheritance,
mainly because the raw arguments are exposed via reflection and won't
be compatible between parent and child attribute in any case.
There are a few options I see to solve this:
- Require constructor compatibility for attribute classes (only other classes)
- Forbid extending attribute classes entirely
- Remove getArguments() from reflection
I think my preferred solution would be to forbid inheritance, because
that also solves the autoloading inconsistencies (given that
implemented interfaces aren't respected in
ReflectionFunction::getAttributes()
and others.
Extensions, and Third Party library authors can easily guard their
attribute classes against writes from the outside with the usual OOP
abstractions and if application authors deem their attributes to be
writable that is their business.
That's probably fine, yes, if getAsObject()
/ toObject()
/
createObject()
always returns a new object.
Named parameters might some day come to PHP in the future, and this is
precisely the argument for treating an attribute like a regular php class
with a constructor, because the named params syntax would look exactly the
same in attribute declarations then, increasing language consistency.The reason I went with the C# way of mapping to a "plain class" is
simplicity. The concept of attributes is already not the easiest nut to
crack, inventing another keyword and a structure that looks like a class,
but has very different implications requires a lot of words in the
documentation and doesn't provide the easiest access to beginners.
I guess writing your own annotations and processing them isn't a
beginners topic, but using them to attribute code definitely is.
Finally, the naming of "getAsObject" should IMO be improved,
especially if I read the RFC correctly and autoloading and constructor
invocation is performed by this method. I think "createObject" or
"toObject" might be better names.The name getAsObject is an implementation detail in my perspective. I am
open for a better name.
Do you know whether we already have something similar in core?
In summary, I'd probably vote "no" on the current proposal, even if
it's one of the most missed features, because I think we can do
better, and there's only one chance.Sorry to hear and I hope you reconsider after reading my reply, as I built
this RFC around extensibility in the future primarily to address a lot of
voter feedback from the last RFCs.For example things you mentioned: named parameters, readonly properties,
constructor property promotion, PHP namespace. These topics are all
currently or recently discussed and would all automatically lead to
improvements for attributes, when they "just" map to classes and their
constructor. If we would go with special cases for Attributes, we have to
solve all of these problems a second time, and this RFC would explode in
complexity.
I think it's fine solving these in the future. However, the naming and
autoloading behavior can't be solved in the future, so I hope we can
get some improvements in before the vote.
Best,
Niklas
Hi Niklas,
What's more important here IMO is the restriction of inheritance,
mainly because the raw arguments are exposed via reflection and won't
be compatible between parent and child attribute in any case.
Could you explain a bit more about the problem you see here? My
understanding is that the reflection API is designed to allow two usage
styles:
- Treating attributes as untyped mappings from a name to a list of
arguments (fetch all, or filter by exact name, then call getName() and
getArguments()) - Treating attributes as classes instantiated with the argument given
(filter by INSTANCE_OF, then call newInstance(), formerly called
getAsObject())
For the first style, inheritance doesn't matter, because the attribute name
is never resolved as a class anyway, it's just an identifier (it might
happen to look like one, to avoid naming collisions, in the same way XML
namespaces look like URLs). For the second style, it's up to the user to
provide the right arguments for the attribute they used, just as it would
be if they instantiated it directly with the "new" keyword.
The only case I can see where incompatible constructors would be a problem
would be if someone filtered by INSTANCE_OF, and then called getArguments()
and expected them to be consistent rather than using newInstance(), but I'm
not sure why anyone would want to do that, so a one line note in the manual
pointing out it's a bad idea would seem sufficient.
But perhaps I'm missing something more fundamental that makes you feel that
inheritance should be restricted?
Regards,
Rowan Tommins
[IMSoP]