On Mon, Aug 17, 2020 at 3:06 AM Theodore Brown <theodor...@outlook.com> wrote:
> On Sun, Aug 16, 2020 at 4:36 AM Benjamin Eberlei <kont...@beberlei.de> > wrote: > > > We have updated the RFC with all (hopefully) of the feedback from > > this discussion: > > > > https://wiki.php.net/rfc/shorter_attribute_syntax_change > > > > Most notable changes are: > > - A new section with several subsections on the benefits of a closing > > delimiter / enclosing syntax. > > - A section on grouping pro/cons > > - Inclusion of @: as per Theodores request > > > > We are looking for further feedback from the community. > > Hi Benjamin and Derick, > > Thank you for taking the time to further flesh out the RFC and > include the `@:` syntax option. Overall it is a lot better now. > However, I still have some concerns with several claims in the RFC > that are inaccurate or incomplete, particularly in the "Discussion > on Ending Delimiter / Enclosing Delimiters" section. [1] > > The RFC intro says "we should strongly favor a syntax with closing > delimiter to keep consistency with other parts of the language". It > is then argued that "Many complex syntax constructs in PHP have an > ending delimiter", and therefore attributes should have one as well. > > However, ending delimiters in PHP have little to do with how "complex" > a syntax construct is (which is a rather loose definition, anyway). > As I've pointed out before, standalone statements and declarations > generally require an end symbol, but modifiers before a declaration > do not. Attributes fall into the latter category, and therefore the > lack of an end delimiter is consistent. > A docblock is also a "modifier" under your declaration and it has an ending symbol. The RFC gives a definition of the complexity as just a token vs a set of name, arugment_list and constant expression parser rules. It shows an example porting an ORM\JoinTable Attribute, which has arguments, named parameter usage, complex definitions of default values (Strings, arrays). This piece of code probably touches 10-20 parser rules. Most modifiers don't even have a parser rule, as they only match their token. > > The RFC responds to this counterargument with a series of four > claims, none of which is fully accurate in describing declaration > modifiers: > > 1. Even if we only consider keyword declaration modifiers, the claim > that "these modifier keywords all have only exactly one token that > can immediately follow them, T_WHITESPACE" is not correct. Just as > with attributes, modifier keywords can also be followed by a comment > token (#, //, or /**/): > > @@Attribute// some comment > final// some comment > class Foo {...} > > Strangely, the RFC only lists keywords like public and final as > declaration modifiers, and neglects to mention that type declarations > are also in this category (which I have pointed out in all my > previous responses to this argument). > > Type declarations can be not only followed by whitespace and comment > tokens, but also a `T_VARIABLE` token: > > function foo(Type$bar) {} > > 2. The RFC goes on to claim that "[declaration modifiers] are all > non-complex and are only made up of a handful ascii letters". Again, > this fails to consider type declarations, which can contain the same > non-ascii characters that attribute names can. > Types are missing indeed, and they are more complex than a simple token, but less complex than attribute declarations. I guess the difference is that union types are new, and type definitions used to be simple. Attributes however are *new* and already complex, so we still have the option of always enclosing them. > 3. Next, the RFC says "these keywords are always on a single line". > But union type declarations can be spread across multiple lines, > and still don't have an end delimiter: > > function foo( > \My\FullyQualified\ClassName > |string $param > ) {...} > > 4. Finally, the RFC says that "visibility keywords are only boolean > or bitflags in Reflection, but Attributes are a full fledged > `ReflectionAttribute` representing their own distinct language concept." > You can probably guess where I'm going with this... Type declarations > are likewise a fully fledged `ReflectionType` representing their own > distinct language concept. > > So the RFC's argument that attributes need to have a distinct ending > symbol for consistency is not convincing. The lack of an ending > delimiter is fully consistent with other declaration modifiers, > whether simple or complex. > > ## Benefits for IDEs and editors? > > The RFC suggests a benefit of "Consistent colouring for being an end > of the attribute syntax and the keywords in between can use different > colors." I don't really understand this argument. How would an end > delimiter change the syntax highlighting provided by IDEs? > If you consider the three elements of an attribute declaration: 1. Syntax for Attributes 2. Atttribute Name 3. Arguments then if you color them in three different ways, then with an ending symbol it improves the human readability to have the end be in the same color als the beginning. > Another suggested benefit is that IDEs can "Implement regions to > open/close the grouped declaration of one or multiple attributes." > But IDEs can still do this without an end delimiter, and groups of > attributes could even be opened/closed independently by separating > them with an empty line (which should improve readability for humans > as well). > Yes regions would be possible for a whole block of attributes with @@ as a single block. or when adding an empty line between them. But that completely detaches attributes from the declaration visually to have an empty line between them, so coding styles would probably not allow that. With grouping and end delimiters this is potentially easier and more flexible. I don't say this is a particularly important argument, but there is a difference. We are getting flak for not including the littelest argument in this RFC, even though many things are quite obvious. if we are going for completeness then I am going to write literally everything down. > > ## Easier machine parsing? > > The RFC shows a list of different ways that attributes with the `@@` > syntax can end, and claims "This makes programmatic token based > scanning for attribute syntax without a closing delimiter such as `@@` > unnecessarily complicated." > > But I've worked with userland token stream scanners myself, and it's > not difficult to skip `T_WHITESPACE` and `T_COMMENT` tokens. Once you do > that, parsing an attribute is as simple as finding any `T_ATTRIBUTE` > token followed by the name token, then checking for an optional > argument list. If there's not an argument list, that's then end of > the attribute, otherwise the end is the end of the argument list. > > With the `@[]` and `#[]` syntaxes, a userland token parser is actually > *more* complex due to grouping. It not only has to do the same things > listed above, but it also has to check whether there are multiple > comma-separated attributes between the start/end delimiters (making > sure not to confuse a comma-separated attribute with a comma-separated > argument, or the end of an array argument with the attribute end > delimiter). > > So I don't understand how the RFC can claim that attributes without > an end symbol "introduce additional complexity" for machines, when if > anything the opposite is true. > > (And don't get me started about the extra difficulty for token stream > scanners with the `<<>>` syntax which has no `T_ATTRIBUTE` token). > > ## Forcing @@ attributes to end with parenthesis? > > I don't really see the point of this section in the RFC. What issue > would forcing an end parenthesis solve? And what does whitespace > between the attribute name and argument list have to do with anything? > Such whitespace is allowed with *all* of the proposed syntaxes. > I included this section because multiple people (Peter, Rowan, + more i believe) suggested this as a compromise to keep @@ but fix the issues of not having an ending delimiter. The example shows how it does not guarantee an attribute could be scanned as a whole block, due to the fact that you can have whitespaces in there. > > I do agree, though, that requiring an end parenthesis for `@@` would > be inconsistent with object instantiation - that's one of the main > reasons I didn't require it in the Shorter Attribute Syntax RFC. > Exactly > > ## Potential Future Benefits of Enclosed Delimiter Syntax? > > The RFC shows an example of a potential "simpler" attribute using a > string instead of a class name. I honestly have no idea what this is > supposed to do or what benefit it would have over normal attributes. > > The concept of attributes being a class name with optional arguments > has been proven over many years by its use in docblock annotations, > and if there was some deficiency in what this syntax allows it seems > like we would have discovered it by now. > I agree on just the string, but a closure would make 100% sense for a decorating feature. Javascript and Python "Attributes" work as decorators, i.e. they get called around the decorated function. It is not a completely unrealistic feature to think off: @[fn($x) => syslog(LOG_INFO, "Called function foo with x: " . $x)] function foo($message) {} > ## Attribute nesting > > The RFC points out that all the syntaxes can allow attribute nesting, > which is true. However, it would be nice to include an example of > potential future nesting for each syntax, as was included in the > original Shorter Attribute Syntax RFC. The reason is that some of the > syntaxes are less readable than others when nested (particularly `<<>>`, > though arguably `#[]` and `@[]` as well since the attribute end > delimiter can be confused with the end bracket of an array argument. > > @@JoinTable( > "User_Group", > @@JoinColumn("User_id", "id"), > @@JoinColumn("Group_id", "id"), > ) > private $groups; > > #[JoinTable( > "User_Group", > #[JoinColumn("User_id", "id")], > #[JoinColumn("Group_id", "id")], > )] > private $groups; > > @[JoinTable( > "User_Group", > @[JoinColumn("User_id", "id")], > @[JoinColumn("Group_id", "id")], > )] > private $groups; > > <<JoinTable( > "User_Group", > <<JoinColumn("User_id", "id")>>, > <<JoinColumn("Group_id", "id")>>, > )>> > private $groups; > > @:JoinTable( > "User_Group", > @:JoinColumn("User_id", "id"), > @:JoinColumn("Group_id", "id"), > ) > private $groups; > I wanted to skip adding this, because the RFC is already so long and this is really sometihng that everyone can make up in their mind thinking about it. I agree that with <<>> the nested syntax would be hard to stomach, but for @@, @[] and #[] it is just using the same syntax that you prefer or not prefer. > > > With appreciation, > Theodore > > [1]: > https://wiki.php.net/rfc/shorter_attribute_syntax_change#discussion_on_ending_delimiterenclosing_delimiters
