# List of attributes

- Thread: https://externals.io/message/111936
- Original archive: https://news-web.php.net/php.internals/111936
- Started by Nicolas Grekas on 2020-09-27T08:22:48+00:00
- Messages: 25
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #111936 — Nicolas Grekas — 2020-09-27T08:22:48+00:00

URL: https://externals.io/message/111936#111936

Hi Benjamin, hi everyone

I'm wondering if the syntax that allows for several attributes is really
future-proof when considering nested attributes:


*1.*
#[foo]
#[bar]

VS


*2.*
#[foo, bar]

Add nested attributes to the mix, here are two possible ways:


*A.*
#[foo(
    #[bar]
)]

or


*B.*
#[foo(
    bar
)]

The A. syntax is consistent with the 1. list.
I feel like syntax B is not desired and could be confusing from a grammar
pov.
BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
syntax B is consistent with 2.

Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
attributes are introduced?

I voted yes for syntax 2. when the attributes were using << >>. I would
vote NO now with the new syntax.

Nicolas

---

## #111937 — Rowan Tommins — 2020-09-27T10:15:28+00:00

URL: https://externals.io/message/111936#111937 · In reply to: #111936

>The A. syntax is consistent with the 1. list.
>I feel like syntax B is not desired and could be confusing from a
>grammar
>pov.
>BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
>syntax B is consistent with 2.


I think it's fairly straightforward to see #[...] as meaning "list of attributes", rather than as "change meaning of all identifiers recursively". So, this will look for a constant "MAGIC", not a nested attribute:

#[Foo(true, 42), Bar(MAGIC)] class X ...


Nested attributes are a slightly confusing concept anyway, because "normal" attributes are always attached to something, but nested attributes occur in place of a normal value. I can see their usefulness, but having the extra #[...] to say "not a normal value, extra magic happening here" seems sensible.


Presumably we could also allow grouping of nested attributes such that this:

#[Foo(#[Bar, Baz]) class X ...

Would just be equivalent to this:

#[Foo([ #[Bar], #[Baz] ]) class X ...

Regards,

-- 
Rowan Tommins
[IMSoP]

---

## #111938 — Benjamin Eberlei — 2020-09-27T10:56:33+00:00

URL: https://externals.io/message/111936#111938 · In reply to: #111936

On Sun, Sep 27, 2020 at 10:23 AM Nicolas Grekas <nicolas.grekas@gmail.com>
wrote:

> Hi Benjamin, hi everyone
>
> I'm wondering if the syntax that allows for several attributes is really
> future-proof when considering nested attributes:
>

I feel this question is what an RFC for nested attributes has to weigh. We
have established that in practice nested is possible syntax wise.
Personally I am not interested in introducing nested attributes, because
they would allow a complexity in attribute usage that might be better
handled by actual configuration files (XML, JSON, YAML, INI choose your
poison).

As such I am not looking into working on this addition myself and haven't
put any thought into the actual syntax.

In general we have not established yet if the support for nested attributes
would get a super majority of voters and its increased complexity is the
major reason why I opted to leave it out of the original RFC.

For Doctrine ORM for example I prototyped this new attribute based driver
that flattens the structure so that nested attributes are not needed
https://github.com/doctrine/orm/pull/8266 - without making the usage less
clear (in my opinion).

>
>
> *1.*
> #[foo]
> #[bar]
>
> VS
>
>
> *2.*
> #[foo, bar]
>
> Add nested attributes to the mix, here are two possible ways:
>
>
> *A.*
> #[foo(
>     #[bar]
> )]
>
> or
>
>
> *B.*
> #[foo(
>     bar
> )]
>

I'll throw a few more options into the mix

- #[foo(bar())]
- #[foo(#bar())]
- #[foo(new bar())]


> The A. syntax is consistent with the 1. list.
> I feel like syntax B is not desired and could be confusing from a grammar
> pov.
> BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
> syntax B is consistent with 2.
>
> Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
> attributes are introduced?
>
> I voted yes for syntax 2. when the attributes were using << >>. I would
> vote NO now with the new syntax.
>
> Nicolas

---

## #111939 — Derick Rethans — 2020-09-27T22:42:11+00:00

URL: https://externals.io/message/111936#111939 · In reply to: #111936

>Hi Benjamin, hi everyone
>
>I'm wondering if the syntax that allows for several attributes is
>really
>future-proof when considering nested attributes:
>
>
>*1.*
>#[foo]
>#[bar]
>
>VS
>
>
>*2.*
>#[foo, bar]

<snip>

>Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
>attributes are introduced?

No. You're about 2 months too late for that.

I'm also not in favour of nested attributes. 

cheers,
Derick

---

## #111941 — Nikita Popov — 2020-09-28T10:24:26+00:00

URL: https://externals.io/message/111936#111941 · In reply to: #111936

On Sun, Sep 27, 2020 at 10:23 AM Nicolas Grekas <nicolas.grekas@gmail.com>
wrote:

> Hi Benjamin, hi everyone
>
> I'm wondering if the syntax that allows for several attributes is really
> future-proof when considering nested attributes:
>
>
> *1.*
> #[foo]
> #[bar]
>
> VS
>
>
> *2.*
> #[foo, bar]
>
> Add nested attributes to the mix, here are two possible ways:
>
>
> *A.*
> #[foo(
>     #[bar]
> )]
>
> or
>
>
> *B.*
> #[foo(
>     bar
> )]
>
> The A. syntax is consistent with the 1. list.
> I feel like syntax B is not desired and could be confusing from a grammar
> pov.
> BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
> syntax B is consistent with 2.
>
> Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
> attributes are introduced?
>
> I voted yes for syntax 2. when the attributes were using << >>. I would
> vote NO now with the new syntax.
>
> Nicolas
>

As far as my understanding goes, if we introduce "nested" attributes, it
will be in the form of relaxing constraints on constant expressions, i.e.
by allowing you to write #[Attr(new NestedAttr)].

Nikita

---

## #111945 — Nicolas Grekas — 2020-09-28T12:58:03+00:00

URL: https://externals.io/message/111936#111945 · In reply to: #111941

> Hi Benjamin, hi everyone
> > […]
>
> As far as my understanding goes, if we introduce "nested" attributes, it
> will be in the form of relaxing constraints on constant expressions, i.e.
> by allowing you to write #[Attr(new NestedAttr)].
>

That's what I've read in previous discussions and in other replies in this
thread.
This would break the possibility to read annotations without failing hard
when a class is missing.

I would much prefer a solution that preserves this capability (which is the
reason why we have ReflectionAttribute::newInstance(): unless this one is
called, no autoloading happens.

To me, this means that "new Foo()" nested in an attribute can't be a
solution.
Neither should we be able to nest PHP constants there btw.

Since we borrowed the syntax to Rust, here is the syntax they use:

> #[validate(length(min = 1), custom = "validate_unique_username")]

I think this would fit quite nicely for us too, by turning eg length into
an instance of ReflectionAttribute.

Note that a use case for nested attributes is discussed in
https://github.com/symfony/symfony/pull/38309, to replace things like:
/**
@Assert\All([
    @Assert\Email,
    @Assert\NotBlank,
    @Assert\Length(max=100)
])
*/

We can work around of course, and we will for 8.0, but it would be nice to
have a plan forward because similar use cases (grouping attributes in a
wrapper attribute) are going to be pretty common IMHO.

But we're getting a bit of topic. I'm fine keeping things as is for 8.0. I
just wanted to raise a point about the syntax for list of attributes but it
didn't get traction apparently.

Cheers,
Nicolas

---

## #111952 — Benjamin Eberlei — 2020-09-29T07:35:08+00:00

URL: https://externals.io/message/111936#111952 · In reply to: #111945

On Mon, Sep 28, 2020 at 2:58 PM Nicolas Grekas <nicolas.grekas@gmail.com>
wrote:

>
> Hi Benjamin, hi everyone
> > […]
>
> That's what I've read in previous discussions and in other replies in this
> thread.
> This would break the possibility to read annotations without failing hard
> when a class is missing.
>
> I would much prefer a solution that preserves this capability (which is
> the reason why we have ReflectionAttribute::newInstance(): unless this one
> is called, no autoloading happens.
>
> To me, this means that "new Foo()" nested in an attribute can't be a
> solution.
> Neither should we be able to nest PHP constants there btw.
>

Can you clarify your reasoning here to get to this conclusion? because with
#[Foo(Foo::BAR)] we also not trigger autoloading right now until
newInstance()
or getArguments() is called. This is the way it is working right now:
https://gist.github.com/beberlei/8150f60abaab3b0a70ebb76ce6a379b0

Attributes allow constant expressions as arguments, the same which you can
do in constant or property declarations for the default value.

An approach with new NestedAttr() would work the same, only triggering
autoload when argument needs to be evaluated for use.

>
> Since we borrowed the syntax to Rust, here is the syntax they use:
>
> > […]
>
> I think this would fit quite nicely for us too, by turning eg length into
> an instance of ReflectionAttribute.
>
> Note that a use case for nested attributes is discussed in
> https://github.com/symfony/symfony/pull/38309, to replace things like:
> /**
> @Assert\All([
>     @Assert\Email,
>     @Assert\NotBlank,
>     @Assert\Length(max=100)
> ])
> */
>
> We can work around of course, and we will for 8.0, but it would be nice to
> have a plan forward because similar use cases (grouping attributes in a
> wrapper attribute) are going to be pretty common IMHO.
>
> But we're getting a bit of topic. I'm fine keeping things as is for 8.0. I
> just wanted to raise a point about the syntax for list of attributes but it
> didn't get traction apparently.
>
> Cheers,
> Nicolas

---

## #112007 — Nicolas Grekas — 2020-10-05T14:24:09+00:00

URL: https://externals.io/message/111936#112007 · In reply to: #111952

>
> Hi Benjamin, hi everyone
> > […]
>
> Can you clarify your reasoning here to get to this conclusion? because
> with #[Foo(Foo::BAR)] we also not trigger autoloading right now until
> newInstance()
> or getArguments() is called. This is the way it is working right now:
> https://gist.github.com/beberlei/8150f60abaab3b0a70ebb76ce6a379b0
>
> Attributes allow constant expressions as arguments, the same which you can
> do in constant or property declarations for the default value.
>
> An approach with new NestedAttr() would work the same, only triggering
> autoload when argument needs to be evaluated for use.
>

My bad about constants, I was wrong, they already work as part of constant
expressions as you highlighted.

I was propagating this laziness requirement to nested attributes: they
should also be represented as instances of ReflectionAttribute to me at
some point. It would look important to me to be able to inspect a tree of
attributes and decide what to do with each of them before actually
instantiating them (and triggering the autoload cascade).

I'm not convinced that nested attributes should be brought to us via an
improvement of constant-expressions: first because that would break this
laziness requirement, second because I just doubt that constant-expressions
should support objects.

About providing laziness for attributes, this could be done by
ReflectionAttribute::getAttributes():
#[Foo(bar())] could lead to getAttribute returning an array containing a
ReflectionAttribute instance for attribute "bar".

Of course, this has many consequences and also many alternatives that'd
need to be evaluated.

I won't be able to lead an effort towards nested attributes in the short
term, so I'll just let this on the table here :)

Cheers,
Nicolas



>
> > […]

---

## #112077 — Theodore Brown — 2020-10-19T15:17:12+00:00

URL: https://externals.io/message/111936#112077 · In reply to: #112007

> > […]
>
> As far as my understanding goes, if we introduce "nested" attributes, it
> will be in the form of relaxing constraints on constant expressions, i.e.
> by allowing you to write #[Attr(new NestedAttr)].
>
> Nikita

Back when the original `<<>>` attribute syntax was being implemented,
there was an attempt to do just this. But it turned out to be very
difficult to implement, and it was ultimately given up on since it
required significant changes to const expressions. Earlier this year
there was also a poll to gauge interest in supporting function calls
in constant expressions, but most voters opposed it. [1]

Even if a proposal for relaxing constraints on constant expressions
gains enough support, it's not clear this is the ideal path forward
for nested attributes, since as Nicolas pointed out this wouldn't
have the same lazy-loading semantics as existing attributes.

Straightforward support for nested attributes was one of my main
motivations for proposing the `@@` syntax in the Shorter Attribute
Syntax RFC, [2] and I had hoped to collaborate on a follow-up RFC to
support nested attributes with the AT-AT syntax. This would allow
existing nested docblock annotations such as Nicolas's example to
translate intuitively to native attributes:

    @@Assert\All([
        @@Assert\Email,
        @@Assert\NotBlank,
        @@Assert\Length(max: 100)
    ])

This would also preserve the lazy-loading feature where these
attribute classes aren't loaded until code calls `newInstance`
on one of the `ReflectionAttribute` objects.

But then the Shorter Attribute Syntax Change RFC [3] came along and
derailed this plan...

In theory nested attributes could be supported in the same way with
the `#[]` syntax, but it's more verbose and I think less intuitive
(e.g. people may try to use the grouped syntax in this context, but
it wouldn't work). Also the combination of brackets delineating both
arrays and attributes reduces readability:

    #[Assert\All([
        #[Assert\Email],
        #[Assert\NotBlank],
        #[Assert\Length(max: 100)]
    ])]

But at this point I assume this is the most viable path forward for
nested attributes (barring another syntax re-vote and delay of PHP 8).
I know Derick and Benjamin have stated they aren't in favor of nested
attributes and didn't put any thought into the syntax for them, but I
feel this is unfortunate since nested attributes are an established
pattern with legitimate use cases in existing libraries.

Best regards,  
Theodore

[1]: https://wiki.php.net/rfc/calls_in_constant_expressions_poll
[2]: https://wiki.php.net/rfc/shorter_attribute_syntax
[3]: https://wiki.php.net/rfc/shorter_attribute_syntax_change

---

## #112078 — Benas IML — 2020-10-19T19:16:37+00:00

URL: https://externals.io/message/111936#112078 · In reply to: #112077

> On Mon, Oct 5, 2020 at 9:24 AM Nicolas Grekas <nicolas.grekas@gmail.com>
> wrote:
>
> > […]
>
> Back when the original `<<>>` attribute syntax was being implemented,
> there was an attempt to do just this. But it turned out to be very
> difficult to implement, and it was ultimately given up on since it
> required significant changes to const expressions. Earlier this year
> there was also a poll to gauge interest in supporting function calls
> in constant expressions, but most voters opposed it. [1]
>
> Even if a proposal for relaxing constraints on constant expressions
> gains enough support, it's not clear this is the ideal path forward
> for nested attributes, since as Nicolas pointed out this wouldn't
> have the same lazy-loading semantics as existing attributes.
>
> Straightforward support for nested attributes was one of my main
> motivations for proposing the `@@` syntax in the Shorter Attribute
> Syntax RFC, [2] and I had hoped to collaborate on a follow-up RFC to
> support nested attributes with the AT-AT syntax. This would allow
> existing nested docblock annotations such as Nicolas's example to
> translate intuitively to native attributes:
>
>     @@Assert\All([
>         @@Assert\Email,
>         @@Assert\NotBlank,
>         @@Assert\Length(max: 100)
>     ])
>
> This would also preserve the lazy-loading feature where these
> attribute classes aren't loaded until code calls `newInstance`
> on one of the `ReflectionAttribute` objects.
>
> But then the Shorter Attribute Syntax Change RFC [3] came along and
> derailed this plan...
>
> In theory nested attributes could be supported in the same way with
> the `#[]` syntax, but it's more verbose and I think less intuitive
> (e.g. people may try to use the grouped syntax in this context, but
> it wouldn't work). Also the combination of brackets delineating both
> arrays and attributes reduces readability:
>

Welp, both variants seem to be readable.


>     #[Assert\All([
>         #[Assert\Email],
>         #[Assert\NotBlank],
>         #[Assert\Length(max: 100)]
>     ])]
>
> But at this point I assume this is the most viable path forward for
> nested attributes (barring another syntax re-vote and delay of PHP 8).
>

Are you actually kidding me? 4th revote? Please no.

I know Derick and Benjamin have stated they aren't in favor of nested
> attributes and didn't put any thought into the syntax for them, but I
> feel this is unfortunate since nested attributes are an established
> pattern with legitimate use cases in existing libraries.
>
> Best regards,
> Theodore


Best regards,
Benas

>
> [1]: https://wiki.php.net/rfc/calls_in_constant_expressions_poll
> [2]: https://wiki.php.net/rfc/shorter_attribute_syntax
> [3]: https://wiki.php.net/rfc/shorter_attribute_syntax_change
> --
> 
> To unsubscribe, visit:

https://www.php.net/unsub.php

---

## #112080 — Rowan Tommins — 2020-10-20T15:13:55+00:00

URL: https://externals.io/message/111936#112080 · In reply to: #112077

>
> In theory nested attributes could be supported in the same way with
> the `#[]` syntax, but it's more verbose and I think less intuitive
> (e.g. people may try to use the grouped syntax in this context, but
> it wouldn't work). Also the combination of brackets delineating both
> arrays and attributes reduces readability:
>
>     #[Assert\All([
>         #[Assert\Email],
>         #[Assert\NotBlank],
>         #[Assert\Length(max: 100)]
>     ])]
>


I think you're presupposing how a feature would work that hasn't even been
specced out yet. On the face of it, it would seem logical and achievable
for the above to be equivalent to this:

#[Assert\All(
   #[
        Assert\Email,
        Assert\NotBlank,
        Assert\Length(max: 100)
   ]
)]

i.e. for a list of grouped attributes in nested context to be equivalent to
an array of nested attributes.

Unless nested attributes were limited to being direct arguments to another
attribute, the *semantics* of nested attributes inside arrays would have to
be defined anyway (e.g. how they would look in reflection, whether they
would be recursively instantiated by newInstance(), etc).

Regards,
-- 
Rowan Tommins
[IMSoP]

---

## #112106 — Theodore Brown — 2020-10-23T15:52:24+00:00

URL: https://externals.io/message/111936#112106 · In reply to: #112080

> 
> 
> > […]
> 
> I think you're presupposing how a feature would work that hasn't
> even been specced out yet. On the face of it, it would seem logical
> and achievable for the above to be equivalent to this:
> 
>     #[Assert\All(
>        #[
>             Assert\Email,
>             Assert\NotBlank,
>             Assert\Length(max: 100)
>        ]
>     )]
> 
> i.e. for a list of grouped attributes in nested context to be
> equivalent to an array of nested attributes.

Hi Rowan,

The problem with this is that it results in inconsistent semantics,
where the number of items in an attribute group results in a
different type of value being passed. I.e. if you remove two of the
three attributes from the list, suddenly the code will break since
the `Assert\All` attribute is no longer being passed an array.

// error when Assert\All is instantiated since it's no longer being  
// passed an array of attributes:

    #[Assert\All(
        #[
            Assert\Email,
        ]
    )]

So when removing nested attributes from a list such that there's only
one left in the list, you'd have to remember to additionally wrap the
attribute group in array brackets:

    #[Assert\All(
        [#[
            Assert\Email,
        ]]
    )]

And of course when you want to add additional attributes to a group
that originally had only one attribute, you'd have to remember to
remove the extra array brackets.

Generally speaking, having the number of items in a list change the
type of the list is a recipe for confusion and unexpected errors. So
I think the only sane approach is to ban using the grouped syntax in
combination with nested attributes.

Unfortunately, no one thought through these issues when proposing to
change the shorter attribute syntax away from @@ and add the grouped
syntax, and now it seems like we're stuck with a syntax that is 
unnecessarily complex and confusing to use for nested attributes.

> Unless nested attributes were limited to being direct arguments to
> another attribute, the *semantics* of nested attributes inside
> arrays would have to be defined anyway (e.g. how they would look in
> reflection, whether they would be recursively instantiated by
> newInstance(), etc).

Yes, the exact semantics of nested attributes need to be defined, but
this is a completely separate topic from the grouped syntax issue
described above.

As a user I would expect nested attributes to be reflected the same
as any other attribute (i.e. as a `ReflectionAttribute` instance).
Calling `getArguments` on a parent attribute would return an array
with `ReflectionAttribute` objects for any nested attribute passed
as a direct argument or array value.

On first thought I think it makes sense to recursively instantiate
nested attributes when calling `newInstance` on a parent attribute.
This would allow attribute class constructors to specify the type
for each attribute argument. But again, this is a separate discussion
from the issue of nested attribute syntax.

Kind regards,  
Theodore

---

## #112107 — vorismi3@fel.cvut.cz — 2020-10-23T17:08:02+00:00

URL: https://externals.io/message/111936#112107 · In reply to: #112106

Wdyt about dedicating "#" char for attributes? 

We can deprecate comments with "#" in 8.0 or 8.1 

and in later version support attributes with "#" single character -
without [] brackets, ie. support: 

#Attribute1
#Attribte2
public function x()... 

which will be equivalent to: 

#[
#Attribute1
#Attribte2
]
public function x()... 

Is there anything against excl. BC break of "#" comments? Who is for it?


With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,

Michael Voříšek



> 
> 
> 
> 
> In theory nested attributes could be supported in the same way with
> the `#[]` syntax, but it's more verbose and I think less intuitive
> (e.g. people may try to use the grouped syntax in this context, but
> it wouldn't work). Also the combination of brackets delineating both
> arrays and attributes reduces readability:
> 
> #[Assert\All([
> #[Assert\Email],
> #[Assert\NotBlank],
> #[Assert\Length(max: 100)]
> ])]
> 
> I think you're presupposing how a feature would work that hasn't
> even been specced out yet. On the face of it, it would seem logical
> and achievable for the above to be equivalent to this:
> 
> #[Assert\All(
> #[
> Assert\Email,
> Assert\NotBlank,
> Assert\Length(max: 100)
> ]
> )]
> 
> i.e. for a list of grouped attributes in nested context to be
> equivalent to an array of nested attributes.

Hi Rowan,

The problem with this is that it results in inconsistent semantics,
where the number of items in an attribute group results in a
different type of value being passed. I.e. if you remove two of the
three attributes from the list, suddenly the code will break since
the `Assert\All` attribute is no longer being passed an array.

// error when Assert\All is instantiated since it's no longer being  
// passed an array of attributes:

    #[Assert\All(
        #[
            Assert\Email,
        ]
    )]

So when removing nested attributes from a list such that there's only
one left in the list, you'd have to remember to additionally wrap the
attribute group in array brackets:

    #[Assert\All(
        [#[
            Assert\Email,
        ]]
    )]

And of course when you want to add additional attributes to a group
that originally had only one attribute, you'd have to remember to
remove the extra array brackets.

Generally speaking, having the number of items in a list change the
type of the list is a recipe for confusion and unexpected errors. So
I think the only sane approach is to ban using the grouped syntax in
combination with nested attributes.

Unfortunately, no one thought through these issues when proposing to
change the shorter attribute syntax away from @@ and add the grouped
syntax, and now it seems like we're stuck with a syntax that is 
unnecessarily complex and confusing to use for nested attributes.

> Unless nested attributes were limited to being direct arguments to
> another attribute, the *semantics* of nested attributes inside
> arrays would have to be defined anyway (e.g. how they would look in
> reflection, whether they would be recursively instantiated by
> newInstance(), etc).

Yes, the exact semantics of nested attributes need to be defined, but
this is a completely separate topic from the grouped syntax issue
described above.

As a user I would expect nested attributes to be reflected the same
as any other attribute (i.e. as a `ReflectionAttribute` instance).
Calling `getArguments` on a parent attribute would return an array
with `ReflectionAttribute` objects for any nested attribute passed
as a direct argument or array value.

On first thought I think it makes sense to recursively instantiate
nested attributes when calling `newInstance` on a parent attribute.
This would allow attribute class constructors to specify the type
for each attribute argument. But again, this is a separate discussion
from the issue of nested attribute syntax.

Kind regards,  
Theodore

--

To unsubscribe, visit: https://www.php.net/unsub.php

---

## #112108 — Benjamin Morel — 2020-10-23T17:14:25+00:00

URL: https://externals.io/message/111936#112108 · In reply to: #112107

On Fri, 23 Oct 2020 at 19:08, Michael Voříšek - ČVUT FEL <
vorismi3@fel.cvut.cz> wrote:

> We can deprecate comments with "#" in 8.0 or 8.1
>
> ...
>
> Is there anything against excl. BC break of "#" comments? Who is for it?
>
>
I'm all for it, especially considering that it's trivial to fix old
codebases with automated tools with 100% confidence.
But I'm afraid it's too late to do this for 8.0, even though a major
release would have been the perfect time to deprecate such a feature.

— Benjamin

---

## #112109 — Rowan Tommins — 2020-10-23T18:06:04+00:00

URL: https://externals.io/message/111936#112109 · In reply to: #112106

> The problem with this is that it results in inconsistent semantics,
> where the number of items in an attribute group results in a
> different type of value being passed. I.e. if you remove two of the
> three attributes from the list, suddenly the code will break since
> the `Assert\All` attribute is no longer being passed an array.


That's a fair point. Perhaps nested attributes could *always* be 
considered a list?

That may sound odd, but it's actually quite consistent with "normal" 
attributes:

#[Foo]
class C {}
var_dump( (new ReflectionClass('C'))->getAttributes() );

Gives an array with one item in it, not an object:

array(1) {
     [0]=> object(ReflectionAttribute)#2 (0) { }
}


So similarly:

#[Foo( 42, #[Bar] )
class C {}
var_dump( (new ReflectionClass('C'))->getAttributes()[0]->getArguments() );

Could give an array with one item in it as the argument:

array(2) {
     [0] => int(42),
     [1] => array(1) {
         [0]=> object(ReflectionAttribute)#3 (0) { }
     }
}


This actually feels quite intuitive, because we're used to square 
brackets representing arrays. The only downside is a slight 
inconvenience for cases where you want an argument of exactly one nested 
attribute, but I don't know how common that is.

Regards,

-- 
Rowan Tommins (né Collins)
[IMSoP]

---

## #112135 — Nicolas Grekas — 2020-10-28T16:58:40+00:00

URL: https://externals.io/message/111936#112135 · In reply to: #112106

Hi all!

about why we'd need nested attributes, here is a discussion about nested
validation constraints:
https://github.com/symfony/symfony/issues/38503

You'll see that we're looking hard into ways around using nested
attributes, but they all fall short. We actually concluded that we should
not create a hacky syntax before php-internals settled on the topic.
I invite you to review the ideas and arguments posted there to see a real
world need on the topic (please don't mind sharing your thoughts about the
topic in the PR of course if you think you can help us move forward.)

See more comments below:

Le ven. 23 oct. 2020 à 17:52, Theodore Brown <theodorejb@outlook.com> a
écrit :

> On Tue, Oct 20, 2020 at 10:13 AM Rowan Tommins <rowan.collins@gmail.com>
> wrote:
>
> > […]
> wrote:
> > […]
>
> Hi Rowan,
>
> The problem with this is that it results in inconsistent semantics,
> where the number of items in an attribute group results in a
> different type of value being passed. I.e. if you remove two of the
> three attributes from the list, suddenly the code will break since
> the `Assert\All` attribute is no longer being passed an array.
>

Yes.
This would be solved by NOT accepting #[] inside attribute declarations.
Since we borrowed from Rust, let's borrow this again:
http://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/reference/attributes.html#attributes

The Rust syntax accepts the #[] only on the outside of the declaration.
The example above should be written as:
     #[Assert\All([
             Assert\Email(),
             Assert\NotBlank(),
             Assert\Length(max: 100)
     ])]

The closing brackets () would be required to remove any ambiguity with
constants.
Since constant expressions won't ever allow functions in them, this isn't
ambiguous with function calls.



> // error when Assert\All is instantiated since it's no longer being
> // passed an array of attributes:
>
>     #[Assert\All(
>         #[
>             Assert\Email,
>         ]
>     )]
>
> So when removing nested attributes from a list such that there's only
> one left in the list, you'd have to remember to additionally wrap the
> attribute group in array brackets:
>
>     #[Assert\All(
>         [#[
>             Assert\Email,
>         ]]
>     )]
>
> And of course when you want to add additional attributes to a group
> that originally had only one attribute, you'd have to remember to
> remove the extra array brackets.
>
> Generally speaking, having the number of items in a list change the
> type of the list is a recipe for confusion and unexpected errors. So
> I think the only sane approach is to ban using the grouped syntax in
> combination with nested attributes.
>
> Unfortunately, no one thought through these issues when proposing to
> change the shorter attribute syntax away from @@ and add the grouped
> syntax, and now it seems like we're stuck with a syntax that is
> unnecessarily complex and confusing to use for nested attributes.
>
> > […]
>
> Yes, the exact semantics of nested attributes need to be defined, but
> this is a completely separate topic from the grouped syntax issue
> described above.
>
> As a user I would expect nested attributes to be reflected the same
> as any other attribute (i.e. as a `ReflectionAttribute` instance).
> Calling `getArguments` on a parent attribute would return an array
> with `ReflectionAttribute` objects for any nested attribute passed
> as a direct argument or array value.
>

I 100% agree with this.


On first thought I think it makes sense to recursively instantiate
> nested attributes when calling `newInstance` on a parent attribute.
> This would allow attribute class constructors to specify the type
> for each attribute argument. But again, this is a separate discussion
> from the issue of nested attribute syntax.
>

Also 100% agree with this.

Cheers,
Nicolas

---

## #112139 — Rowan Tommins — 2020-10-28T17:57:11+00:00

URL: https://externals.io/message/111936#112139 · In reply to: #112135

> about why we'd need nested attributes, here is a discussion about 
> nested validation constraints:
> https://github.com/symfony/symfony/issues/38503


Thanks, it's useful to see some real-world examples. As I suspected, 
nearly all of these explicitly take a *list* of nested attributes, not a 
single attribute.

 > While most of the constraints receive the nested constraints as 
simple array, |Collection| however requires a mapping (field to 
constraint) and is usually combined with other composite constraints, 
which gives us a second nesting level.


There is much discussion of Collection as an edge case, but although the 
nested attributes are indeed inside a mapping, the *value* of that 
mapping is again a list of attributes. Single items are allowed, but 
this seems to be a special case for convenience.

In the below example from 
https://symfony.com/doc/current/reference/constraints/Collection.html 
the @Assert\Email could equivalently be written as an array with one item:

     /**
      * @Assert\Collection(
      *     fields = {
      *         "personal_email" = @Assert\Email,
      *         "short_bio" = {
      *             @Assert\NotBlank,
      *             @Assert\Length(
      *                 max = 100,
      *                 maxMessage = "Your short bio is too long!"
      *             )
      *         }
      *     },
      *     allowMissingFields = true
      * )
      */


This reinforces my earlier suggestion 
(https://externals.io/message/111936#112109) that #[Foo] in a nested 
context can simply imply an array of one attribute, rendering the above as:

#[Assert\Collection(
     fields: [
          "personal_email" => #[Assert\Email],
          "short_bio" => #[
                 Assert\NotBlank,
                 Assert\Length(
                       max: 100,
                       maxMessage: "Your short bio is too long!"
                )
         ]
     ],
     allowMissingFields: true
]


Regards,

-- 
Rowan Tommins (né Collins)
[IMSoP]

---

## #112149 — Theodore Brown — 2020-10-30T18:47:23+00:00

URL: https://externals.io/message/111936#112149 · In reply to: #112139

Hi Rowan and Nicolas (and internals),



> 
> > […]
> 
> Thanks, it's useful to see some real-world examples. As I suspected,
> nearly all of these explicitly take a *list* of nested attributes,
> not a single attribute.
> 
> > […]
> 
> There is much discussion of Collection as an edge case, but although
> the nested attributes are indeed inside a mapping, the *value* of
> that mapping is again a list of attributes. Single items are allowed,
> but this seems to be a special case for convenience. ... This
> reinforces my earlier suggestion (https://externals.io/message/111936#112109)
> that #[Foo] in a nested context can simply imply an array of one
> attribute,


While passing all nested attributes as an array would at least enable
consistent semantics, it has the notable disadvantage of preventing
some use cases from being expressed in PHP's type system. Specifically,
how would you express that an attribute parameter must be passed a
single attribute of a particular type?

For example, suppose a developer wants to create/use an attribute
like the following:

    #[Limit(
        min: 10,
        max: 100,
        error: #[CustomError(message: "...", notify: true, withTrace: false)]
    )]

I would expect to be able to write the `Limit` attribute class like this:

    #[Attribute]
    class Limit {
        public function __construct(
            public int $min,
            public int $max,
            public CustomError $error,
        ) {}
    }

But if nested attributes are always passed as an array, the `$error`
parameter would have to have an `array` type, which provides
essentially no context about what is really required. The type system
would be perfectly happy to allow an empty array, or an array with
multiple values instead of the expected single `CustomError` attribute.

It may be possible to enable static analysis type checks and IDE
autocompletion by adding extra docblock annotations or attributes
specifying that the array has to contain exactly one `CustomError`
value, but this is a workaround requiring a lot more effort than
should be necessary. Overall the dev experience becomes a lot worse
for these use cases.




> > […]
> 
> Yes.
> This would be solved by NOT accepting #[] inside attribute declarations.
> Since we borrowed from Rust, let's borrow this again:
> http://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/reference/attributes.html#attributes
> 
> The Rust syntax accepts the #[] only on the outside of the declaration.
> The example above should be written as:
>
>     #[Assert\All([
>         Assert\Email(),
>         Assert\NotBlank(),
>         Assert\Length(max: 100)
>     ])]
> 
> The closing brackets () would be required to remove any ambiguity
> with constants. Since constant expressions won't ever allow functions
> in them, this isn't ambiguous with function calls.


This does seem more readable and also avoids the grouped syntax
inconsistency. On the other hand, presumably it would prevent ever
supporting function calls in constant expressions. At present I'm not
convinced constant expressions *should* support this, but nevertheless
I have reservations about a syntax choice that would rule it out
completely.

Furthermore, the requirement of parentheses seems inconsistent with
normal attribute declarations and `new ClassName` instantiations,
where parentheses are optional.

As I see it, we have the following other options:

1. Simply ban the grouped syntax in nested context. This has the
downside of extra verbosity and that it may be unexpected/surprising
for developers.

2. Drop support for attribute grouping before PHP 8 is finalized. Rust
(which we borrowed the `#[]` syntax from) itself does not support this
anyway. The primary downside of no attribute grouping is additional
verbosity in some circumstances.

   Note: a number of people who voted for `#[]` expressed disfavor of
attribute grouping, which was originally accepted only for the `<<>>`
syntax with the qualification that it would be superseded by any other
RFC that changes the syntax. [1]

3. Vote to switch to a less verbose syntax that avoids these issues.
I.e. revert to `@@`, or alternatively to `##` (the latter would preserve
forward compatibility). This would allow the same syntax to be used for
both parent and nested attributes with identical semantics.

   The downside is that some tooling (e.g. PHP-Parser and PhpStorm) has
already done work to support the `#[]` syntax and this would need to
be changed (though removing attribute grouping will simplify the
implementation for such tooling).

Examples of option 3:

    @@Limit(
        min: 10,
        max: 100,
        error: @@CustomError(message: "...", notify: true, withTrace: false)
    )

    // or

    ##Limit(
        min: 10,
        max: 100,
        error: ##CustomError(message: "...", notify: true, withTrace: false)
    )


Best regards,  
Theodore

[1]: https://wiki.php.net/rfc/attribute_amendments#group_statement_for_attributes

---

## #112150 — Rowan Tommins — 2020-10-30T19:28:41+00:00

URL: https://externals.io/message/111936#112150 · In reply to: #112149

> While passing all nested attributes as an array would at least enable
> consistent semantics, it has the notable disadvantage of preventing
> some use cases from being expressed in PHP's type system. Specifically,
> how would you express that an attribute parameter must be passed a
> single attribute of a particular type?


I acknowledged this use case, but suspected, and still suspect, that 
it's rarer than the list-of-attributes case.


> For example, suppose a developer wants to create/use an attribute
> like the following:
>
>      #[Limit(
>          min: 10,
>          max: 100,
>          error: #[CustomError(message: "...", notify: true, withTrace: false)]
>      )]


What is the fundamental advantage of using an attribute as the argument 
here? The same thing can be expressed with an associative array:

     #[Limit(
         min: 10,
         max: 100,
         error: ["message" => "...", "notify" => true, "withTrace" => false]
     )]

The only thing that seems to lose is the ability to specify the allowed 
names and types of options - but those won't be checked by the 
ReflectionAttribute anyway, only once the actual constructor is run. So 
there's nothing really "attribute-y" about the CustomError class, and 
any "static object declaration" syntax would do, e.g.

     #[Limit(
         min: 10,
         max: 100,
         error: CustomError{message="...", notify=true, withTrace=false}
     )]



> But if nested attributes are always passed as an array, the `$error`
> parameter would have to have an `array` type, which provides
> essentially no context about what is really required. The type system
> would be perfectly happy to allow an empty array, or an array with
> multiple values instead of the expected single `CustomError` attribute.


Again, this is a general problem, not one related to attributes: the 
Symfony validation examples would be best declared as requiring 
"Constraint[] $constraints" rather than "array $constraints".


> 3. Vote to switch to a less verbose syntax [...]
>     The downside is that...


Let me stop you there. The downside is that a mob of Internals regulars 
will come to your house and lynch you for asking them to vote on the 
same thing yet again. It just ain't gonna happen.


Regards,

-- 
Rowan Tommins (né Collins)
[IMSoP]

---

## #112151 — Paul Jones — 2020-10-30T19:34:54+00:00

URL: https://externals.io/message/111936#112151 · In reply to: #112150

> 
> 
> > […]
> 
> Let me stop you there. The downside is that a mob of Internals regulars will come to your house and lynch you for asking them to vote on the same thing yet again. It just ain't gonna happen.

Yes, because we voted repeatedly until the voters "got it right" -- and now there cannot ever be another vote on it again.


-- 
Paul M. Jones
pmjones@pmjones.io <mailto: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 <https://leanpub.com/sn1php

---

## #112153 — Theodore Brown — 2020-10-30T21:54:10+00:00

URL: https://externals.io/message/111936#112153 · In reply to: #112150

> 
> > […]
> 
> I acknowledged this use case, but suspected, and still suspect, that
> it's rarer than the list-of-attributes case.
> 
> > […]
> 
> What is the fundamental advantage of using an attribute as the argument
> here? The same thing can be expressed with an associative array:
> 
>     #[Limit(
>         min: 10,
>         max: 100,
>         error: ["message" => "...", "notify" => true, "withTrace" => false]
>     )]
> 
> The only thing that seems to lose is the ability to specify the allowed
> names and types of options - but those won't be checked by the
> ReflectionAttribute anyway, only once the actual constructor is run.


The fundamental advantage of using an attribute here instead of an
associative array is that it enables IDE autocompletion, and allows
static analyzers to catch any mistakes in the parameter names/types
before the code runs in production.

> So there's nothing really "attribute-y" about the CustomError class,

What do you mean by "attribute-y"? The `CustomError` class would be a
normal attribute, and as such when calling `getArguments()` on the
`Limit` ReflectionAttribute it would return a ReflectionAttribute for
`CustomError`, which would allow its arguments to be inspected without
loading the class.

> and any "static object declaration" syntax would do, e.g.
> 
>     #[Limit(
>         min: 10,
>         max: 100,
>         error: CustomError{message="...", notify=true, withTrace=false}
>     )]

It's not clear what exactly you're proposing here, or how it would work.
E.g. would this still allow inspecting the attribute arguments without
loading the `CustomError` class? Why is a new syntax needed for this?
Would it limit what the `CustomError` constructor is allowed to do? 


> > […]
> 
> 
> Again, this is a general problem, not one related to attributes: the
> Symfony validation examples would be best declared as requiring
> "Constraint[] $constraints" rather than "array $constraints".

There is no general problem of not being able to specify that a
parameter takes a single object with a particular type. This would be
an artificial limitation on attributes to patch over the inherent
inconsistency of the grouped syntax for nested attributes.


> > […]
> 
> Let me stop you there. The downside is that a mob of Internals regulars
> will come to your house and lynch you for asking them to vote on the
> same thing yet again. It just ain't gonna happen.

I doubt anyone is more tired and worn out from discussing and voting
on attributes than I am. But there are serious issues when it comes to
supporting nested attributes with the #[] syntax and attribute grouping.
Shouldn't I bring them up and propose solutions before PHP 8 is released?

Best regards,  
Theodore

---

## #112156 — Rowan Tommins — 2020-10-31T10:06:07+00:00

URL: https://externals.io/message/111936#112156 · In reply to: #112153

>The fundamental advantage of using an attribute here instead of an
>associative array is that it enables IDE autocompletion, and allows
>static analyzers to catch any mistakes in the parameter names/types
>before the code runs in production.


Yes, in general being able to declare structures rather than associative arrays is useful. Note though that that's not unique to attributes, nor are third party static analysers limited by the native facilities of the language.


> > […]
>
>What do you mean by "attribute-y"? 


I mean that unlike the Symfony examples, CustomError is not something that you'd use as an attribute on its own, actually attached to a declaration; it's just a container for some data. 



>The `CustomError` class would be a
>normal attribute, and as such when calling `getArguments()` on the
>`Limit` ReflectionAttribute it would return a ReflectionAttribute for
>`CustomError`, which would allow its arguments to be inspected without
>loading the class.


I don't understand what the advantage of that would be in this case. If you "inspect" the arguments without running the constructor or even loading the class definition, isn't that just the same as having an associative array?


> > […]
>withTrace=false}
> > […]
>
>It's not clear what exactly you're proposing here, or how it would
>work.


I don't want to get bogged down in the details, it was just a quick example; the point I was trying to make was that if CustomError is just a data object, then the requirement is some syntax for creating data objects in scopes where that's not currently allowed. That includes in the arguments to attributes, but also includes, for instance, constants, property defaults, and parameter defaults. 


>There is no general problem of not being able to specify that a
>parameter takes a single object with a particular type. 


No, but there's a general problem of not being able to specify that a parameter takes a list of a certain type, and this would be one of many, many places where that is a minor nuisance largely solved by third party static analysis tools.



> This would be
>an artificial limitation on attributes to patch over the inherent
>inconsistency of the grouped syntax for nested attributes.


There is no artificial limitation; there is a binary choice: does #[Foo] represent an object or a list with one item in? This is completely new syntax, so there is nothing for it to be inconsistent with other than itself.

I am arguing that having it represent a list with one item in is actually *more* consistent, because when you attach attributes to a declaration, they are always retrieved as a list.


Regards,

-- 
Rowan Tommins
[IMSoP]

---

## #112158 — Larry Garfield — 2020-11-01T00:38:33+00:00

URL: https://externals.io/message/111936#112158 · In reply to: #112156

> > […]
> 
> 
> There is no artificial limitation; there is a binary choice: does 
> #[Foo] represent an object or a list with one item in? This is 
> completely new syntax, so there is nothing for it to be inconsistent 
> with other than itself.
> 
> I am arguing that having it represent a list with one item in is 
> actually *more* consistent, because when you attach attributes to a 
> declaration, they are always retrieved as a list.

Perhaps a naive question, but I'm missing the downside of:

#[Foo(Bar(name="baz"))]

#[Attribute]
class Foo {
  public function construct(public Bar $bar) {}
}

class Bar {
  public function construct(public string $name) {}
}

Why is that not OK?  Someone mentioned it means you couldn't call a function there, but... that's not a huge problem because I can't imagine why you would.  If we really wanted to avoid that:

#[Foo(new Bar(name="baz"))]

That would be unambiguous, if a bit ugly.

--Larry Garfield

---

## #112284 — Theodore Brown — 2020-11-24T23:09:44+00:00

URL: https://externals.io/message/111936#112284 · In reply to: #112158

> > […]
>
> There is no artificial limitation; there is a binary choice: does
> #[Foo] represent an object or a list with one item in? This is
> completely new syntax, so there is nothing for it to be inconsistent
> with other than itself.

It is very much an artificial limitation. Developers would be prevented
from declaring that an attribute must be passed a single attribute 
with a particular type as one of its parameters. This isn't a natural
or inherent limitation of attributes, but an artificial one in order
to try to make the `#[Attr1, Attr2]` grouped syntax usable with nested
attributes.

> I am arguing that having it represent a list with one item in is
> actually *more* consistent, because when you attach attributes to a
> declaration, they are always retrieved as a list.

That's not necessarily the case for nested attributes. A developer
may want to declare that an attribute constructor takes a single
attribute with a particular type as one of its arguments. Why should
PHP prevent this by only allowing a (possibly empty) list to be
passed? Again, this would be an artificial limitation that reduces
the flexibility and usefulness of nested attributes.



> Perhaps a naive question, but I'm missing the downside of:
> 
>     #[Foo(Bar(name="baz"))]
> 
>     #[Attribute]
>     class Foo {
>       public function construct(public Bar $bar) {}
>     }
> 
>     class Bar {
>       public function construct(public string $name) {}
>     }
> 
> Why is that not OK?  Someone mentioned it means you couldn't call a
> function there, but... that's not a huge problem because I can't
> imagine why you would.  If we really wanted to avoid that:
> 
>     #[Foo(new Bar(name="baz"))]
>
> That would be unambiguous, if a bit ugly.

As I mentioned in my first email in this thread, there was an attempt
to do this back when the original `<<>>` attribute syntax was being
implemented. But this approach ran into difficulties since it would
require significant changes to constant expressions.

Even if someone finds a reasonable way to make such a syntax *work*,
it still has the downside of requiring a different syntax for top-
level and nested attributes (docblock annotations allow using the
same syntax for both). Moreover, re-purposing the syntax for function
calls or class instantiation is likely to cause confusion, since it
looks like the code is doing something other than what it actually
does (and of course it would prevent ever extending constant
expressions to support function calls or class instantiation).

One of my main motivations for proposing the `@@` shorter attribute
syntax was to avoid the complexity of grouped declarations and allow
using a single consistent syntax for both top level and nested
attributes. I fear that the change to the `#[]` syntax with grouping
will prevent PHP from getting a flexible and intuitive nested
attribute implementation, and devs will be forced to stick with
docblock annotations for these use cases.

Kind regards,  
Theodore

---

## #112285 — Rowan Tommins — 2020-11-25T09:00:58+00:00

URL: https://externals.io/message/111936#112285 · In reply to: #112284

> That's not necessarily the case for nested attributes. A developer
> may want to declare that an attribute constructor takes a single
> attribute with a particular type as one of its arguments.


I'm still not sure what the use case for such an attribute constructor 
would be. Your example with "CustomError" seemed to be abusing nested 
attributes as a way to get a value object into a static declaration; and 
all the examples I've seen of nested attributes/annotations "in the 
wild" are wrapping a list of attributes in some kind of container.


> Moreover, re-purposing the syntax for function
> calls or class instantiation is likely to cause confusion, since it
> looks like the code is doing something other than what it actually
> does (and of course it would prevent ever extending constant
> expressions to support function calls or class instantiation).

Then maybe we should make it *be* instantiation. The example you gave 
earlier, of CustomError, would be just as useful outside attributes, 
e.g. in a property initialiser, so perhaps what we need for that use 
case is not nested attributes at all, but the ability to create some 
subset of objects ("structs"?) inside static contexts.

That can be entirely parallel to a feature that lets you wrap a list of 
attributes inside another attribute, and does some recursive magic with 
reflection to let you delay calling their constructors.

Regards,

-- 
Rowan Tommins (né Collins)
[IMSoP]