PHP-Parser icon indicating copy to clipboard operation
PHP-Parser copied to clipboard

array_pair shall not include empty

Open zhujinxuan opened this issue 6 years ago • 1 comments

Hi, it is about a bug we found in psysh https://github.com/bobthecow/psysh/issues/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

zhujinxuan avatar Dec 25 '18 07:12 zhujinxuan

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.

nikic avatar Dec 25 '18 16:12 nikic