From 54a3a2da8558880fe6e2371a2e2bdced0d3121a2 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 14 Nov 2026 13:33:21 -0800 Subject: [PATCH 1/2] Forget narrowed properties after a possibly impure method call when its values are not remembered With `rememberPossiblyImpureFunctionValues: false`, the return value of a method that only maybe has side effects is no longer remembered, but what was narrowed about the properties of the object it was called on still was. So a method that changes a property it was not declared impure for left its callers with a stale type, and a comparison of that property after the call was reported as always true or always false: $this->state = self::STATE_READY; if (!$this->parseNextTag()) { return $this->state === self::STATE_COMPLETE; // reported as always false } When the option is off, a method call and a static call within the declaring class now invalidate the object's properties, and a static call its static properties, unless the method is known to have no side effects, as they already did for a method that has them. With the option on, which is the default, nothing changes. The native MethodCallHandler and StaticCallHandler of the turbo extension get the same condition. The tests cover calls on `$this`, on another object, and through `?->`, `self::`, `static::`, and `parent::`, and that a pure method and a local variable are left alone. A counterpart fixture holds the default behavior, and a rule test shows the comparison above is no longer reported. Co-Authored-By: Claude Opus 5.5 (medium effort) Claude-Code-Session-Id: dfb24569-a61a-4404-8588-60a387d2cccb --- .../ExprHandler/MethodCallHandler.php | 8 +- .../ExprHandler/StaticCallHandler.php | 1 + ...memberPossiblyImpureFunctionValuesTest.php | 1 + ...ibly-impure-function-values-properties.php | 130 ++++++++++++++++++ ...ibly-impure-function-values-properties.php | 130 ++++++++++++++++++ ...otRememberPossiblyImpureValuesRuleTest.php | 52 +++++++ ...on-property-after-possibly-impure-call.php | 44 ++++++ turbo-ext/src/MethodCallHandler.cpp | 5 +- turbo-ext/src/StaticCallHandler.cpp | 3 +- 9 files changed, 371 insertions(+), 3 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php create mode 100644 tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php create mode 100644 tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php create mode 100644 tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index f13426cdd29..9e47d90c470 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -321,7 +321,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $throwPoints[] = $methodThrowPoint; } - if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) { + if ( + $methodReflection->getName() === '__construct' + || $methodReflection->hasSideEffects()->yes() + // When possibly impure values are not remembered, a possibly impure method + // may also have changed the object's properties, so forget them too. + || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) + ) { $nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage); $scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass(), $methodReflection->isStatic()); } diff --git a/src/Analyser/ExprHandler/StaticCallHandler.php b/src/Analyser/ExprHandler/StaticCallHandler.php index e510200d21c..002784a04b4 100644 --- a/src/Analyser/ExprHandler/StaticCallHandler.php +++ b/src/Analyser/ExprHandler/StaticCallHandler.php @@ -382,6 +382,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex && $methodReflection->getName() === '__construct' ) || $methodReflection->hasSideEffects()->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) ) && $scope->isInClass() && $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName()) diff --git a/tests/PHPStan/Analyser/DoNotRememberPossiblyImpureFunctionValuesTest.php b/tests/PHPStan/Analyser/DoNotRememberPossiblyImpureFunctionValuesTest.php index 75a9797ee38..c0ba1711574 100644 --- a/tests/PHPStan/Analyser/DoNotRememberPossiblyImpureFunctionValuesTest.php +++ b/tests/PHPStan/Analyser/DoNotRememberPossiblyImpureFunctionValuesTest.php @@ -11,6 +11,7 @@ class DoNotRememberPossiblyImpureFunctionValuesTest extends TypeInferenceTestCas public static function dataAsserts(): iterable { yield from self::gatherAssertTypes(__DIR__ . '/data/do-not-remember-possibly-impure-function-values.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/do-not-remember-possibly-impure-function-values-properties.php'); } /** diff --git a/tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php b/tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php new file mode 100644 index 00000000000..208a04deff6 --- /dev/null +++ b/tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php @@ -0,0 +1,130 @@ +state = 'done'; + return true; + } + + /** @phpstan-pure */ + public function pure(): bool + { + return true; + } + + /** @phpstan-impure */ + public function impure(): bool + { + $this->state = 'done'; + return true; + } + + public static function advanceStatic(): bool + { + self::$staticState = 'done'; + return true; + } + + /** @phpstan-pure */ + public static function pureStatic(): bool + { + return true; + } + + public function doFoo(): void + { + $this->state = 'ready'; + $this->advance(); + assertType("'done'|'ready'", $this->state); + + $this->state = 'ready'; + if ($this->advance()) { + assertType("'done'|'ready'", $this->state); + } + + $this->state = 'ready'; + $this->pure(); + assertType("'ready'", $this->state); + + $this->state = 'ready'; + $this->impure(); + assertType("'done'|'ready'", $this->state); + } + + public function doLocalVariable(): void + { + $state = 'ready'; + $this->advance(); + assertType("'ready'", $state); + } + + public function doOtherObject(self $other): void + { + $this->state = 'ready'; + $other->state = 'ready'; + $other->advance(); + assertType("'done'|'ready'", $other->state); + assertType("'ready'", $this->state); + } + + public function doNullsafe(?self $other): void + { + if ($other === null) { + return; + } + + $other->state = 'ready'; + $other?->advance(); + assertType("'done'|'ready'", $other->state); + } + + public function doStatic(): void + { + self::$staticState = 'ready'; + self::advanceStatic(); + assertType("'done'|'ready'", self::$staticState); + + self::$staticState = 'ready'; + static::advanceStatic(); + assertType("'done'|'ready'", self::$staticState); + + self::$staticState = 'ready'; + self::pureStatic(); + assertType("'ready'", self::$staticState); + } + + public function doStaticKeepsInstanceProperties(): void + { + $this->state = 'ready'; + self::advanceStatic(); + assertType("'ready'", $this->state); + } + +} + +class ChildStateMachine extends StateMachine +{ + + public function advance(): bool + { + $this->state = 'ready'; + parent::advance(); + assertType("'done'|'ready'", $this->state); + + return true; + } + +} diff --git a/tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php b/tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php new file mode 100644 index 00000000000..e706a09f9ed --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php @@ -0,0 +1,130 @@ +state = 'done'; + return true; + } + + /** @phpstan-pure */ + public function pure(): bool + { + return true; + } + + /** @phpstan-impure */ + public function impure(): bool + { + $this->state = 'done'; + return true; + } + + public static function advanceStatic(): bool + { + self::$staticState = 'done'; + return true; + } + + /** @phpstan-pure */ + public static function pureStatic(): bool + { + return true; + } + + public function doFoo(): void + { + $this->state = 'ready'; + $this->advance(); + assertType("'ready'", $this->state); + + $this->state = 'ready'; + if ($this->advance()) { + assertType("'ready'", $this->state); + } + + $this->state = 'ready'; + $this->pure(); + assertType("'ready'", $this->state); + + $this->state = 'ready'; + $this->impure(); + assertType("'done'|'ready'", $this->state); + } + + public function doLocalVariable(): void + { + $state = 'ready'; + $this->advance(); + assertType("'ready'", $state); + } + + public function doOtherObject(self $other): void + { + $this->state = 'ready'; + $other->state = 'ready'; + $other->advance(); + assertType("'ready'", $other->state); + assertType("'ready'", $this->state); + } + + public function doNullsafe(?self $other): void + { + if ($other === null) { + return; + } + + $other->state = 'ready'; + $other?->advance(); + assertType("'ready'", $other->state); + } + + public function doStatic(): void + { + self::$staticState = 'ready'; + self::advanceStatic(); + assertType("'ready'", self::$staticState); + + self::$staticState = 'ready'; + static::advanceStatic(); + assertType("'ready'", self::$staticState); + + self::$staticState = 'ready'; + self::pureStatic(); + assertType("'ready'", self::$staticState); + } + + public function doStaticKeepsInstanceProperties(): void + { + $this->state = 'ready'; + self::advanceStatic(); + assertType("'ready'", $this->state); + } + +} + +class ChildStateMachine extends StateMachine +{ + + public function advance(): bool + { + $this->state = 'ready'; + parent::advance(); + assertType("'ready'", $this->state); + + return true; + } + +} diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php new file mode 100644 index 00000000000..ae41ad6ec2f --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php @@ -0,0 +1,52 @@ + + */ +class StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + // @phpstan-ignore argument.type + return new CompositeRule([ + new StrictComparisonOfDifferentTypesRule( + self::getContainer()->getByType(RicherScopeGetTypeHelper::class), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + true, + false, + true, + ), + new ConstantConditionInTraitRule(), + ]); + } + + public function testPropertyAfterPossiblyImpureCall(): void + { + $this->analyse([__DIR__ . '/data/strict-comparison-property-after-possibly-impure-call.php'], [ + [ + "Strict comparison using === between 'ready' and 'complete' will always evaluate to false.", + 41, + ], + ]); + } + + public static function getAdditionalConfigFiles(): array + { + return array_merge( + parent::getAdditionalConfigFiles(), + [ + __DIR__ . '/doNotRememberPossiblyImpureValues.neon', + ], + ); + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php b/tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php new file mode 100644 index 00000000000..a49b3f403cd --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php @@ -0,0 +1,44 @@ +state = self::STATE_READY; + if (!$this->parseNextTag()) { + return $this->state === self::STATE_COMPLETE; + } + + return $this->state === self::STATE_MATCHED; + } + + private function parseNextTag(): bool + { + $this->state = rand(0, 1) === 1 ? self::STATE_MATCHED : self::STATE_COMPLETE; + return $this->state === self::STATE_MATCHED; + } + + /** @phpstan-pure */ + private function isReady(): bool + { + return true; + } + + public function afterPureCall(): bool + { + $this->state = self::STATE_READY; + $this->isReady(); + return $this->state === self::STATE_COMPLETE; + } + +} diff --git a/turbo-ext/src/MethodCallHandler.cpp b/turbo-ext/src/MethodCallHandler.cpp index 69c2bef129e..b1e55b41e53 100644 --- a/turbo-ext/src/MethodCallHandler.cpp +++ b/turbo-ext/src/MethodCallHandler.cpp @@ -446,7 +446,10 @@ class MethodCallHandler if (!invalidate) { zend_long hasSideEffects = reflectionTrinary(methodReflection.raw(), PT_MR_HAS_SIDE_EFFECTS); if (UNEXPECTED(hasSideEffects < 0)) return zv::Val(); - invalidate = hasSideEffects == PT_TRI_YES; + // When possibly impure values are not remembered, a possibly impure method + // may also have changed the object's properties, so forget them too. + invalidate = hasSideEffects == PT_TRI_YES + || (!zend_is_true(OBJ_PROP_NUM(self, slots::rememberPossiblyImpureFunctionValues)) && hasSideEffects != PT_TRI_NO); } if (invalidate) { zval *normalizedVar = exprVar(normalizedExpr.raw()); diff --git a/turbo-ext/src/StaticCallHandler.cpp b/turbo-ext/src/StaticCallHandler.cpp index a18c204676d..00b5d777644 100644 --- a/turbo-ext/src/StaticCallHandler.cpp +++ b/turbo-ext/src/StaticCallHandler.cpp @@ -679,7 +679,8 @@ class StaticCallHandler if (!invalidate) { zend_long hasSideEffects = reflectionTrinary(methodReflection.raw(), PT_MR_HAS_SIDE_EFFECTS); if (UNEXPECTED(hasSideEffects < 0)) return zv::Val(); - invalidate = hasSideEffects == PT_TRI_YES; + invalidate = hasSideEffects == PT_TRI_YES + || (!zend_is_true(OBJ_PROP_NUM(self, slots::rememberPossiblyImpureFunctionValues)) && hasSideEffects != PT_TRI_NO); } if (invalidate && UNEXPECTED(!scopeClassIsDeclaringClass(currentScope.raw(), methodReflection.raw(), invalidate))) return zv::Val(); if (invalidate) { From 9cd1f6c26b09fb6bc601024d32908ebe25c99ade Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 14 Nov 2026 19:55:33 -0800 Subject: [PATCH 2/2] Exempt built-in methods from forgetting narrowed properties, and test the issue's reproducer verbatim A possibly impure method declared in a built-in class, such as `ArrayObject::getIterator()` called on a subclass, invalidated the object's narrowed properties when `rememberPossiblyImpureFunctionValues` is false. The invalidation of volatile expressions after the same call already exempts built-in methods, since they do not change userland properties. The new condition now exempts them too, in the native `MethodCallHandler` and `StaticCallHandler` of the turbo extension as well. `hasSideEffects()` is no longer evaluated twice in the same condition; it is not memoized. In `StaticCallHandler`, the condition is nested once so the value can be held in a variable. The rule test's fixture is now `bug-15734.php`, the reproducer from the issue copied verbatim, and it expects no errors. The comparison after a pure call that the old fixture also covered is already covered by the type inference fixture. Both type inference fixtures gain cases for a method that asserts a property of `$this` with `@phpstan-assert-if-true`, for a static call made from a static method, and for calls to built-in methods. Co-Authored-By: Claude Opus 5.5 (medium effort) Claude-Session: https://claude.ai/code/session_01Tnky3JtcvQrtFGa2fwY9Ay Claude-Code-Session-Id: dfb24569-a61a-4404-8588-60a387d2cccb --- .../ExprHandler/MethodCallHandler.php | 10 +++- .../ExprHandler/StaticCallHandler.php | 36 ++++++++------ ...ibly-impure-function-values-properties.php | 47 +++++++++++++++++++ ...ibly-impure-function-values-properties.php | 47 +++++++++++++++++++ ...otRememberPossiblyImpureValuesRuleTest.php | 9 +--- .../Rules/Comparison/data/bug-15734.php | 27 +++++++++++ ...on-property-after-possibly-impure-call.php | 44 ----------------- turbo-ext/src/MethodCallHandler.cpp | 11 ++++- turbo-ext/src/StaticCallHandler.cpp | 13 ++++- 9 files changed, 173 insertions(+), 71 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-15734.php delete mode 100644 tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index 9e47d90c470..3e1369dc800 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -321,12 +321,18 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $throwPoints[] = $methodThrowPoint; } + $hasSideEffects = $methodReflection->hasSideEffects(); if ( $methodReflection->getName() === '__construct' - || $methodReflection->hasSideEffects()->yes() + || $hasSideEffects->yes() // When possibly impure values are not remembered, a possibly impure method // may also have changed the object's properties, so forget them too. - || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) + // Built-in methods are exempt, as they are from invalidating volatile expressions below. + || ( + !$this->rememberPossiblyImpureFunctionValues + && !$hasSideEffects->no() + && !$methodReflection->getDeclaringClass()->isBuiltin() + ) ) { $nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage); $scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass(), $methodReflection->isStatic()); diff --git a/src/Analyser/ExprHandler/StaticCallHandler.php b/src/Analyser/ExprHandler/StaticCallHandler.php index 002784a04b4..cd2a867a6ef 100644 --- a/src/Analyser/ExprHandler/StaticCallHandler.php +++ b/src/Analyser/ExprHandler/StaticCallHandler.php @@ -373,22 +373,30 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex } } - if ( - $expr->class instanceof Name - && $methodReflection !== null - && ( + if ($expr->class instanceof Name && $methodReflection !== null) { + $hasSideEffects = $methodReflection->hasSideEffects(); + if ( ( - !$methodReflection->isStatic() - && $methodReflection->getName() === '__construct' + ( + !$methodReflection->isStatic() + && $methodReflection->getName() === '__construct' + ) + || $hasSideEffects->yes() + // When possibly impure values are not remembered, a possibly impure method + // may also have changed the object's properties, so forget them too. + // Built-in methods are exempt, as they are from invalidating volatile expressions below. + || ( + !$this->rememberPossiblyImpureFunctionValues + && !$hasSideEffects->no() + && !$methodReflection->getDeclaringClass()->isBuiltin() + ) ) - || $methodReflection->hasSideEffects()->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) - ) - && $scope->isInClass() - && $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName()) - ) { - // a static method never receives $this, so property fetches on it survive - $scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass(), $methodReflection->isStatic()); + && $scope->isInClass() + && $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName()) + ) { + // a static method never receives $this, so property fetches on it survive + $scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass(), $methodReflection->isStatic()); + } } if ( diff --git a/tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php b/tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php index 208a04deff6..5c8686d8605 100644 --- a/tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php +++ b/tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values-properties.php @@ -113,6 +113,53 @@ public function doStaticKeepsInstanceProperties(): void assertType("'ready'", $this->state); } + public static function doStaticFromStaticMethod(): void + { + self::$staticState = 'ready'; + self::advanceStatic(); + assertType("'done'|'ready'", self::$staticState); + + self::$staticState = 'ready'; + self::pureStatic(); + assertType("'ready'", self::$staticState); + } + +} + +class Holder +{ + + public ?\stdClass $foo = null; + + /** @phpstan-assert-if-true !null $this->foo */ + public function hasFoo(): bool + { + return $this->foo !== null; + } + + public function doAssertIfTrue(): void + { + if ($this->hasFoo()) { + assertType('stdClass', $this->foo); + } + } + +} + +/** @extends \ArrayObject */ +class Bag extends \ArrayObject +{ + + public ?int $n = null; + + public function doBuiltinMethod(): void + { + $this->n = 1; + $this->count(); + $this->getIterator(); + assertType('1', $this->n); + } + } class ChildStateMachine extends StateMachine diff --git a/tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php b/tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php index e706a09f9ed..bc9226c3bb6 100644 --- a/tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php +++ b/tests/PHPStan/Analyser/nsrt/remember-possibly-impure-function-values-properties.php @@ -113,6 +113,53 @@ public function doStaticKeepsInstanceProperties(): void assertType("'ready'", $this->state); } + public static function doStaticFromStaticMethod(): void + { + self::$staticState = 'ready'; + self::advanceStatic(); + assertType("'ready'", self::$staticState); + + self::$staticState = 'ready'; + self::pureStatic(); + assertType("'ready'", self::$staticState); + } + +} + +class Holder +{ + + public ?\stdClass $foo = null; + + /** @phpstan-assert-if-true !null $this->foo */ + public function hasFoo(): bool + { + return $this->foo !== null; + } + + public function doAssertIfTrue(): void + { + if ($this->hasFoo()) { + assertType('stdClass', $this->foo); + } + } + +} + +/** @extends \ArrayObject */ +class Bag extends \ArrayObject +{ + + public ?int $n = null; + + public function doBuiltinMethod(): void + { + $this->n = 1; + $this->count(); + $this->getIterator(); + assertType('1', $this->n); + } + } class ChildStateMachine extends StateMachine diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php index ae41ad6ec2f..9967f47f82b 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesDoNotRememberPossiblyImpureValuesRuleTest.php @@ -29,14 +29,9 @@ protected function getRule(): Rule ]); } - public function testPropertyAfterPossiblyImpureCall(): void + public function testBug15734(): void { - $this->analyse([__DIR__ . '/data/strict-comparison-property-after-possibly-impure-call.php'], [ - [ - "Strict comparison using === between 'ready' and 'complete' will always evaluate to false.", - 41, - ], - ]); + $this->analyse([__DIR__ . '/data/bug-15734.php'], []); } public static function getAdditionalConfigFiles(): array diff --git a/tests/PHPStan/Rules/Comparison/data/bug-15734.php b/tests/PHPStan/Rules/Comparison/data/bug-15734.php new file mode 100644 index 00000000000..3293c4b64ce --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-15734.php @@ -0,0 +1,27 @@ += 7.4 + +declare(strict_types = 1); + +namespace Bug15734; + +class Tokenizer +{ + /** @var 'ready'|'matched'|'complete' */ + private string $state = 'ready'; + + public function nextToken(): bool + { + $this->state = 'ready'; + if (!$this->parseNextTag()) { + return $this->state === 'complete'; + } + + return $this->state === 'matched'; + } + + private function parseNextTag(): bool + { + $this->state = rand(0, 1) === 1 ? 'matched' : 'complete'; + return $this->state === 'matched'; + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php b/tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php deleted file mode 100644 index a49b3f403cd..00000000000 --- a/tests/PHPStan/Rules/Comparison/data/strict-comparison-property-after-possibly-impure-call.php +++ /dev/null @@ -1,44 +0,0 @@ -state = self::STATE_READY; - if (!$this->parseNextTag()) { - return $this->state === self::STATE_COMPLETE; - } - - return $this->state === self::STATE_MATCHED; - } - - private function parseNextTag(): bool - { - $this->state = rand(0, 1) === 1 ? self::STATE_MATCHED : self::STATE_COMPLETE; - return $this->state === self::STATE_MATCHED; - } - - /** @phpstan-pure */ - private function isReady(): bool - { - return true; - } - - public function afterPureCall(): bool - { - $this->state = self::STATE_READY; - $this->isReady(); - return $this->state === self::STATE_COMPLETE; - } - -} diff --git a/turbo-ext/src/MethodCallHandler.cpp b/turbo-ext/src/MethodCallHandler.cpp index b1e55b41e53..118fdff32f2 100644 --- a/turbo-ext/src/MethodCallHandler.cpp +++ b/turbo-ext/src/MethodCallHandler.cpp @@ -446,10 +446,17 @@ class MethodCallHandler if (!invalidate) { zend_long hasSideEffects = reflectionTrinary(methodReflection.raw(), PT_MR_HAS_SIDE_EFFECTS); if (UNEXPECTED(hasSideEffects < 0)) return zv::Val(); + invalidate = hasSideEffects == PT_TRI_YES; // When possibly impure values are not remembered, a possibly impure method // may also have changed the object's properties, so forget them too. - invalidate = hasSideEffects == PT_TRI_YES - || (!zend_is_true(OBJ_PROP_NUM(self, slots::rememberPossiblyImpureFunctionValues)) && hasSideEffects != PT_TRI_NO); + // Built-in methods are exempt, as they are from invalidating volatile expressions below. + if (!invalidate && !zend_is_true(OBJ_PROP_NUM(self, slots::rememberPossiblyImpureFunctionValues)) && hasSideEffects != PT_TRI_NO) { + zv::Val declaringClass = pt_extended_method_reflection_call(methodReflection.raw(), PT_MR_GET_DECLARING_CLASS); + if (UNEXPECTED(declaringClass.isUndef())) return zv::Val(); + bool builtin; + if (UNEXPECTED(!pt_class_reflection_is_builtin(Z_OBJ_P(declaringClass.raw()), builtin))) return zv::Val(); + invalidate = !builtin; + } } if (invalidate) { zval *normalizedVar = exprVar(normalizedExpr.raw()); diff --git a/turbo-ext/src/StaticCallHandler.cpp b/turbo-ext/src/StaticCallHandler.cpp index 00b5d777644..cb31dc8e056 100644 --- a/turbo-ext/src/StaticCallHandler.cpp +++ b/turbo-ext/src/StaticCallHandler.cpp @@ -679,8 +679,17 @@ class StaticCallHandler if (!invalidate) { zend_long hasSideEffects = reflectionTrinary(methodReflection.raw(), PT_MR_HAS_SIDE_EFFECTS); if (UNEXPECTED(hasSideEffects < 0)) return zv::Val(); - invalidate = hasSideEffects == PT_TRI_YES - || (!zend_is_true(OBJ_PROP_NUM(self, slots::rememberPossiblyImpureFunctionValues)) && hasSideEffects != PT_TRI_NO); + invalidate = hasSideEffects == PT_TRI_YES; + // When possibly impure values are not remembered, a possibly impure method + // may also have changed the object's properties, so forget them too. + // Built-in methods are exempt, as they are from invalidating volatile expressions below. + if (!invalidate && !zend_is_true(OBJ_PROP_NUM(self, slots::rememberPossiblyImpureFunctionValues)) && hasSideEffects != PT_TRI_NO) { + zv::Val declaringClass = pt_extended_method_reflection_call(methodReflection.raw(), PT_MR_GET_DECLARING_CLASS); + if (UNEXPECTED(declaringClass.isUndef())) return zv::Val(); + bool builtin; + if (UNEXPECTED(!pt_class_reflection_is_builtin(Z_OBJ_P(declaringClass.raw()), builtin))) return zv::Val(); + invalidate = !builtin; + } } if (invalidate && UNEXPECTED(!scopeClassIsDeclaringClass(currentScope.raw(), methodReflection.raw(), invalidate))) return zv::Val(); if (invalidate) {