pxp-lang / pxp

A suite of high-performance tools for PHP developers – includes a code formatter, static analyser, language server and superset language.
https://pxplang.org
Other
784 stars 0 forks source link

Parser: Clone expression #74

Closed joehoyle closed 2 months ago

joehoyle commented 2 months ago

The code clone get_comment( 1 ) will be parsed as:

Clone(
    CloneExpression {
        target: Identifier(
            SimpleIdentifier(
                SimpleIdentifier {
                    span: Span {
                        line: 2431,
                        column: 23,
                        position: 82743,
                    },
                    value: "get_comment",
                },
            ),
        ),
    },
)

This seems wrong, I'd expect the target to be a FunctionCall

ryangjchandler commented 2 months ago

@joehoyle Are you on the latest commit? I can't replicate this on it's own.

ryangjchandler commented 2 months ago

@joehoyle Just wondering if you saw my comment above and whether you'd be able to provide the entire file that you're trying to parse? Looks like a large file given the spans in that dump above.

dotdash commented 2 months ago

I can't replicate that exact case on the current main branch, but the nesting for the clone/function call expressions seems to be the wrong way around.

I just added a clone to the simple-function-all-args.php text fixture like this:

diff --git crates/pxp-parser/tests/fixtures/functions/simple-function-call-args.php crates/pxp-parser/tests/fixtures/functions/simple-function-call-args.php
index 0ed559a..2f5f60e 100644
--- crates/pxp-parser/tests/fixtures/functions/simple-function-call-args.php
+++ crates/pxp-parser/tests/fixtures/functions/simple-function-call-args.php
@@ -1,3 +1,3 @@
 <?php
-
-foo(1, 2);
\ No newline at end of file
+clone
+foo(1, 2);

And the test shows this diff:

Statement {
<        id: 20,
>        id: 22,
         kind: Expression(
             ExpressionStatement {
<                id: 19,
>                id: 21,
                 span: Span {
<                    start: 10,
<                    end: 17,
>                    start: 15,
>                    end: 22,
                 },
                 expression: Expression {
<                    id: 18,
>                    id: 20,
                     kind: FunctionCall(
                         FunctionCallExpression {
<                            id: 17,
>                            id: 19,
                             span: Span {
<                                start: 7,
<                                end: 16,
>                                start: 6,
>                                end: 21,
                             },
                             target: Expression {
<                                id: 6,
<                                kind: Name(
<                                    Name {
<                                        id: 5,
<                                        kind: Resolved(
<                                            ResolvedName {
<                                                id: 5,
<                                                span: Span {
<                                                    start: 7,
<                                                    end: 10,
>                                id: 7,
>                                kind: Clone(
>                                    CloneExpression {

That is, the CloneExpression shows up as the target of the FunctionCallExpression which seems backwards to me.

dotdash commented 2 months ago

FWIW, the nesting is correct if you put parentheses around the function call.

ryangjchandler commented 2 months ago

@dotdash The problem you mentioned above should be fixed now! Seems the precedences were in the wrong order... thank you for spotting that!

@joehoyle I'm wondering if that will fix your problem too. Going to close this issue until I hear any further issues on this scenario.