Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:111945
MIME-Version: 1.0
References: <CAOWwgpkDAq4d229Z3D3=xBWyX-z8N+qC-BG4j0uoZy4Xc6JLKQ@mail.gmail.com>
 <CAF+90c_8Q-tuE-waqLENoH=ZLknJZfZmiji1p0hqJWwAqSfieQ@mail.gmail.com>
In-Reply-To: <CAF+90c_8Q-tuE-waqLENoH=ZLknJZfZmiji1p0hqJWwAqSfieQ@mail.gmail.com>
Date: Mon, 28 Sep 2020 14:58:03 +0200
Message-ID: <CAOWwgpmMhKDoeawbN23nGuR7J6A=AUfbZ4W+PNUkFACMDmvkhg@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Benjamin Eberlei <kontakt@beberlei.de>, PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000038c21505b05f383c"
Subject: Re: [PHP-DEV] List of attributes
From: nicolas.grekas@gmail.com (Nicolas Grekas)

--00000000000038c21505b05f383c
Content-Type: text/plain; charset="UTF-8"

> 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

--00000000000038c21505b05f383c--