[RFC] Attributes v2

Benjamin Eberlei

6 years ago

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

Michał Brzuchalski

6 years ago

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:

1. Usage of the attribute should be allowed once or multiple times
regarding the same target?
2. Targetting attributes should be annotated - no matter if through an
interface or annotation?
3. 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?
4. 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

Stephen Reay

6 years ago

> On 9 Mar 2020, at 21:42, 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

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

Benjamin Eberlei

6 years ago

On Mon, Mar 9, 2020 at 5:18 PM Stephen Reay <php-lists@koalephant.com>
wrote:

>
>
> > On 9 Mar 2020, at 21:42, 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
>
> Hi Benjamin,
>
> I can’t comment on the feasibility of this RFC or the patch, but I like
> the approach it takes to this ‘problem’. I can already imagine a number of
> ‘nicer’ ways to achieve similar/same results using this, so thanks for
> taking up the baton!
>
> My one small query/request is about the `Reflection*::getAttributes`
> method. Is there a philosophical and/or technological reason for why it can
> only ‘filter’ the attributes by a single given name, rather than being
> variadic and filtering against multiple names (i.e. returning those that
> match any of the given names)? I would imagine many uses (particularly
> within libraries) would end up wanting to retrieve multiple attribute types
> they’re aware of all at once, no?
>

The name filtering works when using a common base class or interface, so if
a library has a class "My\BaseAttribute" and all related attributes extend
from it, then in the code you can do:

$attributes = $reflectionClass->getAttributes(\My\BaseAttribute::class);

This would return all attributes matching.

Stephen Reay

6 years ago

> On 9 Mar 2020, at 23:27, Benjamin Eberlei <kontakt@beberlei.de> wrote:
>
>
>
> On Mon, Mar 9, 2020 at 5:18 PM Stephen Reay <php-lists@koalephant.com <mailto:php-lists@koalephant.com>> wrote:
>
>
> > On 9 Mar 2020, at 21:42, Benjamin Eberlei <kontakt@beberlei.de <mailto: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 <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
> > 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?
>
> 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

Robert Hickman

6 years ago

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

Benjamin Eberlei

6 years ago

On Mon, Mar 9, 2020 at 8:29 PM Robert Hickman <robehickman@gmail.com> wrote:

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

Paul M. Jones

6 years ago

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

Benjamin Eberlei

6 years ago

On Mon, Mar 9, 2020 at 5:34 PM Paul M. Jones <pmjones@pmjones.io> wrote:

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

Rowan Collins

6 years ago

Hi Benjamin

On Mon, 9 Mar 2020 at 14:43, Benjamin Eberlei <kontakt@beberlei.de> 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
>

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]

Benjamin Eberlei

6 years ago

On Mon, Mar 9, 2020 at 6:18 PM Rowan Tommins <rowan.collins@gmail.com>
wrote:

> Hi Benjamin
>
> On Mon, 9 Mar 2020 at 14:43, Benjamin Eberlei <kontakt@beberlei.de> 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
> >
>
>
> 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!

Matthew Brown

6 years ago

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!

On Mon, 9 Mar 2020 at 10:43, Benjamin Eberlei <kontakt@beberlei.de> wrote:

Sebastian Bergmann

6 years ago

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

Andrew Faulds

6 years ago

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

A.L.E.C

6 years ago

On 09.03.2020 15:42, Benjamin Eberlei wrote:

> 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

David Rodrigues

6 years ago

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:

Benjamin Eberlei

6 years ago

On Mon, Mar 9, 2020 at 8:35 PM Aleksander Machniak <alec@alec.pl> wrote:

> On 09.03.2020 15:42, Benjamin Eberlei wrote:
> > 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.

azjezz

6 years ago

Really excited to see the discussion about this feature being brought up again, thanks for working on this :)

One thing I would suggest is to add a base interface `Attribute` and multiple sub-interfaces such as `ClassAttribute`, `MethodAttribute`, and `PropertyAttribute` just like hack lang ( https://docs.hhvm.com/search?term=HH%20Attribute )

This allows one to declare attributes that can only be used with classes ( e.g `ORM\Entity` ), so PHP and check against them at runtime and throws an error when an attribute is used in the wrong place.

```php

namespace ORM {

class Entity implements \ClassAttribute { ... }

}

namespace Foo {
use ORM;

<<ORM\Entity(...)>>  // <-- Error
function bar(): void {}
}
```

I also question whether the following syntax would be supported:

```php
<<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.

Larry Garfield

6 years ago

On Mon, Mar 9, 2020, at 9:42 AM, Benjamin Eberlei wrote:

> Hi all,
>
> I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> 2016 with a few changes, incorporating feedback from the mailing list back
> then and from talking to previous no voters.
>
> The RFC is at https://wiki.php.net/rfc/attributes_v2
>
> A working patch is at https://github.com/beberlei/php-src/pull/2 though
> work around the details is still necessary.
>
> The RFC contains a section with common criticism and objections to
> attributes, and I hope to have collected and responded to a good amount
> already from previous discussions.
>
> There is also a fair amount of implementation detail still up for debate,
> which is noted in "Open Issues". I have pre-committed to one approach, but
> listed alternatives there. On these issues I am looking for your feedback.
>
> greetings
> Benjamin

I am very excited to see this back, and overall I like the approach!

Thoughts in no special order:

* It's not entirely clear, but the values in an argument-carrying attribute are passed to its constructor variadically?  Viz:

<<Foo(1, 2, 3)>>
function stuff() {}

class Foo implements Attribute {
public function __construct(int $first, int $second, int $third) { ... }
}

Yes?  (And then if you wanted to capture them all you could use a ...$args parameter in the constructor.)

* It looks like the only way to support named parameters would be to pass an associative array and decompose it yourself.  That's... better than nothing I guess but also we're back to the same lack of self-documentation we always lament.  Is there any opportunity to do named parameters in some other way?

* Is it possible to inline a sub-object, or no?  Viz, is this legal, or would we want it to be legal (I'm assuming it's not at the moment):

<<Foo('a', 'b', Bar('baz'))>>
function stuff() {}

* I *really* dig how you can filter annotations to just those that match a certain parent class/interface.  That was one of the nicer parts of PSR-14 so I'm really glad to see the same "make the type system earn its keep" approach here.  It encourages packages to interface-tag their annotations so they can easily grab "just theirs".  +1

* The "more complex Doctrine ORM use-case" example includes inlining multiple annotations together: <<ORM\Id, ORM\Column, ORM\GeneratedValue>> .  Is that just an oops since that possibility was only just added to the Open Issues?

Regarding open issues:

* If constant expressions are easy enough to support, I don't see a reason not to support them.

* I'm torn on the marker interface.  On the one hand, a marker interface would make it easier to audit a package for all of its attributes; just grep for "implements Attribute".  I'm sad that we didn't get a marker interface in PSR-14 for that reason.  On the other, if it doesn't do anything except be a marker then it doesn't really offer anything else; and it also potentially makes it harder to add some special casing of attributes in the future, say to offer "if you want this extra attribute behavior, use this interface/base class/whatever."  I could go either way here.

* Letting attributes say where they're legal would be a good case for such a more-than-marker interface.

* The aforementioned example of specifying where an attribute is legal implies that attribute classes can themselves have attributes.  True?  If so, that should be made explicit elsewhere in the RFC.  I'm not against attribute-ception, but that should be explicit.

I think my only significant pushback at the moment is that attributes here suffer from the same redundancy problem as any other value object.  To wit:

class Owner implements Attribute {

public readonly string $name;

public readonly string $title;

public readonly int $age;

public function __construct(string $name, string $title, int $age) {
$this->name = $name;
$this->title = $title;
$this->age = $age;
}
}

<<Owner('Larry', 'Manager', 99)>>
function stuff() {}

That necessitates the same quadrupal-repetition that other constructors have.  Since I anticipate the 99% use case to be exactly that, the pain of that redundancy (and the mandatory unnamed order) will be felt rather dramatically here and thus hurts ergonomics.  Is there no way that we address that?

I realize the answer may be "no more than any other class, they all suck equally".  Are we at least certain then that if we can solve that in the future for classes generally that it will automatically work here?  (Eg, if new Owner({name: 'Larry', 'age' => 99, 'title' => 'Manager'}); became a legal shorthand for the above, it would automatically work for annotations as well.)

Overall nice work, and I hope to get to use it.

--Larry Garfield

Larry Garfield

6 years ago

On Mon, Mar 9, 2020, at 5:02 PM, Larry Garfield wrote:

> On Mon, Mar 9, 2020, at 9:42 AM, Benjamin Eberlei wrote:
> > Hi all,
> >
> > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > 2016 with a few changes, incorporating feedback from the mailing list back
> > then and from talking to previous no voters.
> >
> > The RFC is at https://wiki.php.net/rfc/attributes_v2
> >
> > A working patch is at https://github.com/beberlei/php-src/pull/2 though
> > work around the details is still necessary.
> >
> > The RFC contains a section with common criticism and objections to
> > attributes, and I hope to have collected and responded to a good amount
> > already from previous discussions.
> >
> > There is also a fair amount of implementation detail still up for debate,
> > which is noted in "Open Issues". I have pre-committed to one approach, but
> > listed alternatives there. On these issues I am looking for your feedback.
> >
> > greetings
> > Benjamin
>
> I am very excited to see this back, and overall I like the approach!
>
> Thoughts in no special order:
>
> * It's not entirely clear, but the values in an argument-carrying
> attribute are passed to its constructor variadically?  Viz:
>
> <<Foo(1, 2, 3)>>
> function stuff() {}
>
> class Foo implements Attribute {
>   public function __construct(int $first, int $second, int $third) { ... }
> }
>
> Yes?  (And then if you wanted to capture them all you could use a
> ...$args parameter in the constructor.)
>
> * It looks like the only way to support named parameters would be to
> pass an associative array and decompose it yourself.  That's... better
> than nothing I guess but also we're back to the same lack of
> self-documentation we always lament.  Is there any opportunity to do
> named parameters in some other way?
>
> * 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.

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

Benjamin Eberlei

6 years ago

On Mon, Mar 9, 2020 at 11:03 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> On Mon, Mar 9, 2020, at 9:42 AM, Benjamin Eberlei wrote:
> > Hi all,
> >
> > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > 2016 with a few changes, incorporating feedback from the mailing list
> back
> > then and from talking to previous no voters.
> >
> > The RFC is at https://wiki.php.net/rfc/attributes_v2
> >
> > A working patch is at https://github.com/beberlei/php-src/pull/2 though
> > work around the details is still necessary.
> >
> > The RFC contains a section with common criticism and objections to
> > attributes, and I hope to have collected and responded to a good amount
> > already from previous discussions.
> >
> > There is also a fair amount of implementation detail still up for debate,
> > which is noted in "Open Issues". I have pre-committed to one approach,
> but
> > listed alternatives there. On these issues I am looking for your
> feedback.
> >
> > greetings
> > Benjamin
>
> I am very excited to see this back, and overall I like the approach!
>
> Thoughts in no special order:
>
> * It's not entirely clear, but the values in an argument-carrying
> attribute are passed to its constructor variadically?  Viz:
>
> <<Foo(1, 2, 3)>>
> function stuff() {}
>
> class Foo implements Attribute {
>   public function __construct(int $first, int $second, int $third) { ... }
> }
>
> Yes?  (And then if you wanted to capture them all you could use a ...$args
> parameter in the constructor.)
>
> * It looks like the only way to support named parameters would be to pass
> an associative array and decompose it yourself.  That's... better than
> nothing I guess but also we're back to the same lack of self-documentation
> we always lament.  Is there any opportunity to do named parameters in some
> other way?
>

Improvements here are contingent on any named parameters support, which
could

>
> * Is it possible to inline a sub-object, or no?  Viz, is this legal, or
> would we want it to be legal (I'm assuming it's not at the moment):
>
> <<Foo('a', 'b', Bar('baz'))>>
> function stuff() {}
>

No thats not possible, because the constant expression syntax doesn't allow
this. See Tysons proposal lately to allow function calls in constant
expressions: https://externals.io/message/108630

>
> * I *really* dig how you can filter annotations to just those that match a
> certain parent class/interface.  That was one of the nicer parts of PSR-14
> so I'm really glad to see the same "make the type system earn its keep"
> approach here.  It encourages packages to interface-tag their annotations
> so they can easily grab "just theirs".  +1
>
> * The "more complex Doctrine ORM use-case" example includes inlining
> multiple annotations together: <<ORM\Id, ORM\Column, ORM\GeneratedValue>>
> .  Is that just an oops since that possibility was only just added to the
> Open Issues?
>
> Ah this is a mistake, I removed that use-case and settled on a single

syntax.

>
> Regarding open issues:
>
> * If constant expressions are easy enough to support, I don't see a reason
> not to support them.
>
> * I'm torn on the marker interface.  On the one hand, a marker interface
> would make it easier to audit a package for all of its attributes; just
> grep for "implements Attribute".  I'm sad that we didn't get a marker
> interface in PSR-14 for that reason.  On the other, if it doesn't do
> anything except be a marker then it doesn't really offer anything else; and
> it also potentially makes it harder to add some special casing of
> attributes in the future, say to offer "if you want this extra attribute
> behavior, use this interface/base class/whatever."  I could go either way
> here.
>
> * Letting attributes say where they're legal would be a good case for such
> a more-than-marker interface.
>

I am not sure. This is something PHP would validate at
Reflection::getAttribute() time, at which time it is also easy to validate
for userland implementations.

>
> * The aforementioned example of specifying where an attribute is legal
> implies that attribute classes can themselves have attributes.  True?  If
> so, that should be made explicit elsewhere in the RFC.  I'm not against
> attribute-ception, but that should be explicit.
>
> Attributes are just PHP classes, so they can indeed have their own

attributes. So a userland library building on top of attributes, could call
getAttributes() then validate that each attribute returned has itself an
attribute that "marks the target":

<<Target(["class"])>>
class Entity
{
}

> I think my only significant pushback at the moment is that attributes here
> suffer from the same redundancy problem as any other value object.  To wit:
>
> class Owner implements Attribute {
>
>   public readonly string $name;
>
>   public readonly string $title;
>
>   public readonly int $age;
>
>   public function __construct(string $name, string $title, int $age) {
>     $this->name = $name;
>     $this->title = $title;
>     $this->age = $age;
>   }
> }
>
> <<Owner('Larry', 'Manager', 99)>>
> function stuff() {}
>
> That necessitates the same quadrupal-repetition that other constructors
> have.  Since I anticipate the 99% use case to be exactly that, the pain of
> that redundancy (and the mandatory unnamed order) will be felt rather
> dramatically here and thus hurts ergonomics.  Is there no way that we
> address that?
>

This is not something to fix for this RFC, but any solution here and
attributes can benefit from it as they are just PHP classes.

Mike Schinkel

6 years ago

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

I am very excited to see this. I make heavy use of pseduo-attributes in both PhpDoc and using class constants, and having real attributes would be quite the boon.

I do have a few concerns.

1. The syntax makes my eyes bleed.
---------------

I find angle brackets extremely hard to read and fear — have trained many newbies in programming — that it will cause newbies who see PHP to think it is too complex for them to consider learning.

You mention that the good symbols are taken, but why limit ourselves to symbols?  Why not use words like PHP uses for other parts of the language?

So instead of:

<<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") {}


2. How do your attributes relate to traits and interfaces?
---------------

I assume that anything you could do in a class you could do in a trait, but what about interfaces?

Using my syntax, could we have attributes defines in an interface and then require them to be implemented?  For example,


interface Storable attributes
StorageEngine(string $engine),
DoNotTestMethodsPrefixedWith(string $prefix){
function store() attributes PermissionsRequired(string $role) {}
}

When implemented that might look like this:

class AdminOnly() implements Storable attributes
StorageEngine("mongodb"),
DoNotTestMethodsPrefixedWith("_"){
function store() attributes PermissionsRequired("admin") {}
...
}

Other than those two concerns, I'm sold!

-Mike

Matthew Brown

6 years ago

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 Mon, 9 Mar 2020 at 21:02, Mike Schinkel <mike@newclarity.net> wrote:

Mike Schinkel

6 years ago

> On Mar 9, 2020, at 9:36 PM, Matthew Brown <matthewmatthew@gmail.com> wrote:
>
> 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

Matthew Brown

6 years ago

I would not expect someone who’s never written a line of code to understand attributes/decorators, just as I wouldn’t expect them to understand traits, interfaces, or really any of the things one should know about in order to write and maintain production-ready PHP code.

If you want someone to get excited about programming, PHP is probably not the best language for them to start with. There are a ton of great interactive online tools (e.g. Glitch, CodePen) that make it easy to get started without having to worry about installing PHP or mastering the language’s syntax.

Mike Schinkel

6 years ago

> On Mar 10, 2020, at 12:24 AM, Matthew Brown <matthewmatthew@gmail.com> wrote:
>
> I would not expect someone who’s never written a line of code to understand attributes/decorators, just as I wouldn’t expect them to understand traits, interfaces, or really any of the things one should know about in order to write and maintain production-ready PHP code.

Of course not.  But that does not mean we should embrace more obtuse approaches when there are options to consider that are more approachable.

As for traits, interfaces, etc. people may not understand them but with so much open code available to learn from it does not mean they will not be exposed to them when trying to learn code in PHP. And using keywords that can help aid in comprehension and memory devices has makes more sense than to use magic sigils that a reader will have to translate into meaning.

> If you want someone to get excited about programming, PHP is probably not the best language for them to start with. There are a ton of great interactive online tools (e.g. Glitch, CodePen) that make it easy to get started without having to worry about installing PHP or mastering the language’s syntax.

Your comment ignores that are large percentage of PHP developers come to PHP through WordPress, and (almost?) none of them are seeking out Glitch, CodePen, etc. because they are not on most WordPress people's radar.

Be aware that the leader of WordPress has preached "Learn Javascript deeply" a few years ago, and many in the WordPress community are now ignoring PHP and viewing that the majority of new innovation and development should be in Javascript; a direction I find very painful.

Rather than make PHP even more obtuse to would-be programmers, I would like to see it become as approachable as possible, without of course making it less capable for advanced programmers.  Using keywords instead of symbol certainly seems to satisfy both goals.

Now that I stated my view and you have stated yours, let us please put this to rest and let others discuss and/or make their own choices on which approach makes sense. Please let us not continue to bikeshed this topic ala https://xkcd.com/386/

-Mike

Rowan Collins

6 years ago

On Tue, 10 Mar 2020 at 01:02, Mike Schinkel <mike@newclarity.net> wrote:

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

Rowan Collins

6 years ago

On Tue, 10 Mar 2020 at 10:54, Rowan Tommins <rowan.collins@gmail.com> wrote:

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

Mike Schinkel

6 years ago

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

-Mike

Rowan Collins

6 years ago

On Tue, 10 Mar 2020 at 11:46, Mike Schinkel <mike@newclarity.net> wrote:

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

Mike Schinkel

6 years ago

> On Mar 10, 2020, at 8:33 AM, Rowan Tommins <rowan.collins@gmail.com> wrote:

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

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"

Rowan Collins

6 years ago

Hi Mike,

You're right that we're bikeshedding rather, but to clarify a couple of
small points, and apologise on some others...

On Tue, 10 Mar 2020 at 16:27, Mike Schinkel <mike@newclarity.net> wrote:

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

Mike Schinkel

6 years ago

> On Mar 10, 2020, at 6:54 AM, Rowan Tommins <rowan.collins@gmail.com> wrote:
>> 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

A.L.E.C

6 years ago

On 10.03.2020 12:43, Mike Schinkel wrote:

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

Mike Schinkel

6 years ago

> On Mar 10, 2020, at 7:53 AM, Aleksander Machniak <alec@alec.pl> wrote:
>
> On 10.03.2020 12:43, Mike Schinkel wrote:
>>>> 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.

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

Rowan Collins

6 years ago

On Tue, 10 Mar 2020 at 15:04, Mike Schinkel <mike@newclarity.net> wrote:

> > I think he meant return type declaration. That's why the question about
> > the `use` 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]

Benjamin Eberlei

6 years ago

On Tue, Mar 10, 2020 at 4:04 PM Mike Schinkel <mike@newclarity.net> wrote:

> > On Mar 10, 2020, at 7:53 AM, Aleksander Machniak <alec@alec.pl> wrote:
> >
> > On 10.03.2020 12:43, Mike Schinkel wrote:
> >>>> 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.
>
>
> 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 Schinkel

6 years ago

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

Benjamin Eberlei

6 years ago

On Tue, Mar 10, 2020 at 5:30 PM Mike Schinkel <mike@newclarity.net> wrote:

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

Mike Schinkel

6 years ago

>
> On Mar 10, 2020 at 3:49 PM,  <Benjamin Eberlei (mailto:kontakt@beberlei.de)>  wrote:
>
>
>
>
>
> On Tue, Mar 10, 2020 at 5:30 PM Mike Schinkel  <mike@newclarity.net (mailto:mike@newclarity.net)>  wrote:
>
> >   >  On Mar 10, 2020, at 11:28 AM, Benjamin Eberlei  <kontakt@beberlei.de (mailto: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 think you may have misunderstood my question.    I was asking if using an interface we could *require* a class to implement specific attributes. If yes that would be extremely valuable.

-Mike

Ben Ramsey

6 years ago

> On Mar 10, 2020, at 05:54, Rowan Tommins <rowan.collins@gmail.com> wrote:
>
> On Tue, 10 Mar 2020 at 01:02, Mike Schinkel <mike@newclarity.net> wrote:
>
>>
>> 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

azjezz

6 years ago

Really excited to see the discussion about this feature being brought up again, thanks for working on this :)

One thing I would suggest is to add a base interface `Attribute` and multiple sub-interfaces such as `ClassAttribute`, `MethodAttribute`, and `PropertyAttribute` just like hack lang ( https://docs.hhvm.com/search?term=HH%20Attribute )

This allows one to declare attributes that can only be used with classes ( e.g `ORM\Entity` ), so PHP and check against them at runtime and throws an error when an attribute is used in the wrong place.

```php

namespace ORM {

class Entity implements \ClassAttribute { ... }

}

namespace Foo {
use ORM;

<<ORM\Entity(...)>>  // <-- Error
function bar(): void {}
}
```

I also question whether the following syntax would be supported:

```php
<<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.

Alexandru Pătrănescu

6 years ago

Hi Benjamin,

Two small issues/questions:

1. 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 an `Around` attribute for aspect
oriented programming for example.

2. for reflection classes extended with the getAttributes() methods, I
think we need to add ReflectionMethod or change ReflectionFunction with
ReflectionFunctionAbstract.

Alex

On Mon, Mar 9, 2020 at 4:43 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:

Benjamin Eberlei

6 years ago

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


On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:

Iván Arias

6 years ago

> 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

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.

Benjamin Eberlei

6 years ago

On Tue, Apr 14, 2020 at 2:56 PM Iván Arias <txigreman@hotmail.com> wrote:

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

Nicolas Grekas

6 years ago

Hi Benjamin,

I have updated the RFC with much of the feedback received here, on Twitter

> and Reddit.
>
> https://wiki.php.net/rfc/attributes_v2

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

Benjamin Eberlei

6 years ago

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.
>>
>> https://wiki.php.net/rfc/attributes_v2
>
>
> 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.

Larry Garfield

6 years ago

On Tue, Apr 14, 2020, at 6:00 AM, Benjamin Eberlei wrote:

> 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

This looks lovely and I look forward to being able to use it!

Questions:

1. 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?

2. 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?

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

Benjamin Eberlei

6 years ago

On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> On Tue, Apr 14, 2020, at 6:00 AM, Benjamin Eberlei wrote:
> > 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
>
> This looks lovely and I look forward to being able to use it!
>
> Questions:
>
> 1. 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.

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

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

Larry Garfield

6 years ago

On Tue, Apr 14, 2020, at 10:42 AM, Benjamin Eberlei wrote:

> On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield <larry@garfieldtech.com>
> wrote:

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

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

Benjamin Eberlei

6 years ago

On Tue, Apr 14, 2020 at 6:14 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> On Tue, Apr 14, 2020, at 10:42 AM, Benjamin Eberlei wrote:
> > On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield <larry@garfieldtech.com>
> > wrote:
>
> > > 2. 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.
>
> > > 3. 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

6 years ago

On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:

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

Nicolas Grekas

6 years ago

Le jeu. 16 avr. 2020 à 16:29, Larry Garfield <larry@garfieldtech.com> a
écrit :

> On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:
>
> > > > > 3. 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.)
>

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

Benjamin Eberlei

6 years ago

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 :
>
>> On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:
>>
>> > > > > 3. 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.)
>>
>
>
> 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.

Benjamin Eberlei

6 years ago

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:

Theodore Brown

6 years ago

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?

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:

```php
<<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:

```php
@:BitShiftExample(4 >> 1, 4 << 1)
@:OtherAttribute(4 >> 1, 4 << 1)
function foo() {}
```

Best regards,
Theodore

Rowan Collins

6 years ago

On Fri, 17 Apr 2020 at 17:11, Theodore Brown <theodorejb@outlook.com> wrote:

> Consider this example using the shift left/right token syntax:
>
> ```php
> <<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:
>
> ```php
> @: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]

Benjamin Eberlei

6 years ago

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:
>
> ```php
> <<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:
>
> ```php
> @: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
:-)

Theodore Brown

6 years ago

On Fri, Apr 17, 2020 at 12:43 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:

> On Fri, Apr 17, 2020 at 6:11 PM Theodore Brown <theodorejb@outlook.com> wrote:
> 
> > 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?

```php
<<SomeAttribute(2 * 3 + 3)>><<Foo(4 + 5 * 2)>>
function foo() {}
```

How about this one?

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

Guilliam Xavier

6 years ago

On Fri, Apr 17, 2020 at 6:11 PM Theodore Brown <theodorejb@outlook.com> wrote:

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

Niklas Keller

6 years ago

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

Benjamin Eberlei

6 years ago

On Sun, Apr 19, 2020 at 11:30 PM Niklas Keller <me@kelunik.com> wrote:

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

Niklas Keller

6 years ago

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

Rowan Collins

6 years ago

Hi Niklas,

On Mon, 20 Apr 2020 at 07:48, Niklas Keller <me@kelunik.com> wrote:

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