Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112149
To: Rowan Tommins <rowan.collins@gmail.com>, PHP Internals List
	<internals@lists.php.net>, Nicolas Grekas <nicolas.grekas+php@gmail.com>
Thread-Topic: [PHP-DEV] List of attributes
Thread-Index:
 AQHWlKduqAD4JLWTfUSbO6DleNAbtql92cYAgAAq64CAATgcAIAJ4ESAgBYMm8qAAZRFgIAEqHeBgAgHdACAABBagIADMPVM
Date: Fri, 30 Oct 2020 18:47:23 +0000
Message-ID:
 <BYAPR05MB5478A1FBBFC4F66CE8C4CF1CDE150@BYAPR05MB5478.namprd05.prod.outlook.com>
References:
 <CAOWwgpkDAq4d229Z3D3=xBWyX-z8N+qC-BG4j0uoZy4Xc6JLKQ@mail.gmail.com>
 <CAF+90c_8Q-tuE-waqLENoH=ZLknJZfZmiji1p0hqJWwAqSfieQ@mail.gmail.com>
 <CAOWwgpmMhKDoeawbN23nGuR7J6A=AUfbZ4W+PNUkFACMDmvkhg@mail.gmail.com>
 <CAEPJdunxkKSRTxUhT13F-NAO4+3wmMJ3tanfFYh-iV6JUpUQDA@mail.gmail.com>
 <CAOWwgpkrNAVMZ6yj5eZhkd=aCC=x71V8hsLKxvdhqHoR6DKUuQ@mail.gmail.com>
 <BYAPR05MB54782200D1C1505A4EF85BCFDE1E0@BYAPR05MB5478.namprd05.prod.outlook.com>
 <CALKiJKqCMPdEOA-2_LPyBYeRRDEGh9TGgpFp0hWhTCxhCrs1og@mail.gmail.com>
 <BYAPR05MB547858A38633C2AF75A8567CDE1A0@BYAPR05MB5478.namprd05.prod.outlook.com>
 <CAOWwgpmREMkGHvAQMPuUsJckG-5AvGyvpxcc0nh-ZWq9PjuW8Q@mail.gmail.com>,<d335ed43-c797-ddb6-94f2-0588a77b20bf@gmail.com>
In-Reply-To: <d335ed43-c797-ddb6-94f2-0588a77b20bf@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-tmn: [gcDK7AVIbrgeicn7TRLnKdfs/KKOvkDf]
x-ms-publictraffictype: Email
x-incomingheadercount: 44
x-eopattributedmessage: 0
x-ms-office365-filtering-correlation-id: 9d00dd08-eddd-4dbe-74cb-08d87d043d28
x-ms-traffictypediagnostic: BN7NAM10HT150:
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info:
 5C3Vd/d04l1ernjNs33qKM1Xij/X8k7DVHi2zIgjZHWtPkefVqzyk5e4AWJp5y3XYMH+XhDhfeIap48PvBjG6NMmwxA/mSc7VqEciXAqxNJfUK3dWa6rac0juNY9tVwaKfwxDQem3O0tlyekcBaft0iF1suH+pRi4w6DwzZMrohPE2ABaacf9Y8DDX9hvzrZg9lfyCjRIChTUpLTumjEWa3JtpFOyjC3ZVQUV19Xc10MEeIcsM19DZfcZkTPQWkX
x-ms-exchange-antispam-messagedata:
 YylkmDQXoQNLkYnxnxxXH4ItRiDd/W2KPf7IQqwjuZyg8bjkH9PAsgAin2aFEugW/DX+uXg1U/aU2MjL+AvK0pOsbJAmBt/lHHpEK7Pj22kB6GMsO8rZ5vTASDLcDrp2FGKz2bXr5DueQCV772kmig==
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-AuthSource: BN7NAM10FT013.eop-nam10.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 9d00dd08-eddd-4dbe-74cb-08d87d043d28
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Oct 2020 18:47:23.1828
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Internet
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7NAM10HT150
Subject: Re: [PHP-DEV] List of attributes
From: theodorejb@outlook.com (Theodore Brown)

Hi Rowan and Nicolas (and internals),=0A=
=0A=
On Wed, Oct 28, 2020 at 12:57 PM Rowan Tommins <rowan.collins@gmail.com> wr=
ote:=0A=
=0A=
> On 28/10/2020 16:58, Nicolas Grekas wrote:=0A=
> > about why we'd need nested attributes, here is a discussion about=0A=
> > nested validation constraints:=0A=
> > https://github.com/symfony/symfony/issues/38503=0A=
> =0A=
> Thanks, it's useful to see some real-world examples. As I suspected,=0A=
> nearly all of these explicitly take a *list* of nested attributes,=0A=
> not a single attribute.=0A=
> =0A=
> > While most of the constraints receive the nested constraints as=0A=
> > simple array, |Collection| however requires a mapping (field to=0A=
> > constraint) and is usually combined with other composite=0A=
> > constraints, which gives us a second nesting level.=0A=
> =0A=
> There is much discussion of Collection as an edge case, but although=0A=
> the nested attributes are indeed inside a mapping, the *value* of=0A=
> that mapping is again a list of attributes. Single items are allowed,=0A=
> but this seems to be a special case for convenience. ... This=0A=
> reinforces my earlier suggestion (https://externals.io/message/111936#112=
109)=0A=
> that #[Foo] in a nested context can simply imply an array of one=0A=
> attribute,=0A=
=0A=
=0A=
While passing all nested attributes as an array would at least enable=0A=
consistent semantics, it has the notable disadvantage of preventing=0A=
some use cases from being expressed in PHP's type system. Specifically,=0A=
how would you express that an attribute parameter must be passed a=0A=
single attribute of a particular type?=0A=
=0A=
For example, suppose a developer wants to create/use an attribute=0A=
like the following:=0A=
=0A=
    #[Limit(=0A=
        min: 10,=0A=
        max: 100,=0A=
        error: #[CustomError(message: "...", notify: true, withTrace: false=
)]=0A=
    )]=0A=
=0A=
I would expect to be able to write the `Limit` attribute class like this:=
=0A=
=0A=
    #[Attribute]=0A=
    class Limit {=0A=
        public function __construct(=0A=
            public int $min,=0A=
            public int $max,=0A=
            public CustomError $error,=0A=
        ) {}=0A=
    }=0A=
=0A=
But if nested attributes are always passed as an array, the `$error`=0A=
parameter would have to have an `array` type, which provides=0A=
essentially no context about what is really required. The type system=0A=
would be perfectly happy to allow an empty array, or an array with=0A=
multiple values instead of the expected single `CustomError` attribute.=0A=
=0A=
It may be possible to enable static analysis type checks and IDE=0A=
autocompletion by adding extra docblock annotations or attributes=0A=
specifying that the array has to contain exactly one `CustomError`=0A=
value, but this is a workaround requiring a lot more effort than=0A=
should be necessary. Overall the dev experience becomes a lot worse=0A=
for these use cases.=0A=
=0A=
=0A=
On Wed, Oct 28, 2020 at 11:58 AM Nicolas Grekas <nicolas.grekas+php@gmail.c=
om> wrote:=0A=
=0A=
> > The problem with this is that it results in inconsistent semantics,=0A=
> > where the number of items in an attribute group results in a=0A=
> > different type of value being passed. I.e. if you remove two of the=0A=
> > three attributes from the list, suddenly the code will break since=0A=
> > the `Assert\All` attribute is no longer being passed an array.=0A=
> =0A=
> Yes.=0A=
> This would be solved by NOT accepting #[] inside attribute declarations.=
=0A=
> Since we borrowed from Rust, let's borrow this again:=0A=
> http://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/h=
tml/reference/attributes.html#attributes=0A=
> =0A=
> The Rust syntax accepts the #[] only on the outside of the declaration.=
=0A=
> The example above should be written as:=0A=
>=0A=
>     #[Assert\All([=0A=
>         Assert\Email(),=0A=
>         Assert\NotBlank(),=0A=
>         Assert\Length(max: 100)=0A=
>     ])]=0A=
> =0A=
> The closing brackets () would be required to remove any ambiguity=0A=
> with constants. Since constant expressions won't ever allow functions=0A=
> in them, this isn't ambiguous with function calls.=0A=
=0A=
=0A=
This does seem more readable and also avoids the grouped syntax=0A=
inconsistency. On the other hand, presumably it would prevent ever=0A=
supporting function calls in constant expressions. At present I'm not=0A=
convinced constant expressions *should* support this, but nevertheless=0A=
I have reservations about a syntax choice that would rule it out=0A=
completely.=0A=
=0A=
Furthermore, the requirement of parentheses seems inconsistent with=0A=
normal attribute declarations and `new ClassName` instantiations,=0A=
where parentheses are optional.=0A=
=0A=
As I see it, we have the following other options:=0A=
=0A=
1. Simply ban the grouped syntax in nested context. This has the=0A=
downside of extra verbosity and that it may be unexpected/surprising=0A=
for developers.=0A=
=0A=
2. Drop support for attribute grouping before PHP 8 is finalized. Rust=0A=
(which we borrowed the `#[]` syntax from) itself does not support this=0A=
anyway. The primary downside of no attribute grouping is additional=0A=
verbosity in some circumstances.=0A=
=0A=
   Note: a number of people who voted for `#[]` expressed disfavor of=0A=
attribute grouping, which was originally accepted only for the `<<>>`=0A=
syntax with the qualification that it would be superseded by any other=0A=
RFC that changes the syntax. [1]=0A=
=0A=
3. Vote to switch to a less verbose syntax that avoids these issues.=0A=
I.e. revert to `@@`, or alternatively to `##` (the latter would preserve=0A=
forward compatibility). This would allow the same syntax to be used for=0A=
both parent and nested attributes with identical semantics.=0A=
=0A=
   The downside is that some tooling (e.g. PHP-Parser and PhpStorm) has=0A=
already done work to support the `#[]` syntax and this would need to=0A=
be changed (though removing attribute grouping will simplify the=0A=
implementation for such tooling).=0A=
=0A=
Examples of option 3:=0A=
=0A=
    @@Limit(=0A=
        min: 10,=0A=
        max: 100,=0A=
        error: @@CustomError(message: "...", notify: true, withTrace: false=
)=0A=
    )=0A=
=0A=
    // or=0A=
=0A=
    ##Limit(=0A=
        min: 10,=0A=
        max: 100,=0A=
        error: ##CustomError(message: "...", notify: true, withTrace: false=
)=0A=
    )=0A=
=0A=
=0A=
Best regards,  =0A=
Theodore=0A=
=0A=
[1]: https://wiki.php.net/rfc/attribute_amendments#group_statement_for_attr=
ibutes=