Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

array_pair shall not include empty #561

Open
zhujinxuan opened this issue Dec 25, 2018 · 1 comment
Open

array_pair shall not include empty #561

zhujinxuan opened this issue Dec 25, 2018 · 1 comment

Comments

@zhujinxuan
Copy link

zhujinxuan commented Dec 25, 2018

Hi, it is about a bug we found in psysh bobthecow/psysh#543;

It seems that, in php 7.3 and php7.2, the following syntax is not legal:

$a = [1,,];

which means, at least in inner_array_pair_list, we shall not include empty as the definition.

To fix the problem, and to ensure trialing comma of array still. I think we can:

inner_array_pair_list:
     | empty                                                                   {init();}
     | array_pair comma_opt                                  { init($1); }
     | array_pair comma_or_error inner_array_pair_list {pop($3, $1);}
;

I can submit a PR if you think it is a good change. But I have two things unsure:

  1. I do not have the PHP 5, is [,] legal in php 5?
  2. The definition of array_pair effects the definition of dereferencable_scalar. Can I know what dereferencable_scalar is? Then I can test whether the new definition affects dereferencable_scalar rightly.

Happy Holiday,
Jinxuan Zhu

@nikic
Copy link
Owner

nikic commented Dec 25, 2018

These grammar productions are shared between array literals (which don't allow empty elements) and array destructuring (which does), so it's not possible to just modify the grammar here. Instead I would suggest to add a checkArray() method to the ParserAbstract class, which (recursively) checks for null elements and generates the same "Cannot use empty array elements in arrays" error message as PHP does. checkArray() would then be called from dereferencable_scalar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants