Repository navigation
Conversation
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 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
This PR makes
$thisinside aClosure::bind()closure the type of the object that is actually bound ($newThis), refined by$newScope, instead of the scope class. Until now the scope class replaced the bound object's type whenever a scope was given.What was wrong
Closure::bind(fn () => $this, null, Foo::class)typed$thisasFoo, but PHP has no$thisthere ("Using $this when not in object context").Closure::bind(fn () => $this->im(), new NoParent(), Foo::class)was silent, but PHP fails with "Call to undefined method NoParent::im()".Cause
The
Closure::bind()scope factory inStaticCallHandlertook$thisfrom$newThis, then overwrote it with the$newScopeclass (or the object type of the class-string) as soon as a scope argument was present. The native$thistype and the scope classes used for visibility andself::were already right; only the PHPDoc$thistype ignored the bound object.New behaviour
$newThis, or anullone: no$this, whatever the scope.objector mixed bound intoFoo::class:Foo, so the hydrator idiomClosure::bind(fn () => $this->secret, $object, Foo::class)keeps working.SubFoobound intoFoo::class:SubFoo.Foobound intoSubFoo::class:SubFoo.NoParentbound intoFoo::class:NoParent, so undefined methods and properties are reported.$thisof the enclosing class bound into an unrelated class scope:$this(Outer)is kept. Into an interface scope:$this(Outer)&FooInterface.Foo|NoParentbound intoFoo::class:Foo|NoParent. Each member of the union is refined on its own, so a member that cannot be an instance of the scope class is not silently dropped.?Foo: staysFoo|null, asclosure-bind-nullable-this.phpalready pins.'static'scope, or a class-string that names no known class: was*ERROR*, now the bound object's type.$this. PHP itself returns null with a warning when an instance is bound to a static closure, so the rows without$thismatch PHP.Trade-off
A member of the bound object's type that may be an instance of the scope class is assumed to be one.
objector mixed becomes the scope class, which keeps the hydrator idiom working; at runtime a non-Fooobject there would only produce an undefined-property warning. Likewise aFoobound intoSubFoo::classis typed asSubFoo. The native$thistype keeps the unrefined bound-object type.This is the same result as before this PR for these cases, so their existing false positives and false negatives stay: a guard such as
!$this instanceof Fooor$this instanceof SubFoois reported as always true (and the code after it as unreachable), and an unguarded$this->secretor$this->onlyInSubFoo()on an object that turns out not to be the scope class goes unreported. WithtreatPhpDocTypesAsCertain: falsethe guards are not reported, because the native$thisis not narrowed. Runtime behaviour: https://3v4l.org/dhunm#v, PHPStan output: https://phpstan.org/r/51fb207e-3211-4a84-8223-34fd75956707Related
Closure::call()already takes$thisfrom the object it is called with. PHPStan does not re-analysebindTo()closure bodies with a new$thisat all. The scope factory still reads theClosure::bind()arguments by position; named-argument support forClosure::bind()(marking the closure inClosureBindArgVisitor, reordering the arguments in the factory, the lookup inParametersAcceptorSelector) is being split out of #4081 into a separate PR.The native C++ twin in
turbo-ext/src/StaticCallHandler.cppgets the same change, with a walk-trace fixture covering each path.This is split out of #4081, whose
self::instance-call check relies on$thisbeing the bound object rather than the scope class.