From 4d30ed18e55e135ffdade6da711f4b2a09b4ddfa Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:29:10 +0000 Subject: [PATCH 1/2] Strip the final-by-keyword override from the declaring class before comparing a method's return type with `$this` - `new Foo()` (and `$x::class === Foo::class` narrowing) produces an ObjectType whose ClassReflection is `asFinal()`. Methods looked up on it get that reflection as their declaring class, while the `$this` return type resolved from the PHPDoc holds the plain one. `ObjectType::isSuperTypeOf()` then answers Maybe, so a `@return $this` method was no longer treated as having side effects when called on a freshly instantiated object: remembered method call values were not invalidated and the call was reported as "on a separate line has no effect". - PhpMethodReflection::hasSideEffects() and AnnotationMethodReflection::hasSideEffects() (`@method $this foo()`) now build the ThisType from `declaringClass->withoutFinalByKeywordOverride()`. - ArgumentsHandler: same fix for the check that keeps object arguments of a fluent `$this`-returning method from being invalidated; it was hidden by the bug above and started over-invalidating once hasSideEffects() became Yes. - Mirrored both changes in turbo-ext (PhpMethodReflectionAccess.cpp, ArgumentsHandler.cpp). - Probed the other places that build a type from a method's declaring class (Called*/Callback* unresolved prototype reflections, PhpDocsResolver, MethodSignatureRule); they only transform `static`, so they don't run into this mismatch. Co-Authored-By: Claude Opus 5.5 --- src/Analyser/ArgumentsHandler.php | 2 +- .../AnnotationMethodReflection.php | 2 +- src/Reflection/Php/PhpMethodReflection.php | 2 +- tests/PHPStan/Analyser/nsrt/bug-15320.php | 139 ++++++++++++++++++ ...odStatementWithoutImpurePointsRuleTest.php | 11 ++ .../PHPStan/Rules/DeadCode/data/bug-15320.php | 55 +++++++ turbo-ext/src/ArgumentsHandler.cpp | 8 +- turbo-ext/src/PhpMethodReflectionAccess.cpp | 8 +- 8 files changed, 220 insertions(+), 7 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15320.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/bug-15320.php diff --git a/src/Analyser/ArgumentsHandler.php b/src/Analyser/ArgumentsHandler.php index 4e84ba02c59..38da1a427cb 100644 --- a/src/Analyser/ArgumentsHandler.php +++ b/src/Analyser/ArgumentsHandler.php @@ -836,7 +836,7 @@ public function processArgs( } if ( $nakedReturnType === null - || !(new ThisType($nakedMethodReflection->getDeclaringClass()))->isSuperTypeOf($nakedReturnType)->yes() + || !(new ThisType($nakedMethodReflection->getDeclaringClass()->withoutFinalByKeywordOverride()))->isSuperTypeOf($nakedReturnType)->yes() || $nakedMethodReflection->isPure()->no() ) { $nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($arg->value), $scope, $storage); diff --git a/src/Reflection/Annotations/AnnotationMethodReflection.php b/src/Reflection/Annotations/AnnotationMethodReflection.php index 0da8fbc78b7..14079b0a19e 100644 --- a/src/Reflection/Annotations/AnnotationMethodReflection.php +++ b/src/Reflection/Annotations/AnnotationMethodReflection.php @@ -133,7 +133,7 @@ public function hasSideEffects(): TrinaryLogic return TrinaryLogic::createYes(); } - if ((new ThisType($this->declaringClass))->isSuperTypeOf($this->returnType)->yes()) { + if ((new ThisType($this->declaringClass->withoutFinalByKeywordOverride()))->isSuperTypeOf($this->returnType)->yes()) { return TrinaryLogic::createYes(); } diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index db2f8a8d69c..1de8565b82b 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -376,7 +376,7 @@ public function hasSideEffects(): TrinaryLogic return TrinaryLogic::createFromBoolean(!$this->isPure); } - if ((new ThisType($this->declaringClass))->isSuperTypeOf($this->getReturnType())->yes()) { + if ((new ThisType($this->declaringClass->withoutFinalByKeywordOverride()))->isSuperTypeOf($this->getReturnType())->yes()) { return TrinaryLogic::createYes(); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-15320.php b/tests/PHPStan/Analyser/nsrt/bug-15320.php new file mode 100644 index 00000000000..1669b24bf7f --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15320.php @@ -0,0 +1,139 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15320; + +use function PHPStan\Testing\assertType; + +class Foo +{ + public function testOutClassVoid(): void + { + $m = new Model(); + + assert($m->get('name') === 5); + $m->setVoid('name', 10); + assertType('int', $m->get('name')); + } + + public function testOutClassThis(): void + { + $m = new Model(); + + assert($m->get('name') === 5); + $m->setThis('name', 10); + assertType('int', $m->get('name')); + } + + public function testOutClassStatic(): void + { + $m = new Model(); + + assert($m->get('name') === 5); + $m->setStatic('name', 10); + assertType('5', $m->get('name')); + } + + public function testOutClassThisParameter(Model $m): void + { + assert($m->get('name') === 5); + $m->setThis('name', 10); + assertType('int', $m->get('name')); + } + + public function testOutClassThisArgument(): void + { + $m = new Model(); + $other = new Model(); + + assert($other->get('name') === 5); + $m->setThisFromObject($other); + assertType('5', $other->get('name')); + } + + public function testOutClassAnnotationMethod(): void + { + $m = new MagicModel(); + + assert($m->get('name') === 5); + $m->setMagic('name', 10); + assertType('int', $m->get('name')); + } +} + +class Model +{ + public function setVoid(string $field, int $value): void + { + } + + /** + * @return $this + */ + public function setThis(string $field, int $value): object + { + return $this; + } + + public function setStatic(string $field, int $value): static + { + return $this; + } + + /** + * @return $this + */ + public function setThisFromObject(object $other): object + { + return $this; + } + + public function get(string $field): int + { + return 1; + } + + public function testInClassVoid(): void + { + $m = $this; + + assert($m->get('name') === 5); + $m->setVoid('name', 10); + assertType('int', $m->get('name')); + } + + public function testInClassThis(): void + { + $m = $this; + + assert($m->get('name') === 5); + $m->setThis('name', 10); + assertType('int', $m->get('name')); + } + + public function testInClassStatic(): void + { + $m = $this; + + assert($m->get('name') === 5); + $m->setStatic('name', 10); + assertType('5', $m->get('name')); + } +} + +/** + * @method $this setMagic(string $field, int $value) + */ +class MagicModel +{ + public function get(string $field): int + { + return 1; + } + + public function __call(string $name, array $args): mixed + { + return $this; + } +} diff --git a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php index ff80a78dddd..60a62a59f30 100644 --- a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php @@ -116,6 +116,17 @@ public function testBug12379(): void $this->analyse([__DIR__ . '/data/bug-12379.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15320(): void + { + $this->analyse([__DIR__ . '/data/bug-15320.php'], [ + [ + 'Call to method Bug15320DeadCode\\Model::setStatic() on a separate line has no effect.', + 28, + ], + ]); + } + #[RequiresPhp('>= 8.5.0')] public function testPipeOperator(): void { diff --git a/tests/PHPStan/Rules/DeadCode/data/bug-15320.php b/tests/PHPStan/Rules/DeadCode/data/bug-15320.php new file mode 100644 index 00000000000..6ba8ad82c3d --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/bug-15320.php @@ -0,0 +1,55 @@ += 8.0 + +namespace Bug15320DeadCode; + +class Foo +{ + public function testOutClassVoid(): void + { + $m = new Model(); + + assert($m->get('name') === 5); + $m->setVoid('name', 10); + } + + public function testOutClassThis(): void + { + $m = new Model(); + + assert($m->get('name') === 5); + $m->setThis('name', 10); + } + + public function testOutClassStatic(): void + { + $m = new Model(); + + assert($m->get('name') === 5); + $m->setStatic('name', 10); + } +} + +class Model +{ + public function setVoid(string $field, int $value): void + { + } + + /** + * @return $this + */ + public function setThis(string $field, int $value): object + { + return $this; + } + + public function setStatic(string $field, int $value): static + { + return $this; + } + + public function get(string $field): int + { + return 1; + } +} diff --git a/turbo-ext/src/ArgumentsHandler.cpp b/turbo-ext/src/ArgumentsHandler.cpp index 8551f011a8b..f0c4958fa85 100644 --- a/turbo-ext/src/ArgumentsHandler.cpp +++ b/turbo-ext/src/ArgumentsHandler.cpp @@ -2778,8 +2778,14 @@ class ArgumentsHandler if (!invalidate) { zv::Val declaringClass = calleeGetDeclaringClass(w.nakedMethodReflection); if (UNEXPECTED(declaringClass.isUndef())) return false; + if (UNEXPECTED(Z_TYPE_P(declaringClass.raw()) != IS_OBJECT)) { + zend_throw_error(NULL, "Call to a member function withoutFinalByKeywordOverride() on %s", zend_zval_value_name(declaringClass.raw())); + return false; + } + zv::Val declaringClassWithoutOverride = pt_type_call(Z_OBJ_P(declaringClass.raw()), PT_LC("withoutfinalbykeywordoverride"), 0, NULL); + if (UNEXPECTED(declaringClassWithoutOverride.isUndef())) return false; zval thisType; - if (UNEXPECTED(!pt_this_type_new(&thisType, declaringClass.raw()))) return false; + if (UNEXPECTED(!pt_this_type_new(&thisType, declaringClassWithoutOverride.raw()))) return false; zv::Val thisTypeHold = zv::Val::adopt(thisType); zv::Val isSuperType = pt_type_op(Z_OBJ_P(thisTypeHold.raw()), PT_OP_IS_SUPER_TYPE_OF, 1, nakedReturnType.raw()); if (UNEXPECTED(isSuperType.isUndef())) return false; diff --git a/turbo-ext/src/PhpMethodReflectionAccess.cpp b/turbo-ext/src/PhpMethodReflectionAccess.cpp index d9cfae0c683..38f86548a5f 100644 --- a/turbo-ext/src/PhpMethodReflectionAccess.cpp +++ b/turbo-ext/src/PhpMethodReflectionAccess.cpp @@ -197,11 +197,13 @@ Answer hasSideEffects(zend_object *method, zv::Val &out) return PT_PMR_ANSWERED; } - /* (new ThisType($this->declaringClass))->isSuperTypeOf($this->getReturnType())->yes() */ + /* (new ThisType($this->declaringClass->withoutFinalByKeywordOverride()))->isSuperTypeOf($this->getReturnType())->yes() */ zval *declaringClass = slotOf(method, PT_PMR_DECLARING_CLASS); - if (UNEXPECTED(declaringClass == NULL)) return PT_PMR_UNANSWERED; + if (UNEXPECTED(declaringClass == NULL || Z_TYPE_P(declaringClass) != IS_OBJECT)) return PT_PMR_UNANSWERED; + zv::Val declaringClassWithoutOverride = pt_type_call(Z_OBJ_P(declaringClass), PT_LC("withoutfinalbykeywordoverride"), 0, NULL); + if (UNEXPECTED(declaringClassWithoutOverride.isUndef())) return PT_PMR_EXCEPTION; zval thisTypeZv; - if (UNEXPECTED(!pt_this_type_new(&thisTypeZv, declaringClass))) return PT_PMR_EXCEPTION; + if (UNEXPECTED(!pt_this_type_new(&thisTypeZv, declaringClassWithoutOverride.raw()))) return PT_PMR_EXCEPTION; zv::Val thisType = zv::Val::adopt(thisTypeZv); zv::Val result = pt_type_op(Z_OBJ_P(thisType.raw()), PT_OP_IS_SUPER_TYPE_OF, 1, returnType); if (UNEXPECTED(result.isUndef())) return PT_PMR_EXCEPTION; From 65d34e96f6f5e14312de30e17ee5aa629640aa15 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 4 Nov 2026 11:09:37 +0000 Subject: [PATCH 2/2] Explain why the static-returning method call is still reported in the bug-15320 test Co-Authored-By: Claude Opus 5.5 --- .../CallToMethodStatementWithoutImpurePointsRuleTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php index 60a62a59f30..d4c888d8e2f 100644 --- a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php @@ -120,6 +120,10 @@ public function testBug12379(): void public function testBug15320(): void { $this->analyse([__DIR__ . '/data/bug-15320.php'], [ + // Unlike `void` and `$this`, a `static` return type does not imply side effects, + // because the method may return a new instance (immutable wither). + // setStatic() has no impure points in its body and is called on an exact `new Model()`, + // so discarding its result really has no effect - phpstan/phpstan#15320 expects this error. [ 'Call to method Bug15320DeadCode\\Model::setStatic() on a separate line has no effect.', 28,