Skip to content

Commit c8d21d2

Browse files
zonuexeclaude
zonuexe
and
claude
committed
Find the bound closure argument by its attribute when arguments are named
ParametersAcceptorSelector narrowed the $newThis parameter of Closure::bind() to a closure variable's @param-closure-this type only when the closure was the first argument. With named arguments the closure can be anywhere - Closure::bind(newThis: $o, closure: $c) - and ClosureBindArgVisitor marks it wherever it is, so look the marked argument up when the first one is named. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d682601 commit c8d21d2

6 files changed

Lines changed: 143 additions & 9 deletions

File tree

‎src/Reflection/ParametersAcceptorSelector.php‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -532,12 +532,22 @@ public static function applyIntrinsicArgOverrides(
532532
}
533533
}
534534

535+
// the closure is the 1st argument unless the call names its arguments
536+
$closureBindArg = $args[0];
537+
if ($closureBindArg->name !== null) {
538+
foreach ($args as $arg) {
539+
if ($arg->getAttribute(ClosureBindArgVisitor::ATTRIBUTE_NAME) !== null) {
540+
$closureBindArg = $arg;
541+
break;
542+
}
543+
}
544+
}
535545
if (
536-
$args[0]->getAttribute(ClosureBindArgVisitor::ATTRIBUTE_NAME) !== null
537-
&& $args[0]->value instanceof Node\Expr\Variable
538-
&& is_string($args[0]->value->name)
546+
$closureBindArg->getAttribute(ClosureBindArgVisitor::ATTRIBUTE_NAME) !== null
547+
&& $closureBindArg->value instanceof Node\Expr\Variable
548+
&& is_string($closureBindArg->value->name)
539549
) {
540-
$closureVarName = $args[0]->value->name;
550+
$closureVarName = $closureBindArg->value->name;
541551
$inFunction = $scope->getFunction();
542552
if ($inFunction !== null) {
543553
$closureThisParameters = [];

‎tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,28 @@ public function testClosureBindParamClosureThisNamedArguments(): void
853853
$this->checkThisOnly = false;
854854
$this->checkExplicitMixed = true;
855855
$this->checkImplicitMixed = true;
856-
$this->analyse([__DIR__ . '/data/closure-bind-param-closure-this-named.php'], []);
856+
$this->analyse([__DIR__ . '/data/closure-bind-param-closure-this-named.php'], [
857+
[
858+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
859+
19,
860+
],
861+
[
862+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
863+
20,
864+
],
865+
[
866+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
867+
21,
868+
],
869+
[
870+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
871+
22,
872+
],
873+
[
874+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
875+
23,
876+
],
877+
]);
857878
}
858879

859880
public function testClosureBind(): void

‎tests/PHPStan/Rules/Methods/data/closure-bind-param-closure-this-named.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,20 @@
99
class Foo
1010
{
1111

12+
/**
13+
* @param-closure-this \stdClass $c
14+
*/
15+
public function doFoo(\Closure $c): void
16+
{
17+
Closure::bind(closure: $c, newThis: new \stdClass()); // ok
18+
Closure::bind(newThis: new \stdClass(), closure: $c); // ok
19+
Closure::bind(closure: $c, newThis: new self()); // error
20+
Closure::bind(newThis: new self(), closure: $c); // error
21+
Closure::bind($c, newThis: new self()); // error
22+
Closure::bind(newScope: self::class, newThis: new self(), closure: $c); // error
23+
Closure::bind(newThis: new self(), newScope: null, closure: $c); // error
24+
}
25+
1226
/**
1327
* @param-closure-this \stdClass $c
1428
*/

‎turbo-ext/src/ParametersAcceptorSelector.cpp‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1959,9 +1959,25 @@ class ParametersAcceptorSelector
19591959
}
19601960

19611961
{
1962+
// the closure is the 1st argument unless the call names its arguments
1963+
zval *closureBindArgNode = firstArg;
1964+
zv::Val nameHold;
1965+
zval *name = readProperty(pt_pas_arg_name_site, firstArg, PT_LC("name"), nameHold);
1966+
if (UNEXPECTED(name == NULL)) return zv::Val();
1967+
if (Z_TYPE_P(name) != IS_NULL) {
1968+
for (auto entry : zv::ArrRef(argsZv)) {
1969+
zval *arg = entry.value().deref().raw();
1970+
zval *attribute = NULL;
1971+
if (UNEXPECTED(!getAttribute(arg, pt_pas_closure_bind_arg, attribute))) return zv::Val();
1972+
if (attribute != NULL) {
1973+
closureBindArgNode = arg;
1974+
break;
1975+
}
1976+
}
1977+
}
19621978
zval *closureBindArg = NULL;
1963-
if (UNEXPECTED(!getAttribute(firstArg, pt_pas_closure_bind_arg, closureBindArg))) return zv::Val();
1964-
if (closureBindArg != NULL && UNEXPECTED(!applyClosureBindArg(parametersAcceptors, firstArg, scope))) return zv::Val();
1979+
if (UNEXPECTED(!getAttribute(closureBindArgNode, pt_pas_closure_bind_arg, closureBindArg))) return zv::Val();
1980+
if (closureBindArg != NULL && UNEXPECTED(!applyClosureBindArg(parametersAcceptors, closureBindArgNode, scope))) return zv::Val();
19651981
}
19661982

19671983
return parametersAcceptors;
@@ -2417,10 +2433,10 @@ class ParametersAcceptorSelector
24172433
}
24182434

24192435
/* the ClosureBindArgVisitor override; false = pending exception */
2420-
static bool applyClosureBindArg(zv::Val &parametersAcceptors, zval *firstArg, zval *scope)
2436+
static bool applyClosureBindArg(zv::Val &parametersAcceptors, zval *closureBindArg, zval *scope)
24212437
{
24222438
zv::Val valueHold;
2423-
zval *value = argValueOf(firstArg, valueHold);
2439+
zval *value = argValueOf(closureBindArg, valueHold);
24242440
if (UNEXPECTED(value == NULL)) return false;
24252441
bool isVariable = false;
24262442
if (UNEXPECTED(!isA(value, PT_CLASS_VARIABLE, isVariable))) return false;

‎turbo-ext/tests/scope-family-fixture.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,3 +188,20 @@ public function siblingMethod(): int
188188
}
189189

190190
}
191+
192+
/**
193+
* Closure::bind() calls for ParametersAcceptorSelector::selectFromArgs():
194+
* the bound closure's @param-closure-this type narrows the $newThis parameter
195+
* wherever the closure argument is written
196+
*
197+
* @param-closure-this \stdClass $c
198+
*/
199+
function bindNamed(\Closure $c): void
200+
{
201+
\Closure::bind($c, new Holder('a'));
202+
\Closure::bind(closure: $c, newThis: new Holder('b'));
203+
\Closure::bind(newThis: new Holder('c'), closure: $c);
204+
\Closure::bind(newScope: Holder::class, newThis: new Holder('d'), closure: $c);
205+
\Closure::bind(newThis: $c, closure: function (): void {
206+
});
207+
}

‎turbo-ext/tests/scope-family.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2197,6 +2197,62 @@ public function exprRef(Node\Expr $expr): mixed
21972197
}
21982198
}
21992199

2200+
// ---- ParametersAcceptorSelector::selectFromArgs() over the fixture's
2201+
// Closure::bind() calls: the @param-closure-this override of $newThis, with
2202+
// the closure argument found by position or by the visitor's attribute.
2203+
// The walk above parses the fixture as a non-analysed file (function bodies
2204+
// cleaned away), so these calls come from a walk of the rich parser's AST,
2205+
// which also carries the visitors' attributes. Both sides get the walk scope
2206+
// and the walk's own (PHP) variants, so only the selector differs
2207+
/** @var list<array{\PhpParser\Node\Expr\StaticCall, \PHPStan\Analyser\MutatingScope, \PHPStan\Analyser\ExpressionResultStorage|null}> $sfClosureBindCalls */
2208+
$sfClosureBindCalls = [];
2209+
$sfClosureBindCallback = static function (\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope) use (&$sfClosureBindCalls): void {
2210+
if (
2211+
!$node instanceof \PhpParser\Node\Expr\StaticCall
2212+
|| !$node->class instanceof \PhpParser\Node\Name
2213+
|| $node->class->toLowerString() !== 'closure'
2214+
|| !$node->name instanceof \PhpParser\Node\Identifier
2215+
|| $node->name->toLowerString() !== 'bind'
2216+
) {
2217+
return;
2218+
}
2219+
$walkScope = $scope->toWalkScope();
2220+
$sfClosureBindCalls[] = [$node, $walkScope, $walkScope->getCurrentExpressionResultStorage()];
2221+
};
2222+
$sfResolver->processNodes(
2223+
$scContainer->getService('cachedRichParser')->parseFile($sfFile),
2224+
$sfScopeFactory->create(\PHPStan\Analyser\ScopeContext::create($sfFile), $sfClosureBindCallback),
2225+
$sfClosureBindCallback,
2226+
);
2227+
$sfClosureBindVariants = $sfReflectionProvider->getClass(\Closure::class)->getNativeMethod('bind')->getVariants();
2228+
$sfClosureBindNarrowed = 0;
2229+
foreach ($sfClosureBindCalls as $sfCallIndex => [$sfCall, $sfCallScope, $sfCallStorage]) {
2230+
$sfCallStack = $sfHarness->constructorArgs($sfCallScope)['expressionResultStorageStack'];
2231+
$sfCallStack->push($sfCallStorage ?? new \PHPStan\Analyser\ExpressionResultStorage());
2232+
try {
2233+
$sfSelected = [];
2234+
foreach (['php' => \PHPStan\Reflection\ParametersAcceptorSelector::class, 'native' => \PHPStanTurbo\ParametersAcceptorSelector::class] as $side => $sfSelector) {
2235+
try {
2236+
$sfAcceptor = $sfSelector::selectFromArgs($sfCallScope, $sfCall->getArgs(), $sfClosureBindVariants);
2237+
$sfSelected[$side] = array_map(
2238+
static fn ($parameter): string => $parameter->getName() . ': ' . $parameter->getType()->describe(\PHPStan\Type\VerbosityLevel::precise()),
2239+
$sfAcceptor->getParameters(),
2240+
);
2241+
} catch (\Throwable $e) {
2242+
$sfSelected[$side] = get_class($e) . ': ' . $e->getMessage();
2243+
}
2244+
}
2245+
} finally {
2246+
$sfCallStack->pop();
2247+
}
2248+
if (is_array($sfSelected['php']) && in_array('newThis: stdClass', $sfSelected['php'], true)) {
2249+
$sfClosureBindNarrowed++;
2250+
}
2251+
check($sfSelected['php'] === $sfSelected['native'], sprintf('ParametersAcceptorSelector parity: selectFromArgs() over the Closure::bind() call on line %d: %s vs %s', $sfCall->getStartLine(), json_encode($sfSelected['php']), json_encode($sfSelected['native'])));
2252+
}
2253+
check(count($sfClosureBindCalls) >= 5, 'scope-family: the fixture walk saw the Closure::bind() calls (' . count($sfClosureBindCalls) . ')');
2254+
check($sfClosureBindNarrowed === 4, "scope-family: the closure-this override narrowed \$newThis in 4 Closure::bind() calls ($sfClosureBindNarrowed)");
2255+
22002256
if (getenv('SF_DUMP_LABEL') !== false) {
22012257
$sfDumpLabel = getenv('SF_DUMP_LABEL');
22022258
foreach ($sfObservations['php'] as $sfId => $sfPhpObservations) {

0 commit comments

Comments
 (0)