Skip to content

Support every argument syntax for clone() #1092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 2, 2025
Merged

Support every argument syntax for clone() #1092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions grammar/php.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,22 @@ argument_list:
| '(' variadic_placeholder ')' { init($2); }
;

clone_argument_list:
'(' ')' { $$ = array(); }
| '(' non_empty_clone_argument_list optional_comma ')' { $$ = $2; }
| '(' expr ',' ')' { init(Node\Arg[$2, false, false]); }
| '(' variadic_placeholder ')' { init($2); }
;

non_empty_clone_argument_list:
expr ',' argument
{ init(new Node\Arg($1, false, false, stackAttributes(#1)), $3); }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a !!positions test to check the two cases using stackAttributes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have my computer at hand, but the positions are already implicitly tested by the test verifying that the pretty-printer round-trips. The initial implementation had init(Node\Arg[$1, false, false], $3) and similarly for the other case and that resulted in clone($x, ) being stringified as clone($x, )$x, ) or similar.

Shall I nevertheless add a dedicated test?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think that's good enough.

| argument_no_expr
{ init($1); }
| non_empty_clone_argument_list ',' argument
{ push($1, $3); }
;

variadic_placeholder:
T_ELLIPSIS { $$ = Node\VariadicPlaceholder[]; }
;
Expand All @@ -794,14 +810,18 @@ non_empty_argument_list:
| non_empty_argument_list ',' argument { push($1, $3); }
;

argument:
expr { $$ = Node\Arg[$1, false, false]; }
| ampersand variable { $$ = Node\Arg[$2, true, false]; }
argument_no_expr:
ampersand variable { $$ = Node\Arg[$2, true, false]; }
| T_ELLIPSIS expr { $$ = Node\Arg[$2, false, true]; }
| identifier_maybe_reserved ':' expr
{ $$ = new Node\Arg($3, false, false, attributes(), $1); }
;

argument:
expr { $$ = Node\Arg[$1, false, false]; }
| argument_no_expr { $$ = $1; }
;

global_var_list:
non_empty_global_var_list no_comma
;
Expand Down Expand Up @@ -1015,6 +1035,7 @@ expr:
}
| new_expr
| match
| T_CLONE clone_argument_list { $$ = Expr\FuncCall[new Node\Name($1, stackAttributes(#1)), $2]; }
| T_CLONE expr { $$ = Expr\Clone_[$2]; }
| variable T_PLUS_EQUAL expr { $$ = Expr\AssignOp\Plus [$1, $3]; }
| variable T_MINUS_EQUAL expr { $$ = Expr\AssignOp\Minus [$1, $3]; }
Expand Down
Loading