On Tue, Oct 20, 2020 at 10:13 AM Rowan Tommins <rowan.coll...@gmail.com> wrote:
> On Mon, 19 Oct 2020 at 16:17, Theodore Brown <theodor...@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.
// 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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
