feat(php8): named arguments

[czosel]

cc @Selion05

fixes #653

[czosel]

What I suggested in #673 didn't make sense because arguments are a list of Expression objects, where a name property doesn't make sense. This implements a new node kind namedargument instead. Like this, we're not introducing a breaking change for named arguments support.

[coveralls]

Coverage increased (+0.01%) to 96.077% when pulling 39b527c on czosel:named-arguments into 82970fd on glayzzle:php8.

[ichiriac]

would be less error prone to use setState from lexer and check ahead with next
https://github.com/glayzzle/php-parser/blob/master/src/lexer.js#L339

[ichiriac]

hi, not sure that's the right way, you can also go ahead scan next tokens to see f it matches a T_STRING + ':' and if not go for expr call

[czosel]

fixed 🙂

[czosel]

Hi @ichiriac, thanks for the review! Unfortunately I don’t really have time to work on this currently. If you prefer to make the changes yourself, go ahead 😉

[czosel]

@ichiriac I tried to refactor this a you suggested, but it doesn't work when heredocs are used as function arguments (see tests):

stringManipulator(<<<END
   a
  b
 c
END);

Can you point me in the right direction? 🙂

[czosel]

Alright, I found a solution / workaround: I only check ahead if the current token is T_STRING or a keyword - that way, the rollback is not needed for HEREDOC.

[czosel]

@cseufert Can you take a look at these changes before we merge into the php8 branch?

[cseufert]

It appears that the current code does not follow the nikic parser, as it returns some arguments as thier immediate node, rather than returning an argument AST node (Arg in PHP-Parser). Is this something that we can break doing this update, or should we retain the backward compatibility? Also the function arguments return Entry for some of them, however this seems tied to an array entry, so I guess adding a name to this is not the best option.

As I understand it, backtracking is quite expensive in the parser, this patch will backtrack quite regularly, do we want to do a work around to avoid this? The issue here is the workaround will be quite complex.

[czosel]

Hi @cseufert, thanks for the review! I think we should try to keep backwards compatibility as far as possible - in this case, the deviation from the nikic PHP parser must have already existed before PHP8.
Concerning performance: Interesting point, I don't really know about this. The original implementation was not very clean but didn't use backtracking, and @ichiriac asked me to rewrite it using backtracking. I'd be curious about his opinion on this (and the question about backward compat) though.

[czosel]

I'll go ahead and merge this so that we can try it in the prettier plugin. @ichiriac as always, your feedback is still appreciated 🙂