From 6137ecd3820c4309577ac3220bd2b5f23748ebbe Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Tue, 6 Oct 2026 00:14:04 +0900 Subject: [PATCH 1/2] Type $this in a Closure::bind() closure from the bound object The Closure::bind() scope factory took $this from $newThis but then overwrote it with the $newScope class whenever a scope was given, so a closure bound to null or to an unrelated object still saw $this as an instance of the scope class: `Closure::bind(fn () => $this, null, Foo::class)` typed $this as Foo although PHP has no $this there, and `Closure::bind(fn () => $this->im(), new NoParent(), Foo::class)` was silent although PHP fails with a call to undefined method NoParent::im(). $this is now the bound object's type, refined by the scope: each member of $newThis's type that may be an instance of the scope class is intersected with it, a member that cannot be one is kept as it is. - no or null $newThis: no $this, whatever the scope - `object` (or mixed) $newThis: the scope class, so the hydrator idiom `Closure::bind(fn () => $this->secret, $object, Foo::class)` keeps reading Foo's private members - a subclass instance bound into its parent's scope: the subclass - an unrelated object: that object - a union like Foo|NoParent: Foo|NoParent - a nullable $newThis: stays nullable, as closure-bind-nullable-this.php pins it The members are refined one by one because intersecting the whole type drops every member that cannot be an instance of the scope - Foo|NoParent would become Foo and ?Foo would become Foo - turning a possible runtime error into a silently narrower $this. A member that may be an instance of the scope class is assumed to be one: `object` or mixed becomes the scope class, and a Foo bound into SubFoo::class becomes SubFoo. At runtime a non-Foo object in the hydrator idiom only gets an undefined-property warning; the native $this keeps the unrefined type. The scope classes (accessibility, self::) and the native $this type, which already came from $newThis only, are unchanged. A 'static' scope or a class-string naming no class no longer turns $this into *ERROR*: the bound object's type is kept. Co-authored-by: Claude Opus 5.5 --- .../ExprHandler/StaticCallHandler.php | 29 ++- .../Analyser/nsrt/closure-bind-this-type.php | 175 ++++++++++++++++++ .../Rules/Methods/CallMethodsRuleTest.php | 17 ++ .../data/closure-bind-this-from-new-this.php | 36 ++++ .../Properties/AccessPropertiesRuleTest.php | 14 ++ .../data/closure-bind-this-from-new-this.php | 34 ++++ ...edVariableRuleWithoutLoopPollutionTest.php | 16 ++ .../data/closure-bind-this-from-new-this.php | 24 +++ turbo-ext/src/StaticCallHandler.cpp | 57 +++++- .../walk-trace-fixtures/closure-bind-this.php | 87 +++++++++ 10 files changed, 476 insertions(+), 13 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/closure-bind-this-type.php create mode 100644 tests/PHPStan/Rules/Methods/data/closure-bind-this-from-new-this.php create mode 100644 tests/PHPStan/Rules/Properties/data/closure-bind-this-from-new-this.php create mode 100644 tests/PHPStan/Rules/Variables/data/closure-bind-this-from-new-this.php create mode 100644 turbo-ext/tests/walk-trace-fixtures/closure-bind-this.php diff --git a/src/Analyser/ExprHandler/StaticCallHandler.php b/src/Analyser/ExprHandler/StaticCallHandler.php index 2b3d12f6ef3..1b111a53917 100644 --- a/src/Analyser/ExprHandler/StaticCallHandler.php +++ b/src/Analyser/ExprHandler/StaticCallHandler.php @@ -57,6 +57,7 @@ use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use PHPStan\Type\TypeWithClassName; +use PHPStan\Type\UnionType; use ReflectionProperty; use function array_map; use function array_merge; @@ -193,14 +194,32 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $directClassNames = $argValueType->getObjectClassNames(); if (count($directClassNames) > 0) { $scopeClasses = $directClassNames; - $thisTypes = []; + $scopeTypes = []; foreach ($directClassNames as $directClassName) { - $thisTypes[] = new ObjectType($directClassName); + $scopeTypes[] = new ObjectType($directClassName); } - $thisType = TypeCombinator::union(...$thisTypes); + $scopeType = TypeCombinator::union(...$scopeTypes); } else { - $thisType = $argValueType->getClassStringObjectType(); - $scopeClasses = $thisType->getObjectClassNames(); + $scopeType = $argValueType->getClassStringObjectType(); + $scopeClasses = $scopeType->getObjectClassNames(); + } + + // $this is the bound object, not an instance of the scope + // class: the scope only refines the members of the bound + // object's type that may be an instance of it (the + // hydrator idiom binding an `object` into the class whose + // members it reads), a member that cannot be one stays + if ($thisType !== null && count($scopeClasses) > 0) { + $thisMembers = $thisType instanceof UnionType ? $thisType->getTypes() : [$thisType]; + $refinedThisMembers = []; + foreach ($thisMembers as $thisMember) { + if ($scopeType->isSuperTypeOf($thisMember)->no()) { + $refinedThisMembers[] = $thisMember; + continue; + } + $refinedThisMembers[] = TypeCombinator::intersect($thisMember, $scopeType); + } + $thisType = TypeCombinator::union(...$refinedThisMembers); } } return $boundScope->enterClosureBind($thisType, $nativeThisType, $scopeClasses); diff --git a/tests/PHPStan/Analyser/nsrt/closure-bind-this-type.php b/tests/PHPStan/Analyser/nsrt/closure-bind-this-type.php new file mode 100644 index 00000000000..26ed37ddc87 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/closure-bind-this-type.php @@ -0,0 +1,175 @@ += 8.0 + +namespace ClosureBindThisType; + +use Closure; +use function PHPStan\Testing\assertNativeType; +use function PHPStan\Testing\assertType; + +interface FooInterface +{ + +} + +class Foo +{ + + private int $secret = 1; + +} + +class SubFoo extends Foo +{ + +} + +class NoParent +{ + +} + +final class FinalNoParent +{ + +} + +class Test +{ + + /** + * @param class-string $fooClass + */ + public function doFoo( + object $object, + Foo $foo, + SubFoo $subFoo, + NoParent $noParent, + FinalNoParent $finalNoParent, + ?Foo $nullableFoo, + Foo|NoParent $fooOrNoParent, + string $fooClass, + $mixed, + ): void + { + // no bound object: there is no $this, whatever the scope + Closure::bind(function () { + assertType('*ERROR*', $this); + assertNativeType('*ERROR*', $this); + }, null, Foo::class); + + // the scope class refines a bound object that might be an instance of it + Closure::bind(function () { + assertType('ClosureBindThisType\Foo', $this); + assertNativeType('object', $this); + }, $object, Foo::class); + + Closure::bind(function () { + assertType('ClosureBindThisType\SubFoo', $this); + assertNativeType('ClosureBindThisType\SubFoo', $this); + }, $subFoo, Foo::class); + + Closure::bind(function () { + assertType('ClosureBindThisType\Foo', $this); + assertNativeType('ClosureBindThisType\Foo', $this); + }, $foo, Foo::class); + + Closure::bind(function () { + assertType('ClosureBindThisType\FooInterface&ClosureBindThisType\NoParent', $this); + assertNativeType('ClosureBindThisType\NoParent', $this); + }, $noParent, FooInterface::class); + + // a bound object that may be an instance of the scope class is assumed to be one + Closure::bind(function () { + assertType('ClosureBindThisType\Foo', $this); + assertNativeType('mixed', $this); + }, $mixed, Foo::class); + + Closure::bind(function () { + assertType('ClosureBindThisType\SubFoo', $this); + assertNativeType('ClosureBindThisType\Foo', $this); + }, $foo, SubFoo::class); + + // binding $this into another class's scope keeps the enclosing $this + Closure::bind(function () { + assertType('$this(ClosureBindThisType\Test)', $this); + assertNativeType('$this(ClosureBindThisType\Test)', $this); + }, $this, Foo::class); + + Closure::bind(function () { + assertType('$this(ClosureBindThisType\Test)&ClosureBindThisType\FooInterface', $this); + assertNativeType('$this(ClosureBindThisType\Test)', $this); + }, $this, FooInterface::class); + + // ... but never replaces a bound object that cannot be one + Closure::bind(function () { + assertType('ClosureBindThisType\NoParent', $this); + assertNativeType('ClosureBindThisType\NoParent', $this); + }, $noParent, Foo::class); + + Closure::bind(function () { + assertType('ClosureBindThisType\FinalNoParent', $this); + assertNativeType('ClosureBindThisType\FinalNoParent', $this); + }, $finalNoParent, FooInterface::class); + + Closure::bind(function () { + assertType('ClosureBindThisType\Foo|ClosureBindThisType\NoParent', $this); + assertNativeType('ClosureBindThisType\Foo|ClosureBindThisType\NoParent', $this); + }, $fooOrNoParent, Foo::class); + + Closure::bind(function () { + assertType('ClosureBindThisType\Foo|null', $this); + assertNativeType('ClosureBindThisType\Foo|null', $this); + }, $nullableFoo, Foo::class); + + // without a scope the bound object is $this as it is + Closure::bind(function () { + assertType('ClosureBindThisType\NoParent', $this); + assertNativeType('ClosureBindThisType\NoParent', $this); + }, $noParent); + + Closure::bind(function () { + assertType('object', $this); + assertNativeType('object', $this); + }, $object); + + // the scope given as an object + Closure::bind(function () { + assertType('ClosureBindThisType\Foo', $this); + assertNativeType('object', $this); + }, $object, $foo); + + Closure::bind(function () { + assertType('ClosureBindThisType\NoParent', $this); + assertNativeType('ClosureBindThisType\NoParent', $this); + }, $noParent, $foo); + + // the scope given as a class-string + Closure::bind(function () { + assertType('ClosureBindThisType\Foo', $this); + assertNativeType('object', $this); + }, $object, $fooClass); + + Closure::bind(function () { + assertType('ClosureBindThisType\NoParent', $this); + assertNativeType('ClosureBindThisType\NoParent', $this); + }, $noParent, $fooClass); + + // 'static' keeps the current scope, it names no class + Closure::bind(function () { + assertType('object', $this); + assertNativeType('object', $this); + }, $object, 'static'); + + // arrow functions + Closure::bind(fn () => assertType('ClosureBindThisType\NoParent', $this), $noParent, Foo::class); + Closure::bind(fn () => assertType('ClosureBindThisType\Foo', $this), $object, Foo::class); + Closure::bind(fn () => assertType('*ERROR*', $this), null, Foo::class); + + // a static closure has no $this to bind + Closure::bind(static function () { + assertType('*ERROR*', $this); + }, $object, Foo::class); + Closure::bind(static fn () => assertType('*ERROR*', $this), $object, Foo::class); + } + +} diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index e03a97fc739..8170bf9fb3d 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -1097,6 +1097,23 @@ public function testBug4865(): void $this->analyse([__DIR__ . '/data/bug-4865.php'], []); } + public function testClosureBindThisFromNewThis(): void + { + $this->checkThisOnly = false; + $this->checkNullables = true; + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/data/closure-bind-this-from-new-this.php'], [ + [ + 'Call to an undefined method ClosureBindThisFromNewThis\\NoParent::im().', + 26, + ], + [ + 'Call to an undefined method ClosureBindThisFromNewThis\\NoParent::im().', + 28, + ], + ]); + } + #[RequiresPhp('>= 8.0.0')] public function testBug14715(): void { diff --git a/tests/PHPStan/Rules/Methods/data/closure-bind-this-from-new-this.php b/tests/PHPStan/Rules/Methods/data/closure-bind-this-from-new-this.php new file mode 100644 index 00000000000..3127a55cf2e --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/closure-bind-this-from-new-this.php @@ -0,0 +1,36 @@ +im(); + }, $noParent, Foo::class); + \Closure::bind(fn () => $this->im(), new NoParent(), Foo::class); + + // the hydrator idiom: an `object` bound into the class whose private members it calls + \Closure::bind(function () { + $this->im(); + }, $object, Foo::class); + } + +} diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index 11329e9f8b5..2391267e2a6 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -941,6 +941,20 @@ public function testBug393(): void $this->analyse([__DIR__ . '/data/bug-393.php'], []); } + public function testClosureBindThisFromNewThis(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = true; + $this->checkDynamicProperties = false; + $this->analyse([__DIR__ . '/data/closure-bind-this-from-new-this.php'], [ + [ + 'Access to an undefined property ClosureBindThisFromNewThisProperties\\NoParent::$secret.', + 30, + 'Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property', + ], + ]); + } + public function testObjectShapes(): void { $this->checkThisOnly = false; diff --git a/tests/PHPStan/Rules/Properties/data/closure-bind-this-from-new-this.php b/tests/PHPStan/Rules/Properties/data/closure-bind-this-from-new-this.php new file mode 100644 index 00000000000..9dd5c055591 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/closure-bind-this-from-new-this.php @@ -0,0 +1,34 @@ +secret; + }, $object, Foo::class); + $secret = \Closure::bind(fn () => $this->secret, $object, Foo::class); + + // $this is the bound object, which has no $secret + $secret = \Closure::bind(function () { + return $this->secret; + }, $noParent, Foo::class); + } + +} diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleWithoutLoopPollutionTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleWithoutLoopPollutionTest.php index b68002fdb4e..40c3291ce76 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleWithoutLoopPollutionTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleWithoutLoopPollutionTest.php @@ -788,6 +788,22 @@ public function testBug393(): void $this->analyse([__DIR__ . '/data/bug-393.php'], []); } + public function testClosureBindThisFromNewThis(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->checkMaybeUndefinedVariables = true; + $this->analyse([__DIR__ . '/data/closure-bind-this-from-new-this.php'], [ + [ + 'Undefined variable: $this', + 17, + ], + [ + 'Undefined variable: $this', + 19, + ], + ]); + } + public function testBug9023(): void { $this->cliArgumentsVariablesRegistered = true; diff --git a/tests/PHPStan/Rules/Variables/data/closure-bind-this-from-new-this.php b/tests/PHPStan/Rules/Variables/data/closure-bind-this-from-new-this.php new file mode 100644 index 00000000000..93db8c5072b --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/closure-bind-this-from-new-this.php @@ -0,0 +1,24 @@ +doFoo(); + }, null, Foo::class); + \Closure::bind(fn () => $this->doFoo(), null, Foo::class); + + \Closure::bind(function () { + $this->doFoo(); + }, $foo, Foo::class); +}; diff --git a/turbo-ext/src/StaticCallHandler.cpp b/turbo-ext/src/StaticCallHandler.cpp index 2a3d0246406..84c0ca61e45 100644 --- a/turbo-ext/src/StaticCallHandler.cpp +++ b/turbo-ext/src/StaticCallHandler.cpp @@ -1394,13 +1394,14 @@ class StaticCallHandler zv::Val argValueType = readArgType(boundScope, storage, value, false); if (UNEXPECTED(argValueType.isUndef())) return; + zv::Val scopeType; zv::Val directClassNames = pt_type_op(Z_OBJ_P(argValueType.raw()), PT_OP_GET_OBJECT_CLASS_NAMES, 0, NULL); if (UNEXPECTED(directClassNames.isUndef())) return; zend_long directClassCount = countOf(directClassNames.raw()); if (UNEXPECTED(directClassCount < 0)) return; if (directClassCount > 0) { scopeClasses = zv::Val::copyOf(zv::Ref(directClassNames.raw())); - zv::Arr thisTypes = zv::Arr::create((uint32_t) directClassCount); + zv::Arr scopeTypes = zv::Arr::create((uint32_t) directClassCount); for (zv::ArrayEntry entry : zv::ArrRef(directClassNames.raw())) { zval *directClassName = entry.value().deref().raw(); if (UNEXPECTED(Z_TYPE_P(directClassName) != IS_STRING)) { @@ -1409,18 +1410,58 @@ class StaticCallHandler } zval objectType; if (UNEXPECTED(!pt_object_type_new(&objectType, Z_STR_P(directClassName)))) return; - thisTypes.push(zv::Val::adopt(objectType)); + scopeTypes.push(zv::Val::adopt(objectType)); } - HashTable *thisTypesTable = thisTypes.table(); - thisType = pt_type_combinator_union(zend_hash_num_elements(thisTypesTable), thisTypesTable->arPacked); - if (UNEXPECTED(thisType.isUndef())) return; + HashTable *scopeTypesTable = scopeTypes.table(); + scopeType = pt_type_combinator_union(zend_hash_num_elements(scopeTypesTable), scopeTypesTable->arPacked); + if (UNEXPECTED(scopeType.isUndef())) return; } else { - thisType = getClassStringObjectType(argValueType.raw()); - if (UNEXPECTED(thisType.isUndef())) return; - zv::Val classNames = pt_type_op(Z_OBJ_P(thisType.raw()), PT_OP_GET_OBJECT_CLASS_NAMES, 0, NULL); + scopeType = getClassStringObjectType(argValueType.raw()); + if (UNEXPECTED(scopeType.isUndef())) return; + zv::Val classNames = pt_type_op(Z_OBJ_P(scopeType.raw()), PT_OP_GET_OBJECT_CLASS_NAMES, 0, NULL); if (UNEXPECTED(classNames.isUndef())) return; scopeClasses = std::move(classNames); } + + // $this is the bound object, not an instance of the scope + // class: the scope only refines the members of the bound + // object's type that may be an instance of it (the + // hydrator idiom binding an `object` into the class whose + // members it reads), a member that cannot be one stays + zend_long scopeClassCount = countOf(scopeClasses.raw()); + if (UNEXPECTED(scopeClassCount < 0)) return; + if (!thisType.isNull() && scopeClassCount > 0) { + zv::Val thisMembers; + if (instanceof_function(Z_OBJCE_P(thisType.raw()), pt_ce_union_type)) { + thisMembers = pt_union_type_get_types(Z_OBJ_P(thisType.raw())); + if (UNEXPECTED(thisMembers.isUndef())) return; + } else { + zv::Arr single = zv::Arr::create(1); + single.push(zv::Ref(thisType.raw())); + thisMembers = zv::Val(std::move(single)); + } + zend_long thisMemberCount = countOf(thisMembers.raw()); + if (UNEXPECTED(thisMemberCount < 0)) return; + zv::Arr refinedThisMembers = zv::Arr::create((uint32_t) thisMemberCount); + for (zv::ArrayEntry entry : zv::ArrRef(thisMembers.raw())) { + zval *thisMember = entry.value().deref().raw(); + zv::Val isSuperType = pt_type_op(Z_OBJ_P(scopeType.raw()), PT_OP_IS_SUPER_TYPE_OF, 1, thisMember); + if (UNEXPECTED(isSuperType.isUndef())) return; + zend_long isSuperTypeValue = pt_type_result_trinary(isSuperType.raw()); + if (UNEXPECTED(isSuperTypeValue < 0)) return; + if (isSuperTypeValue == PT_TRI_NO) { + refinedThisMembers.push(zv::Ref(thisMember)); + continue; + } + zv::Args intersectArgv{thisMember, scopeType.raw()}; + zv::Val refinedThisMember = pt_type_combinator_intersect(2, intersectArgv); + if (UNEXPECTED(refinedThisMember.isUndef())) return; + refinedThisMembers.push(std::move(refinedThisMember)); + } + HashTable *refinedThisMembersTable = refinedThisMembers.table(); + thisType = pt_type_combinator_union(zend_hash_num_elements(refinedThisMembersTable), refinedThisMembersTable->arPacked); + if (UNEXPECTED(thisType.isUndef())) return; + } } zv::Val bound = pt_mutating_scope_enter_closure_bind(Z_OBJ_P(boundScope), thisType.raw(), nativeThisType.raw(), scopeClasses.raw()); if (UNEXPECTED(bound.isUndef())) return; diff --git a/turbo-ext/tests/walk-trace-fixtures/closure-bind-this.php b/turbo-ext/tests/walk-trace-fixtures/closure-bind-this.php new file mode 100644 index 00000000000..6e137480f04 --- /dev/null +++ b/turbo-ext/tests/walk-trace-fixtures/closure-bind-this.php @@ -0,0 +1,87 @@ + $fooClass + */ +function bindAll( + object $object, + Foo $foo, + SubFoo $subFoo, + NoParent $noParent, + FinalNoParent $finalNoParent, + ?Foo $nullableFoo, + Foo|NoParent $fooOrNoParent, + string $fooClass, + $mixed, +): void +{ + \Closure::bind(function () { + return $this; + }, null, Foo::class); + \Closure::bind(function () { + return $this->secret; + }, $object, Foo::class); + \Closure::bind(fn () => $this, $subFoo, Foo::class); + \Closure::bind(fn () => $this, $foo, Foo::class); + \Closure::bind(fn () => $this, $noParent, FooInterface::class); + \Closure::bind(fn () => $this, $noParent, Foo::class); + \Closure::bind(fn () => $this, $finalNoParent, FooInterface::class); + \Closure::bind(fn () => $this, $fooOrNoParent, Foo::class); + \Closure::bind(fn () => $this, $nullableFoo, Foo::class); + \Closure::bind(fn () => $this, $noParent); + \Closure::bind(fn () => $this, $object, $foo); + \Closure::bind(fn () => $this, $noParent, $foo); + \Closure::bind(fn () => $this, $object, $fooClass); + \Closure::bind(fn () => $this, $noParent, $fooClass); + \Closure::bind(fn () => $this, $object, 'static'); + \Closure::bind(static fn () => $this, $object, Foo::class); + \Closure::bind(fn () => $this, $mixed, Foo::class); + \Closure::bind(fn () => $this, $foo, SubFoo::class); +} + +class Outer +{ + + public function bindThis(): void + { + \Closure::bind(fn () => $this, $this, Foo::class); + \Closure::bind(fn () => $this, $this, FooInterface::class); + } + +} From 361b630d7dc765d3cb9c00f4ecca63f3a06975bc Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Tue, 6 Oct 2026 00:36:19 +0900 Subject: [PATCH 2/2] Bump expected turbo version Co-authored-by: Claude Opus 5.5 --- src/Turbo/TurboExtensionEnabler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index a0c9829a03e..ad61213c52b 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -33,7 +33,7 @@ final class TurboExtensionEnabler { - public const EXPECTED_EXTENSION_VERSION = '6351afb'; + public const EXPECTED_EXTENSION_VERSION = '6137ecd'; private static bool $active = false;