php / php-langspec

PHP Language Specification
http://www.php.net
Other
2.3k stars 271 forks source link

Change isset expression #229

Closed zhujinxuan closed 5 years ago

zhujinxuan commented 5 years ago

Enable trialing comma in isset. Fix #228

nikic commented 5 years ago

This syntax was added as part of https://wiki.php.net/rfc/trailing-comma-function-calls, which also added support for unset($foo,) and call($foo,). Maybe you would like to add those two as well?

zhujinxuan commented 5 years ago

@nikic Good. I will add them as well tonight :)

zhujinxuan commented 5 years ago

@nikic About trialing comma, there is another thing I am not sure how to describe in php-langspec or in php-parser. If we have a trialing comma inside an empty array:

[,];
array(,);

In current php, it will give a Fatal error rather than a Parser error. So I am not sure whether we shall describe it in php-langspec.

Similar thing happens in list; If we write

list(,) = [1];

which is a legal expression in 10-expression.md. It will also give a Fatal error.

nikic commented 5 years ago

@zhujinxuan We don't really make a distinction between parse error and fatal error in the langspec. It's only important whether the code is valid or not, the type of error doesn't matter.

zhujinxuan commented 5 years ago

@nikic Then shall we remove the ',' in unkeyed-list-expression-list: as well? (As list(,) gives a Fatal error)

nikic commented 5 years ago

@zhujinxuan Ah, this case is more complicated, because generally , is allowed inside list, e.g. you can write list($a,,,,,$b). What is not allowed is a list that consists only of ,, such as list(,,,,,). This kind of rule is best noted in the "Constraints" section, rather than the grammar.

zhujinxuan commented 5 years ago

@nikic I think using Constraints would harm a simplicity that a parser writer just need to read expressions for writing a parser.

How about:

list-intrinsic:
   list   (   list-expression-list   )

list-expression-list:
  commas? list-element
  commas? list-element , list-expression-list?
  commas? list-element commas

commas:
  ,
  , commas

list-element
   expression   =>   list-or-variable
   list-or-variable
zhujinxuan commented 5 years ago

Hi, @nikic . This PR is ready for review now.

I have made the change for unset, functions, methods and new statements. I have also rebase all commits in case this repo do not 'squash and merge' the PR.

For the list statement, I have open a new issue for that. And if we decide to change list expression documentation, we can make another PR for it.

Merry Christmas, Jinxuan Zhu

nikic commented 5 years ago

Merged as https://github.com/php/php-langspec/commit/197a397cf84bdcd3f896aebf15a73b5fd45a174d. Thanks for taking care of this!