Skip to content

Commit ab12474

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 544dbd5 commit ab12474

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
@@ -855,7 +855,28 @@ public function testClosureBindParamClosureThisNamedArguments(): void
855855
$this->checkThisOnly = false;
856856
$this->checkExplicitMixed = true;
857857
$this->checkImplicitMixed = true;
858-
$this->analyse([__DIR__ . '/data/closure-bind-param-closure-this-named.php'], []);
858+
$this->analyse([__DIR__ . '/data/closure-bind-param-closure-this-named.php'], [
859+
[
860+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
861+
19,
862+
],
863+
[
864+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
865+
20,
866+
],
867+
[
868+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
869+
21,
870+
],
871+
[
872+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
873+
22,
874+
],
875+
[
876+
'Parameter $newThis of static method Closure::bind() expects stdClass, ClosureBindParamClosureThisNamed\\Foo given.',
877+
23,
878+
],
879+
]);
859880
}
860881

861882
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
@@ -2238,6 +2238,62 @@ public function exprRef(Node\Expr $expr): mixed
22382238
}
22392239
}
22402240

2241+
// ---- ParametersAcceptorSelector::selectFromArgs() over the fixture's
2242+
// Closure::bind() calls: the @param-closure-this override of $newThis, with
2243+
// the closure argument found by position or by the visitor's attribute.
2244+
// The walk above parses the fixture as a non-analysed file (function bodies
2245+
// cleaned away), so these calls come from a walk of the rich parser's AST,
2246+
// which also carries the visitors' attributes. Both sides get the walk scope
2247+
// and the walk's own (PHP) variants, so only the selector differs
2248+
/** @var list<array{\PhpParser\Node\Expr\StaticCall, \PHPStan\Analyser\MutatingScope, \PHPStan\Analyser\ExpressionResultStorage|null}> $sfClosureBindCalls */
2249+
$sfClosureBindCalls = [];
2250+
$sfClosureBindCallback = static function (\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope) use (&$sfClosureBindCalls): void {
2251+
if (
2252+
!$node instanceof \PhpParser\Node\Expr\StaticCall
2253+
|| !$node->class instanceof \PhpParser\Node\Name
2254+
|| $node->class->toLowerString() !== 'closure'
2255+
|| !$node->name instanceof \PhpParser\Node\Identifier
2256+
|| $node->name->toLowerString() !== 'bind'
2257+
) {
2258+
return;
2259+
}
2260+
$walkScope = $scope->toWalkScope();
2261+
$sfClosureBindCalls[] = [$node, $walkScope, $walkScope->getCurrentExpressionResultStorage()];
2262+
};
2263+
$sfResolver->processNodes(
2264+
$scContainer->getService('cachedRichParser')->parseFile($sfFile),
2265+
$sfScopeFactory->create(\PHPStan\Analyser\ScopeContext::create($sfFile), $sfClosureBindCallback),
2266+
$sfClosureBindCallback,
2267+
);
2268+
$sfClosureBindVariants = $sfReflectionProvider->getClass(\Closure::class)->getNativeMethod('bind')->getVariants();
2269+
$sfClosureBindNarrowed = 0;
2270+
foreach ($sfClosureBindCalls as $sfCallIndex => [$sfCall, $sfCallScope, $sfCallStorage]) {
2271+
$sfCallStack = $sfHarness->constructorArgs($sfCallScope)['services']->expressionResultStorageStack;
2272+
$sfCallStack->push($sfCallStorage ?? new \PHPStan\Analyser\ExpressionResultStorage());
2273+
try {
2274+
$sfSelected = [];
2275+
foreach (['php' => \PHPStan\Reflection\ParametersAcceptorSelector::class, 'native' => \PHPStanTurbo\ParametersAcceptorSelector::class] as $side => $sfSelector) {
2276+
try {
2277+
$sfAcceptor = $sfSelector::selectFromArgs($sfCallScope, $sfCall->getArgs(), $sfClosureBindVariants);
2278+
$sfSelected[$side] = array_map(
2279+
static fn ($parameter): string => $parameter->getName() . ': ' . $parameter->getType()->describe(\PHPStan\Type\VerbosityLevel::precise()),
2280+
$sfAcceptor->getParameters(),
2281+
);
2282+
} catch (\Throwable $e) {
2283+
$sfSelected[$side] = get_class($e) . ': ' . $e->getMessage();
2284+
}
2285+
}
2286+
} finally {
2287+
$sfCallStack->pop();
2288+
}
2289+
if (is_array($sfSelected['php']) && in_array('newThis: stdClass', $sfSelected['php'], true)) {
2290+
$sfClosureBindNarrowed++;
2291+
}
2292+
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'])));
2293+
}
2294+
check(count($sfClosureBindCalls) >= 5, 'scope-family: the fixture walk saw the Closure::bind() calls (' . count($sfClosureBindCalls) . ')');
2295+
check($sfClosureBindNarrowed === 4, "scope-family: the closure-this override narrowed \$newThis in 4 Closure::bind() calls ($sfClosureBindNarrowed)");
2296+
22412297
if (getenv('SF_DUMP_LABEL') !== false) {
22422298
$sfDumpLabel = getenv('SF_DUMP_LABEL');
22432299
foreach ($sfObservations['php'] as $sfId => $sfPhpObservations) {

0 commit comments

Comments
 (0)