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

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

Reply via email to