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..d4c888d8e2f 100644 --- a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php @@ -116,6 +116,21 @@ 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'], [ + // 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, + ], + ]); + } + #[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;