Skip to content

Fix serialization of multiple closures on the same line #120

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
taylorotwell merged 4 commits into from
Jan 8, 2026
Merged

Fix serialization of multiple closures on the same line #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1b46b7
Fix serialization of multiple closures on the same line
michaelruelas Jan 7, 2026
b611adb
Apply style fixes and maintain same-line closure tests
michaelruelas Jan 7, 2026
9448a5a
Apply additional style fixes to tests
michaelruelas Jan 7, 2026
e2b37fc
formatting
taylorotwell Jan 8, 2026
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
206 changes: 193 additions & 13 deletions src/Support/ReflectionClosure.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ public function getCode()
$isUsingScope = false;
$isUsingThisObject = false;

$candidates = [];

for ($i = 0, $l = count($tokens); $i < $l; $i++) {
$token = $tokens[$i];

Expand Down Expand Up @@ -296,7 +298,14 @@ public function getCode()
case '}':
$code .= '}';
if (--$open === 0 && ! $isShortClosure) {
break 3;
$reset = $this->collectCandidate($candidates, $code, $use, $isShortClosure, $isUsingThisObject, $isUsingScope);
$code = $reset['code'];
$state = $reset['state'];
$open = $reset['open'];
$use = $reset['use'];
$isShortClosure = $reset['isShortClosure'];
$isUsingThisObject = $reset['isUsingThisObject'];
$isUsingScope = $reset['isUsingScope'];
} elseif ($inside_structure) {
$inside_structure = ! ($open === $inside_structure_mark);
}
Expand All @@ -312,7 +321,15 @@ public function getCode()
case ']':
if ($isShortClosure) {
if ($open === 0) {
break 3;
$reset = $this->collectCandidate($candidates, $code, $use, $isShortClosure, $isUsingThisObject, $isUsingScope);
$code = $reset['code'];
$state = $reset['state'];
$open = $reset['open'];
$use = $reset['use'];
$isShortClosure = $reset['isShortClosure'];
$isUsingThisObject = $reset['isUsingThisObject'];
$isUsingScope = $reset['isUsingScope'];
continue 3;
}
$open--;
}
Expand All @@ -321,7 +338,15 @@ public function getCode()
case ',':
case ';':
if ($isShortClosure && $open === 0) {
break 3;
$reset = $this->collectCandidate($candidates, $code, $use, $isShortClosure, $isUsingThisObject, $isUsingScope);
$code = $reset['code'];
$state = $reset['state'];
$open = $reset['open'];
$use = $reset['use'];
$isShortClosure = $reset['isShortClosure'];
$isUsingThisObject = $reset['isUsingThisObject'];
$isUsingScope = $reset['isUsingScope'];
continue 3;
}
$code .= $token[0];
break;
Expand Down Expand Up @@ -670,16 +695,6 @@ public function getCode()
}
}

if ($isShortClosure) {
$this->useVariables = $this->getStaticVariables();
} else {
$this->useVariables = empty($use) ? $use : array_intersect_key($this->getStaticVariables(), array_flip($use));
}

$this->isShortClosure = $isShortClosure;
$this->isBindingRequired = $isUsingThisObject;
$this->isScopeRequired = $isUsingScope;

$attributesCode = array_map(function ($attribute) {
$arguments = $attribute->getArguments();

Expand All @@ -697,6 +712,36 @@ public function getCode()
return "#[$name($arguments)]";
}, $this->getAttributes());

if (count($candidates) > 1) {
$lastItem = array_pop($candidates);

foreach ($candidates as $candidate) {
if (! $this->verifyCandidateSignature($candidate)) {
continue;
}

$this->applyCandidate($candidate);

$code = $candidate['code'];

if (! empty($attributesCode)) {
$code = implode("\n", array_merge($attributesCode, [$code]));
}

$this->code = $code;

return $this->code;
}

$candidates[] = $lastItem;
}

$lastItem = array_pop($candidates);

$this->applyCandidate($lastItem);

$code = $lastItem['code'];

if (! empty($attributesCode)) {
$code = implode("\n", array_merge($attributesCode, [$code]));
}
Expand Down Expand Up @@ -727,6 +772,10 @@ public function getUseVariables()
return $this->useVariables;
}

if ($this->isShortClosure()) {
return $this->useVariables;
}

$tokens = $this->getTokens();
$use = [];
$state = 'start';
Expand Down Expand Up @@ -1240,4 +1289,135 @@ protected function parseNameQualified($token)

return [$id_start, $id_start_ci, $id_name];
}

/**
* Collect a closure candidate and reset state for finding the next one.
*
* @param array $candidates
* @param string $code
* @param array $use
* @param bool $isShortClosure
* @param bool $isUsingThisObject
* @param bool $isUsingScope
* @return array
*/
protected function collectCandidate(&$candidates, $code, $use, $isShortClosure, $isUsingThisObject, $isUsingScope)
{
$candidates[] = [
'code' => $code,
'use' => $use,
'isShortClosure' => $isShortClosure,
'isUsingThisObject' => $isUsingThisObject,
'isUsingScope' => $isUsingScope,
];

return [
'code' => '',
'state' => 'start',
'open' => 0,
'use' => [],
'isShortClosure' => false,
'isUsingThisObject' => false,
'isUsingScope' => false,
];
}

/**
* Apply a candidate's properties to this instance.
*
* @param array $candidate
* @return void
*/
protected function applyCandidate($candidate)
{
if ($candidate['isShortClosure']) {
$this->useVariables = $this->getStaticVariables();
} else {
$this->useVariables = empty($candidate['use'])
? $candidate['use']
: array_intersect_key($this->getStaticVariables(), array_flip($candidate['use']));
}

$this->isShortClosure = $candidate['isShortClosure'];
$this->isBindingRequired = $candidate['isUsingThisObject'];
$this->isScopeRequired = $candidate['isUsingScope'];
}

/**
* Verify that a candidate matches the closure's signature.
*
* @param array $candidate
* @return bool
*/
protected function verifyCandidateSignature($candidate)
{
$code = $candidate['code'];
$use = $candidate['use'];
$isShortClosure = $candidate['isShortClosure'];

// Check if code starts with 'static' (more precise than searching anywhere in code)
$isStaticCode = strtolower(substr(ltrim($code), 0, 6)) === 'static';
if (parent::isStatic() !== $isStaticCode) {
return false;
}

// Parse the candidate to extract parameters and variables
$tokens = token_get_all('<?php '.$code);
$params = [];
$vars = [];
$state = 'start';

foreach ($tokens as $token) {
if (! is_array($token)) {
if ($token === '(' && $state === 'start') {
$state = 'params';
} elseif ($token === ')' && $state === 'params') {
$state = 'body';
}

continue;
}

if ($token[0] === T_VARIABLE) {
$name = substr($token[1], 1);

if ($state === 'params') {
$params[] = $name;
} elseif ($state === 'body' && $name !== 'this') {
$vars[$name] = true;
}
}
}

// Verify parameter count
if (parent::getNumberOfParameters() !== count($params)) {
return false;
}

// Verify use/captured variables
if ($isShortClosure) {
$actualVars = array_keys(parent::getStaticVariables());
$foundCaptures = array_diff(array_keys($vars), $params);

if (count($foundCaptures) !== count($actualVars)) {
return false;
}

if (count(array_diff($foundCaptures, $actualVars)) > 0) {
return false;
}
} else {
$actualStaticVariables = array_keys(parent::getStaticVariables());

if (! empty($use) && count(array_diff($use, $actualStaticVariables)) > 0) {
return false;
}

if (count($use) !== count(parent::getStaticVariables())) {
return false;
}
}

return true;
}
}
110 changes: 110 additions & 0 deletions tests/MultipleClosuresOnSameLineTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
<?php

use Laravel\SerializableClosure\SerializableClosure;

test('multiple closures on same line with different arguments', function () {
$c1 = fn ($a) => $a;
$c2 = fn ($b) => $b; // @phpstan-ignore-line

$s1 = new SerializableClosure($c1);
$s2 = new SerializableClosure($c2);

expect($s1->getClosure()(1))->toBe(1);
expect($s2->getClosure()(2))->toBe(2);

$u1 = unserialize(serialize($s1))->getClosure();
$u2 = unserialize(serialize($s2))->getClosure();

expect($u1(1))->toBe(1);
expect($u2(2))->toBe(2);
});

test('multiple closures on same line with different static variables', function () {
$a = 1;
$b = 2;
$c1 = fn () => $a;
$c2 = fn () => $b; // @phpstan-ignore-line

$s1 = new SerializableClosure($c1);
$s2 = new SerializableClosure($c2);

expect($s1->getClosure()())->toBe(1);
expect($s2->getClosure()())->toBe(2);

$u1 = unserialize(serialize($s1))->getClosure();
$u2 = unserialize(serialize($s2))->getClosure();

expect($u1())->toBe(1);
expect($u2())->toBe(2);
});

test('mixture of static and non-static closures', function () {
$c1 = fn () => 1;
$c2 = static fn () => 2; // @phpstan-ignore-line

$s1 = new SerializableClosure($c1);
$s2 = new SerializableClosure($c2);

expect($s1->getClosure()())->toBe(1);
expect($s2->getClosure()())->toBe(2);

$u1 = unserialize(serialize($s1))->getClosure();
$u2 = unserialize(serialize($s2))->getClosure();

expect($u1())->toBe(1);
expect($u2())->toBe(2);
});

test('closure using variable named static is not detected as static closure', function () {
$static = 'not a static closure';
$c1 = fn () => $static;
$c2 = fn () => 'other'; // @phpstan-ignore-line

$s1 = new SerializableClosure($c1);
$s2 = new SerializableClosure($c2);

expect($s1->getClosure()())->toBe('not a static closure');
expect($s2->getClosure()())->toBe('other');

$u1 = unserialize(serialize($s1))->getClosure();
$u2 = unserialize(serialize($s2))->getClosure();

expect($u1())->toBe('not a static closure');
expect($u2())->toBe('other');
});

test('multiple traditional closures on same line', function () {
$c1 = function () { return 1; };
$c2 = function () { return 2; }; // @phpstan-ignore-line

$s1 = new SerializableClosure($c1);
$s2 = new SerializableClosure($c2);

expect($s1->getClosure()())->toBe(1);
expect($s2->getClosure()())->toBe(2);

$u1 = unserialize(serialize($s1))->getClosure();
$u2 = unserialize(serialize($s2))->getClosure();

expect($u1())->toBe(1);
expect($u2())->toBe(2);
});

test('multiple traditional closures with use clause on same line', function () {
$a = 1;
$b = 2;
$c1 = function () use ($a) { return $a; };
$c2 = function () use ($b) { return $b; }; // @phpstan-ignore-line

$s1 = new SerializableClosure($c1);
$s2 = new SerializableClosure($c2);

expect($s1->getClosure()())->toBe(1);
expect($s2->getClosure()())->toBe(2);

$u1 = unserialize(serialize($s1))->getClosure();
$u2 = unserialize(serialize($s2))->getClosure();

expect($u1())->toBe(1);
expect($u2())->toBe(2);
});