Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110332
MIME-Version: 1.0
References: <CAEPJdum6xCwABjpor1YYvuosSB1dEb4xnCZEMABKsEEvNXvcQQ@mail.gmail.com>
 <BYAPR05MB65353FFCCE742728E4193F9DDE8D0@BYAPR05MB6535.namprd05.prod.outlook.com>
In-Reply-To: <BYAPR05MB65353FFCCE742728E4193F9DDE8D0@BYAPR05MB6535.namprd05.prod.outlook.com>
Date: Tue, 2 Jun 2020 00:48:22 +0200
Message-ID: <CAEPJdunpZwLcVUAMB0n6_fhiGU0P8WvycqNO63n=frU8xH_xmQ@mail.gmail.com>
To: Theodore Brown <theodorejb@outlook.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000030e06b05a70d981d"
Subject: Re: [PHP-DEV] [RFC] Amendments to Attributes
From: kontakt@beberlei.de (Benjamin Eberlei)

--00000000000030e06b05a70d981d
Content-Type: text/plain; charset="UTF-8"

On Mon, Jun 1, 2020 at 1:48 AM Theodore Brown <theodorejb@outlook.com>
wrote:

> On Wed, May 20, 2020 at 12:07 PM Benjamin Eberlei <kontakt@beberlei.de>
> wrote:
>
> > the Attributes RFC was rather large already, so a few things were left
> > open or discussions during the vote have made us rethink a things.
> >
> > https://wiki.php.net/rfc/attribute_amendments
> >
> > These points are handled by the Amendments RFC to Attributes:
> >
> > 1. Proposing to add a grouped syntax <<Attr1, Attr2>
>
> Hi Benjamin,
>
> I find the grouped attribute proposal somewhat troubling. The RFC
> contains the following example:
>
> ```php
> <<Attr2("foo"),
>   Attr2("bar")>>
> public function test()
> {
> }
> ```
>
> The problem with this syntax is that adding a new attribute at the
> start or end of the list, or removing one of the attributes, will
> require modifying multiple lines. For some time the language has been
> moving away from this (see the various RFCs to allow trailing commas
> in more places), so this feels like a step backwards.
>
> If trailing commas are allowed in grouped attributes, you could
> write it this way instead:
>
> ```php
> <<
>   Attr2("foo"),
>   Attr2("bar"),
> >>
> public function test()
> {
> }
> ```
>
> But to me this still feels rather clunky. It requires two extra lines,
> and when moving from two attributes to one attribute (or vice versa),
> you'd still probably end up modifying multiple lines.
>

I have added trailing commas to the attribute grouping. Now that its
possible everywhere, we should go with it here directly from the beginning.

About clunkyness, I am not sure this makes sense as an argument, bceause
the extra lines are present for all other cases of trailing commas in
popular coding styles:

[
"arg1",
"arg2",
]

or

foo(
  "arg1",
  "arg2",
);

>
> Another issue with the grouped syntax is that comma separated
> attributes can be easy to confuse with comma separated attribute
> arguments. For example:
>
> ```php
> <<FooAttr(2 * 3 + 3), Bar(4 + 5 * 2)>>
> <<BarAttr(2 * (3 + 3), Baz, (4 + 5) * 2)>>
> function bar() {}
> ```
>
> It can be hard to tell which line contains multiple attributes vs.
> multiple attribute arguments.
>

The same is true about commas in nested short arrays.

>
> Ultimately it seems like the grouped attribute proposal is attempting
> to work around the poor usability of the current verbose syntax. Maybe
> it would be better to instead propose a simpler syntax that avoids
> these issues. I know that some internals members expressed interest
> in an `@@` token, but this was never voted on.
>

If you feel strongly about it, please create your own RFC for this. You
would need to put it under discussion within this week though, as the
feature freeze deadlines are looming.

Personally I have no interest in opening up the syntax question again,
because there are really not many objective arguments to be made, its
mostly subjective and feeling based.

>
> Having a distinct token for attributes would entirely avoid the issues
> of having to modify multiple lines when adding/removing attributes, as
> well as confusion with shift operators and comma-separated attribute
> arguments. E.g. the RFC example would look like this instead:
>

@@ is not a distinct token though, because it is already valid syntax to
have the error silence operator multiple times after each other.

>
> ```php
> @@Attr2("foo")
> @@Attr2("bar")
> public function test()
> {
> }
> ```
>
> To me this would be a lot cleaner and fit in better with the rest of
> the language.
>
> Best regards,
> Theodore
>

--00000000000030e06b05a70d981d--