Repository navigation
Conversation
phcorp
marked this pull request as ready for review
November 8, 2026 20:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PHP 8.0+ allows a trailing comma in a closure's
uselist. Formatters add one whenever the list spans several lines:elephc rejected this with
Expected variable in use() capture list: after a,, the capture loop always expected another variable. The list now also ends when)follows the comma.This PR also closes the opposite gap. main accepted an empty
use ()and compiled it, while PHP rejects it (unexpected token ")", expecting variable). The parser now requires at least one capture.use (,)anduse ($a,,)are still rejected.Found while compiling a Symfony/TUI application, where formatter output uses this form throughout.
Repro
error: Expected variable in use() capture list110/100110/100use (&$a,)static function () use ($a,)use ()Expected variable in use() capture listuse (,)Expected variable in use() capture listuse ($a,,)Expected variable in use() capture listfn() use ($a,) => $aHow it works
parse_closure:The trailing-comma check runs only after a capture has been read, inside the
!captures.is_empty()branch. A leading,therefore still reaches the "expected variable" error. The empty-list check reuses the same diagnostic, so all malformed lists report the same message.Tests
test_parse_closure_use_list_trailing_comma(parser): a one-line and a multi-line list keep both captures and the&$bby-reference flag.test_closure_use_list_trailing_comma(codegen): runs a multi-line by-value plus by-reference list, and astatic function () use ($c,). It prints110|7, matching PHP underELEPHC_PHP_CHECK=1.test_error_closure_use_list_requires_a_captureandtest_error_closure_use_list_double_trailing_comma(errors):use (),use (,)anduse ($a,,)are rejected.Run against main's parser, the codegen test fails at parse time and the
use ()error test fails because main accepts the list. Runs on this branch:cargo test --test parser_tests closure: 15 passedcargo test --test error_tests closure: 35 passedELEPHC_PHP_CHECK=1 cargo test --test codegen_tests test_closure_use_list_trailing_comma: 1 passedNote
CI status: this PR comes from a fork, so the
CI,PDO Live DatabasesandPHP extension host SDKworkflows are waiting for maintainer approval (action_required). The focused tests above ran locally on macOS arm64, and the codegen tests were also compared with PHP 8.5.12 (ELEPHC_PHP_CHECK=1). This is a front-end change (parser/checker only) with no target-specific code.Limits
eval()parser incrates/elephc-magicianstill rejects the form:eval('$a = 1; $f = function () use ($a,) { return $a; }; return $f();')fails withParse error: eval() fragment is invalid.Follow-ups
eval()parser.src/parser/expr/prefix_complex.rs, in different functions, and add tests totests/error_tests/syntax.rsat different places. A scratch merge is clean and the combined tests pass. The PRs can merge in either order.