List of attributes View as Markdown

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

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]

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:

On Mon, 19 Oct 2020 at 16:17, Theodore Brown theodorejb@outlook.com
wrote:

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.

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.

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.

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