fix: new ast for some language constructs

[KennedyTedesco]

This is a work in progress to address the issue discussed here: https://github.com/php-rust-tools/parser/issues/258

Some notes:

1) Currently, die() and exit() are both expressions. I have kept them that way, but I am wondering whether it would be more concise to construct them as statements instead. This is tricky because they behave like functions, but they are not.

I have coded the spans start and end using an Option<Span> for die() and exit(), as parentheses are optional for them: 1a0fd30

print() handles this differently with value: Parenthesized {:

https://github.com/php-rust-tools/parser/blob/main/tests/fixtures/0269/ast.txt#L349-L355

2) print is treated as an expression, but it was coded differently: https://github.com/php-rust-tools/parser/blob/main/src/parser/expressions.rs#L1099

(The Precedence::Prefix shouldn´t be Precedence::Print?)

I am wondering whether die() and exit() should follow the same ideas used in print() or if we need to change print() to follow die() and exit().

3) isset() and unset() are special because they cannot accept all kind of argument. For example, isset(null) is not valid. Therefore, we need to create a new issue to describe the need for a new function to parse their parameters and throw parser errors when necessary. I think this could be work for another PR.

Not trivial to decide those things. 😅

[azjezz]

2. print is treated as an expression, but it was coded differently: https://github.com/php-rust-tools/parser/blob/main/src/parser/expressions.rs#L1099

the logic there makes sense for print $var;, but not for print($var);, print($var) should be parsed the same way as other language constructs

[KennedyTedesco]

So, if I understand correctly, should I make print work similarly to die, exit, etc., sharing the same ideas and such?

Or your idea is to parse print 1; as a statement and print(1) as an expression (which means, parse them differently)?

[azjezz]

all language constructs should be statements IMO, wdyt @ryangjchandler?

as for print, i think we can create a seperate enum for it, something like:

pub enum Print { Expression { print: Span, expression: Expression, ending: Ending, // <- not a semicolon, use `utils::skip_ending(state)?;` }, Arguments { print: Span, arguments: ArgumentList, ending: Ending, } }

---

I think to make the code a bit more clean, we can add src/parser/ast/langauge_constructs.rs to define structures and enums for all language constructs.

[ryangjchandler]

Couple of thoughts from me:

1. print has a return value, so should be an expression logically.
2. die() and exit() are the same, they can be used in the same places as an expression.
3. Using ArgumentList doesn't feel right since print only accepts a single argument.

[azjezz]

print only accepts a single argument.

right..., maybe we can add a new SingleArgument node?

[KennedyTedesco]

I've created a POC here of a new node called SingleArgument:

KennedyTedesco@3a4988f

It appears to be reasonable. What you guys think?

This could be used for die, exit...

[azjezz]

@KennedyTedesco can you please push the change to this PR so we can review it?

[KennedyTedesco]

@azjezz Changes pushed, only for die() at this point, so we can decide if we're good to go this way.

Ps: Tests are failing when trying to install some composer dependencies. Locally, all good.

[KennedyTedesco]

This is ready for review.

Overall changes:

1. exit(), die(), eval() and empty() now holds an SingleArgument node;

2. unset() and isset() now holds an ArgumentList node.

3. print behaves as an expression, but it holds an argument and a value. When used as print 1, value is Some otherwise argument is.

4. Tree new parser errors were introduced.

I think that's it. Latter, I'll open some issues about things we need to work on/fix on some parts of the parsing analyses.