Re: List of attributes

> 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