Php-parser: [bug/feature] impossible detect `&` in staticlookup

[alexander-akait]

Input:

$var = &self::getElementByPath();

I think parent and static also affected.

[alexander-akait]

Same for $c =& new C; (compat with php5)

[ichiriac]

instead of using a byref property on nodes, it may be simpler to create a byref node - I'm not sure if this kind of syntax that can compose any node should be used as a property or a node decorator, same for @ symbol : https://github.com/glayzzle/php-parser/issues/195

If we use a property, location will be outside, and you will not be able to resolve something like $c =$ /* comment */ new C;

[alexander-akait]

@ichiriac maybe introducing new node is good idea, but it is just compatibility with php5 and maybe we can avoid big works on this issue, anyway any help is appreciate

[alexander-akait]

/cc @ichiriac i investigate this problem and find two way:

1. Add AssignReference node
2. Add operator =& to assign, but people write this $a = &$b, but it is not problem.

Also we need remove byRef from variable, it is require many work, but it is possible

[ichiriac]

you can use byref in php in this syntaxes :

<?php
function &foo(&$arg) {
  $foo = &foo(&$arg);
  return &$foo;
}

Some are deprecated, but still valid as syntax. Here use cases :

• declaring a function/method
• declaring a parameter
• as an expression

The latest includes the call statement, the return statement, and the argument. As it's a part of expr, it's not only used on assignments, you may parse AST as : assign(var $foo, ByRef( expr... )), or return(ByRef(expr))

But the function/method declaration it's not the same, as the byRef is a part of its declaration flags (just like public/protected/private) so you may keep the byRef boolean property, and the same should apply for function parameters.

[alexander-akait]

@ichiriac i.e. we should add byRef to call?

[ichiriac]

yes, byref before any expression, like a decorator

[alexander-akait]

Need implement for:

• staticlookup
• propertylookup
• offsetlookup ? (need test)
• new (deprecated, but need for backward compatibility)

[ichiriac]

also related to #348

[ichiriac]

hi @evilebottnawi, could you try to integrate this commit - it's a breaking change on byref property and also includes the fix you need

[alexander-akait]

@ichiriac hi, I will try to find the time for this in near future (hope this week)

[ichiriac]

@evilebottnawi - with this commit I'm switching to old behavior and byref is now on every tag needed - test it and if it's ok, I'll finish the implementation (must check null & remove byref node)

[alexander-akait]

@ichiriac i think we should change byRef behaviour, look
https://github.com/php/php-src/blob/php-7.4.0beta4/Zend/zend_language_parser.y#L886
and https://github.com/php/php-src/blob/PHP-5.4/Zend/zend_language_parser.y#L728

I think better add byRef to assign node (in case when we have $var = &(SOMETHING)), anyway it was deprecated and maybe removed in 8.0.0, so i think no need new node or complex implementation for this cases

[ichiriac]

Take a look at the last commit : https://github.com/glayzzle/php-parser/commit/72cde999df66069aee8ca3f58b4147f7df4090cc

The byref property is handled on : variables, offsetlookup, call

It's harder to make it work on assign node, but simpler on targeted nodes.

[alexander-akait]

@ichiriac what about $var = &(SOMETHING);? Which node will have byref? It is introduce new problems, all parsers for php store byRef on assign node (only for this https://github.com/php/php-src/blob/php-7.4.0beta4/Zend/zend_language_parser.y#L886 scenario), we should keep byRef on variable for arguments/array/etc. only if = and & together we should keep byRef on assign node

[ichiriac]

Hi @evilebottnawi,

Your example is not a valid syntax, lets assumeyou write $var = &SOMETHING; which is valid on php-parser but not on php 7, it will assign byref = true on the classreference SOMETHING.

Ok, as you pointing out =& is an edge grammar case, I've made some test on PHP :

$foo = 2;
$bar = &$foo + 1;
// bar = 2 (executes ($bar =& $foo) + 1);
$bar = &($foo + 1);
// parse error
$bar = $foo + 1; 
// bar = 3 (executes $bar = ($foo + 1));

Didn't saw this before, thanks for pointing me this - :+1: for AssignRef - also need to refactor the way byref syntax is allowed :

• returns_ref : https://github.com/php/php-src/blob/php-7.4.0beta4/Zend/zend_language_parser.y#L1035
• use : https://github.com/php/php-src/blob/php-7.4.0beta4/Zend/zend_language_parser.y#L1038
• array_pair : https://github.com/php/php-src/blob/php-7.4.0beta4/Zend/zend_language_parser.y#L1230
• parameter : https://github.com/php/php-src/blob/php-7.4.0beta4/Zend/zend_language_parser.y#L646

For example return &$var; will throw a parse error

[alexander-akait]

@ichiriac We still can't update parser, other problem:

$b = &$a;

In this case we have & on variable, but according your logic we check right node on byref and print &, but variable inside argument (function ($a, &$b) {}) also can byref and we have print & when byref on variable too. So using this logic case above transform into:

$b = &&$a;

@ichiriac Can we add byref on assign, or we need fork this project? Using byref on right node is invalid and and no parser for php does it

[ichiriac]

please, read the previous comment https://github.com/glayzzle/php-parser/issues/205#issuecomment-526834214

[alexander-akait]

@ichiriac i can't understand answer from comment above

[alexander-akait]

Something means $var or $var->foo or $var::test and etc.

Grammar for ?=
php 5.4 - https://github.com/php/php-src/blob/PHP-5.4/Zend/zend_language_parser.y#L728
php 7.4 - https://github.com/php/php-src/blob/php-7.4.0beta4/Zend/zend_language_parser.y#L886

It should be very easy move byref on assign node and it is right solution

[ichiriac]

so, we are agree, can you send a PR ?

[alexander-akait]

@ichiriac :+1: