From 193abeaa4d0c9248292f96895eab56f481d64a91 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:29:56 +0000 Subject: [PATCH 1/2] Do not remember constructor property initialization for classes with custom serialization or `unset($this->prop)` * `MutatingScope::rememberConstructorExpressions()` now drops the `PropertyInitializationExpr` entries (and the readonly `PropertyFetch` value entries) it used to carry from the constructor scope into the other methods when the class declares `__sleep()`, `__serialize()`, `__unserialize()`, or implements `Serializable` with an `unserialize()` method - `unserialize()` rebuilds such an object without running the constructor and the author decides which properties survive the round trip. * `MutatingScope::unsetInitializedProperty()` + the `Unset_` branch of `NodeScopeResolver` remove the initialization fact when `unset($this->prop)` runs, so `$this->prop ?? ...` right after an `unset()` is no longer reported as redundant (and an `unset()` at the end of the constructor now correctly leaves the property uninitialized). * `NodeScopeResolver::getUnsetPropertiesInCurrentClassLike()` collects the properties `unset()` anywhere in the class body and passes them to `rememberConstructorScope()`, covering the case where the `unset()` lives in a different method than the `??`/`isset()`/`empty()` check. * The same code path backs `nullCoalesce.initializedProperty`, `isset.initializedProperty` and `empty.initializedProperty`, and the fix also covers promoted constructor properties, `??=`, anonymous classes, inherited and trait-provided `__sleep()`. `__wakeup()` alone still keeps the assumption, since default serialization round-trips every initialized property. --- src/Analyser/MutatingScope.php | 69 +++- src/Analyser/NodeScopeResolver.php | 50 ++- tests/PHPStan/Analyser/nsrt/bug-15056.php | 51 +++ .../PHPStan/Rules/Variables/EmptyRuleTest.php | 30 ++ .../PHPStan/Rules/Variables/IssetRuleTest.php | 30 ++ .../Rules/Variables/NullCoalesceRuleTest.php | 39 +++ .../Rules/Variables/data/bug-15056.php | 30 ++ ...ty-initialization-custom-serialization.php | 317 ++++++++++++++++++ .../data/property-initialization-unset.php | 72 ++++ 9 files changed, 681 insertions(+), 7 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15056.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-15056.php create mode 100644 tests/PHPStan/Rules/Variables/data/property-initialization-custom-serialization.php create mode 100644 tests/PHPStan/Rules/Variables/data/property-initialization-unset.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 0f33ced2e3..59134833fe 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -102,6 +102,7 @@ use PHPStan\Type\UnionType; use PHPStan\Type\VerbosityLevel; use PHPStan\Type\VoidType; +use Serializable; use Throwable; use function abs; use function array_filter; @@ -141,6 +142,9 @@ class MutatingScope implements Scope, NodeCallbackInvoker, CollectedDataEmitter public const KEEP_VOID_ATTRIBUTE_NAME = 'keepVoid'; private const COMPLEX_UNION_TYPE_MEMBER_LIMIT = 8; + /** Magic methods that let the author decide which properties survive a serialize()/unserialize() round trip. */ + private const CUSTOM_SERIALIZATION_METHODS = ['__sleep', '__serialize', '__unserialize']; + /** * @internal accessed by ScopeOps (native and PHP implementations) * @var array @@ -302,10 +306,12 @@ public function enterDeclareStrictTypes(): self /** * @param array $currentExpressionTypes + * @param array $propertyNamesToForget * @return array */ - private function rememberConstructorExpressions(array $currentExpressionTypes): array + private function rememberConstructorExpressions(array $currentExpressionTypes, array $propertyNamesToForget): array { + $rememberPropertyState = !$this->classHasCustomSerialization(); $expressionTypes = []; foreach ($currentExpressionTypes as $exprString => $expressionTypeHolder) { $expr = $expressionTypeHolder->getExpr(); @@ -318,10 +324,21 @@ private function rememberConstructorExpressions(array $currentExpressionTypes): continue; } } elseif ($expr instanceof PropertyFetch) { - if (!$this->isReadonlyPropertyFetch($expr, true)) { + if (!$rememberPropertyState || !$this->isReadonlyPropertyFetch($expr, true)) { continue; } - } elseif (!$expr instanceof ConstFetch && !$expr instanceof PropertyInitializationExpr) { + + if ( + $expr->name instanceof Identifier + && array_key_exists($expr->name->toString(), $propertyNamesToForget) + ) { + continue; + } + } elseif ($expr instanceof PropertyInitializationExpr) { + if (!$rememberPropertyState || array_key_exists($expr->getPropertyName(), $propertyNamesToForget)) { + continue; + } + } elseif (!$expr instanceof ConstFetch) { continue; } @@ -335,15 +352,41 @@ private function rememberConstructorExpressions(array $currentExpressionTypes): return $expressionTypes; } - public function rememberConstructorScope(): self + /** + * A class with custom serialization logic can be rebuilt by unserialize() + * without the constructor ever running, and the author decides which properties + * make the round trip - so nothing the constructor established can be relied upon + * in the other methods. + */ + private function classHasCustomSerialization(): bool + { + if (!$this->isInClass()) { + return false; + } + + $classReflection = $this->getClassReflection(); + foreach (self::CUSTOM_SERIALIZATION_METHODS as $methodName) { + if ($classReflection->hasNativeMethod($methodName)) { + return true; + } + } + + return $classReflection->implementsInterface(Serializable::class) + && $classReflection->hasNativeMethod('unserialize'); + } + + /** + * @param array $propertyNamesToForget + */ + public function rememberConstructorScope(array $propertyNamesToForget = []): self { return $this->scopeFactory->create( $this->context, $this->isDeclareStrictTypes(), null, $this->getNamespace(), - $this->rememberConstructorExpressions($this->expressionTypes), - $this->rememberConstructorExpressions($this->nativeExpressionTypes), + $this->rememberConstructorExpressions($this->expressionTypes, $propertyNamesToForget), + $this->rememberConstructorExpressions($this->nativeExpressionTypes, $propertyNamesToForget), $this->conditionalExpressions, $this->inClosureBindScopeClasses, $this->anonymousFunctionReflection, @@ -3172,6 +3215,20 @@ public function assignInitializedProperty(Type $fetchedOnType, string $propertyN return $scope; } + /** unset() makes a typed property uninitialized again, undoing what assignInitializedProperty() recorded. */ + public function unsetInitializedProperty(Type $fetchedOnType, string $propertyName): self + { + if (!$this->isInClass()) { + return $this; + } + + if (TypeUtils::findThisType($fetchedOnType) === null) { + return $this; + } + + return $this->invalidateExpression(new PropertyInitializationExpr($propertyName)); + } + public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self { $exprStringToInvalidate = $this->getNodeKey($expressionToInvalidate); diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 244217b17a..c88c586a1d 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -214,6 +214,12 @@ class NodeScopeResolver /** @var array */ private array $calledMethodResults = []; + /** @var Node\Stmt[] */ + private array $currentClassLikeStatements = []; + + /** @var array|null lazily resolved for $currentClassLikeStatements */ + private ?array $unsetPropertiesInCurrentClassLike = null; + /** * @param ExtensionsCollection $functionParameterOutTypeExtensions * @param ExtensionsCollection $methodParameterOutTypeExtensions @@ -1066,7 +1072,7 @@ public function processStmtNode( } if ($finalScope !== null) { - $scope = $finalScope->rememberConstructorScope(); + $scope = $finalScope->rememberConstructorScope($this->getUnsetPropertiesInCurrentClassLike()); } } @@ -1266,7 +1272,15 @@ public function processStmtNode( return [!$a->isStatic(), $a->name->toLowerString() !== '__construct'] <=> [!$b->isStatic(), $b->name->toLowerString() !== '__construct']; }); + $previousClassLikeStatements = $this->currentClassLikeStatements; + $previousUnsetProperties = $this->unsetPropertiesInCurrentClassLike; + $this->currentClassLikeStatements = $classLikeStatements; + $this->unsetPropertiesInCurrentClassLike = null; + $this->processStmtNodesInternal($stmt, $classLikeStatements, $classScope, $storage, $classStatementsGatherer, $context); + + $this->currentClassLikeStatements = $previousClassLikeStatements; + $this->unsetPropertiesInCurrentClassLike = $previousUnsetProperties; $this->callNodeCallback($nodeCallback, new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensions, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classStatementsGatherer->getPropertyAssigns(), $classReflection), $classScope, $storage); $this->callNodeCallback($nodeCallback, new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls(), $classReflection), $classScope, $storage); $this->callNodeCallback($nodeCallback, new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches(), $classReflection), $classScope, $storage); @@ -2431,6 +2445,9 @@ public function processStmtNode( }; $scope = $this->processVirtualAssign($scope, $storage, $stmt, $buildExistingChain($var->var), new UnsetOffsetExpr($var->var, $var->dim), $nodeCallback)->getScope(); } elseif ($var instanceof PropertyFetch) { + if ($var->name instanceof Node\Identifier) { + $scope = $scope->unsetInitializedProperty($scope->getType($var->var), $var->name->toString()); + } $scope = $scope->invalidateExpression($var); $impurePoints[] = new ImpurePoint( $scope, @@ -2684,6 +2701,37 @@ private function getOverridingThrowPoints(Node\Stmt $statement, MutatingScope $s return null; } + /** + * Properties unset() anywhere in the class body cannot be assumed initialized in the + * other methods just because the constructor assigned them - unset() can run in between. + * + * @return array + */ + private function getUnsetPropertiesInCurrentClassLike(): array + { + if ($this->unsetPropertiesInCurrentClassLike !== null) { + return $this->unsetPropertiesInCurrentClassLike; + } + + $propertyNames = []; + foreach ((new NodeFinder())->findInstanceOf($this->currentClassLikeStatements, Unset_::class) as $unset) { + foreach ($unset->vars as $var) { + if ( + !$var instanceof PropertyFetch + || !$var->name instanceof Node\Identifier + || !$var->var instanceof Variable + || $var->var->name !== 'this' + ) { + continue; + } + + $propertyNames[$var->name->toString()] = true; + } + } + + return $this->unsetPropertiesInCurrentClassLike = $propertyNames; + } + private function getCurrentClassReflection(Node\Stmt\ClassLike $stmt, string $className, Scope $scope): ClassReflection { if (!$this->reflectionProvider->hasClass($className)) { diff --git a/tests/PHPStan/Analyser/nsrt/bug-15056.php b/tests/PHPStan/Analyser/nsrt/bug-15056.php new file mode 100644 index 0000000000..ff3decb5bc --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15056.php @@ -0,0 +1,51 @@ += 8.1 + +declare(strict_types = 1); + +namespace Bug15056; + +use function PHPStan\Testing\assertType; + +class ServiceWithSleep +{ + private readonly string $readonlyString; + private string $string; + + public function __construct() + { + $this->readonlyString = 'foo'; + $this->string = 'bar'; + } + + public function doFoo(): void + { + assertType('string', $this->readonlyString); + assertType('string|null', $this->readonlyString ?? null); + assertType('string|null', $this->string ?? null); + } + + /** @return list */ + public function __sleep(): array + { + return []; + } +} + +class ServiceWithoutSleep +{ + private readonly string $readonlyString; + private string $string; + + public function __construct() + { + $this->readonlyString = 'foo'; + $this->string = 'bar'; + } + + public function doFoo(): void + { + assertType("'foo'", $this->readonlyString); + assertType("'foo'", $this->readonlyString ?? null); + assertType('string|null', $this->string ?? null); + } +} diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index 6ab498be10..4ef814d848 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -257,4 +257,34 @@ public function testNullCoalesceAssignRightSideScope(): void $this->analyse([__DIR__ . '/data/null-coalesce-assign-right-side-scope.php'], []); } + #[RequiresPhp('>= 8.2')] + public function testPropertyInitializationCustomSerialization(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/property-initialization-custom-serialization.php'], [ + [ + 'Property PropertyInitializationCustomSerialization\NoSerialization::$true in empty() is not falsy nor uninitialized.', + 23, + ], + [ + 'Property PropertyInitializationCustomSerialization\OnlyWakeup::$true in empty() is not falsy nor uninitialized.', + 44, + ], + ]); + } + + #[RequiresPhp('>= 8.2')] + public function testPropertyInitializationUnset(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/property-initialization-unset.php'], [ + [ + 'Property PropertyInitializationUnset\NoUnset::$true in empty() is not falsy nor uninitialized.', + 21, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index db77f2aef5..cda49c340a 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -518,6 +518,36 @@ public function testIssetAfterRememberedConstructor(): void ]); } + #[RequiresPhp('>= 8.2')] + public function testPropertyInitializationCustomSerialization(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/property-initialization-custom-serialization.php'], [ + [ + 'Property PropertyInitializationCustomSerialization\NoSerialization::$string in isset() is not nullable nor uninitialized.', + 21, + ], + [ + 'Property PropertyInitializationCustomSerialization\OnlyWakeup::$string in isset() is not nullable nor uninitialized.', + 42, + ], + ]); + } + + #[RequiresPhp('>= 8.2')] + public function testPropertyInitializationUnset(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/property-initialization-unset.php'], [ + [ + 'Property PropertyInitializationUnset\NoUnset::$string in isset() is not nullable nor uninitialized.', + 19, + ], + ]); + } + public function testPr4374(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 1635fe7170..637967d010 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -597,6 +597,45 @@ public function testNullCoalesceAssignRightSideScope(): void ]); } + public function testBug15056(): void + { + $this->analyse([__DIR__ . '/data/bug-15056.php'], []); + } + + #[RequiresPhp('>= 8.2')] + public function testPropertyInitializationCustomSerialization(): void + { + $this->analyse([__DIR__ . '/data/property-initialization-custom-serialization.php'], [ + [ + 'Property PropertyInitializationCustomSerialization\NoSerialization::$string on left side of ?? is not nullable nor uninitialized.', + 20, + ], + [ + 'Property PropertyInitializationCustomSerialization\OnlyWakeup::$string on left side of ?? is not nullable nor uninitialized.', + 41, + ], + [ + 'Property PropertyInitializationCustomSerialization\PromotedNoSerialization::$string on left side of ?? is not nullable nor uninitialized.', + 238, + ], + [ + 'Property PropertyInitializationCustomSerialization\CoalesceAssignNoSerialization::$string on left side of ??= is not nullable nor uninitialized.', + 271, + ], + ]); + } + + #[RequiresPhp('>= 8.2')] + public function testPropertyInitializationUnset(): void + { + $this->analyse([__DIR__ . '/data/property-initialization-unset.php'], [ + [ + 'Property PropertyInitializationUnset\NoUnset::$string on left side of ?? is not nullable nor uninitialized.', + 18, + ], + ]); + } + public function testBug15046(): void { $this->analyse([__DIR__ . '/data/bug-15046.php'], [ diff --git a/tests/PHPStan/Rules/Variables/data/bug-15056.php b/tests/PHPStan/Rules/Variables/data/bug-15056.php new file mode 100644 index 0000000000..cbbb8d9f0e --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-15056.php @@ -0,0 +1,30 @@ +initDependencies($priceInfo); + } + + public function getPriceInfo(): string + { + $this->initDependencies($this->priceInfo ?? null); + + return $this->priceInfo; + } + + protected function initDependencies(?string $priceInfo): void + { + $this->priceInfo = $priceInfo ?? 'foobar'; // in reality: fetch from contiainer + } + + public function __sleep(): array + { + return []; // priceInfo will be lost on serialization + } +} diff --git a/tests/PHPStan/Rules/Variables/data/property-initialization-custom-serialization.php b/tests/PHPStan/Rules/Variables/data/property-initialization-custom-serialization.php new file mode 100644 index 0000000000..499f913c17 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/property-initialization-custom-serialization.php @@ -0,0 +1,317 @@ += 8.2 + +namespace PropertyInitializationCustomSerialization; + +use Serializable; + +class NoSerialization +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } +} + +class OnlyWakeup +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } + + public function __wakeup(): void + { + } +} + +class Sleep +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } + + /** @return list */ + public function __sleep(): array + { + return []; + } +} + +class SerializeAndUnserialize +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } + + /** @return array */ + public function __serialize(): array + { + return []; + } + + /** @param array $data */ + public function __unserialize(array $data): void + { + } +} + +class OnlyUnserialize +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } + + /** @param array $data */ + public function __unserialize(array $data): void + { + } +} + +class ParentWithSleep +{ + /** @return list */ + public function __sleep(): array + { + return []; + } +} + +class InheritsSleep extends ParentWithSleep +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } +} + +trait SleepTrait +{ + /** @return list */ + public function __sleep(): array + { + return []; + } +} + +class SleepFromTrait +{ + use SleepTrait; + + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } +} + +class OldSchoolSerializable implements Serializable +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } + + public function serialize(): string + { + return ''; + } + + public function unserialize(string $data): void + { + } +} + +class PromotedNoSerialization +{ + public function __construct(private string $string) + { + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + } +} + +class PromotedSleep +{ + public function __construct(private string $string) + { + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + } + + /** @return list */ + public function __sleep(): array + { + return []; + } +} + +class CoalesceAssignNoSerialization +{ + private string $string; + + public function __construct() + { + $this->string = 'foo'; + } + + public function doFoo(): void + { + echo $this->string ??= 'default'; + } +} + +class CoalesceAssignSleep +{ + private string $string; + + public function __construct() + { + $this->string = 'foo'; + } + + public function doFoo(): void + { + echo $this->string ??= 'default'; + } + + /** @return list */ + public function __sleep(): array + { + return []; + } +} + +function anonymousClassWithSleep(): object +{ + return new class { + private string $string; + + public function __construct() + { + $this->string = 'foo'; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + } + + /** @return list */ + public function __sleep(): array + { + return []; + } + }; +} diff --git a/tests/PHPStan/Rules/Variables/data/property-initialization-unset.php b/tests/PHPStan/Rules/Variables/data/property-initialization-unset.php new file mode 100644 index 0000000000..07cb616bd0 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/property-initialization-unset.php @@ -0,0 +1,72 @@ += 8.2 + +namespace PropertyInitializationUnset; + +class NoUnset +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } +} + +class UnsetInSameMethod +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function doFoo(): void + { + unset($this->string, $this->true); + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } +} + +class UnsetInAnotherMethod +{ + private string $string; + private true $true; + + public function __construct() + { + $this->string = 'foo'; + $this->true = true; + } + + public function reset(): void + { + unset($this->string, $this->true); + } + + public function doFoo(): void + { + echo $this->string ?? 'default'; + if (isset($this->string)) { + } + if (empty($this->true)) { + } + } +} From e8160b0e7ac5fd4ee4639c779aaa966520713654 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Thu, 6 Aug 2026 14:49:26 +0000 Subject: [PATCH 2/2] Drop the unset($this->prop) part, keep only the custom-serialization fix Reviewer feedback: keep this PR focused on the serialization use-case. Reverts `unsetInitializedProperty()`, the `$propertyNamesToForget` parameter threaded through `rememberConstructorScope()` / `rememberConstructorExpressions()`, and the class-body `unset()` scan in NodeScopeResolver, along with the `property-initialization-unset.php` test data and the three rule tests using it. `unset($this->prop)` can be handled separately. Co-Authored-By: Claude Opus 5 --- src/Analyser/MutatingScope.php | 35 ++------- src/Analyser/NodeScopeResolver.php | 50 +------------ .../PHPStan/Rules/Variables/EmptyRuleTest.php | 13 ---- .../PHPStan/Rules/Variables/IssetRuleTest.php | 13 ---- .../Rules/Variables/NullCoalesceRuleTest.php | 11 --- .../data/property-initialization-unset.php | 72 ------------------- 6 files changed, 6 insertions(+), 188 deletions(-) delete mode 100644 tests/PHPStan/Rules/Variables/data/property-initialization-unset.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 59134833fe..59626e60ab 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -306,10 +306,9 @@ public function enterDeclareStrictTypes(): self /** * @param array $currentExpressionTypes - * @param array $propertyNamesToForget * @return array */ - private function rememberConstructorExpressions(array $currentExpressionTypes, array $propertyNamesToForget): array + private function rememberConstructorExpressions(array $currentExpressionTypes): array { $rememberPropertyState = !$this->classHasCustomSerialization(); $expressionTypes = []; @@ -327,15 +326,8 @@ private function rememberConstructorExpressions(array $currentExpressionTypes, a if (!$rememberPropertyState || !$this->isReadonlyPropertyFetch($expr, true)) { continue; } - - if ( - $expr->name instanceof Identifier - && array_key_exists($expr->name->toString(), $propertyNamesToForget) - ) { - continue; - } } elseif ($expr instanceof PropertyInitializationExpr) { - if (!$rememberPropertyState || array_key_exists($expr->getPropertyName(), $propertyNamesToForget)) { + if (!$rememberPropertyState) { continue; } } elseif (!$expr instanceof ConstFetch) { @@ -375,18 +367,15 @@ private function classHasCustomSerialization(): bool && $classReflection->hasNativeMethod('unserialize'); } - /** - * @param array $propertyNamesToForget - */ - public function rememberConstructorScope(array $propertyNamesToForget = []): self + public function rememberConstructorScope(): self { return $this->scopeFactory->create( $this->context, $this->isDeclareStrictTypes(), null, $this->getNamespace(), - $this->rememberConstructorExpressions($this->expressionTypes, $propertyNamesToForget), - $this->rememberConstructorExpressions($this->nativeExpressionTypes, $propertyNamesToForget), + $this->rememberConstructorExpressions($this->expressionTypes), + $this->rememberConstructorExpressions($this->nativeExpressionTypes), $this->conditionalExpressions, $this->inClosureBindScopeClasses, $this->anonymousFunctionReflection, @@ -3215,20 +3204,6 @@ public function assignInitializedProperty(Type $fetchedOnType, string $propertyN return $scope; } - /** unset() makes a typed property uninitialized again, undoing what assignInitializedProperty() recorded. */ - public function unsetInitializedProperty(Type $fetchedOnType, string $propertyName): self - { - if (!$this->isInClass()) { - return $this; - } - - if (TypeUtils::findThisType($fetchedOnType) === null) { - return $this; - } - - return $this->invalidateExpression(new PropertyInitializationExpr($propertyName)); - } - public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self { $exprStringToInvalidate = $this->getNodeKey($expressionToInvalidate); diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index c88c586a1d..244217b17a 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -214,12 +214,6 @@ class NodeScopeResolver /** @var array */ private array $calledMethodResults = []; - /** @var Node\Stmt[] */ - private array $currentClassLikeStatements = []; - - /** @var array|null lazily resolved for $currentClassLikeStatements */ - private ?array $unsetPropertiesInCurrentClassLike = null; - /** * @param ExtensionsCollection $functionParameterOutTypeExtensions * @param ExtensionsCollection $methodParameterOutTypeExtensions @@ -1072,7 +1066,7 @@ public function processStmtNode( } if ($finalScope !== null) { - $scope = $finalScope->rememberConstructorScope($this->getUnsetPropertiesInCurrentClassLike()); + $scope = $finalScope->rememberConstructorScope(); } } @@ -1272,15 +1266,7 @@ public function processStmtNode( return [!$a->isStatic(), $a->name->toLowerString() !== '__construct'] <=> [!$b->isStatic(), $b->name->toLowerString() !== '__construct']; }); - $previousClassLikeStatements = $this->currentClassLikeStatements; - $previousUnsetProperties = $this->unsetPropertiesInCurrentClassLike; - $this->currentClassLikeStatements = $classLikeStatements; - $this->unsetPropertiesInCurrentClassLike = null; - $this->processStmtNodesInternal($stmt, $classLikeStatements, $classScope, $storage, $classStatementsGatherer, $context); - - $this->currentClassLikeStatements = $previousClassLikeStatements; - $this->unsetPropertiesInCurrentClassLike = $previousUnsetProperties; $this->callNodeCallback($nodeCallback, new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensions, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classStatementsGatherer->getPropertyAssigns(), $classReflection), $classScope, $storage); $this->callNodeCallback($nodeCallback, new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls(), $classReflection), $classScope, $storage); $this->callNodeCallback($nodeCallback, new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches(), $classReflection), $classScope, $storage); @@ -2445,9 +2431,6 @@ public function processStmtNode( }; $scope = $this->processVirtualAssign($scope, $storage, $stmt, $buildExistingChain($var->var), new UnsetOffsetExpr($var->var, $var->dim), $nodeCallback)->getScope(); } elseif ($var instanceof PropertyFetch) { - if ($var->name instanceof Node\Identifier) { - $scope = $scope->unsetInitializedProperty($scope->getType($var->var), $var->name->toString()); - } $scope = $scope->invalidateExpression($var); $impurePoints[] = new ImpurePoint( $scope, @@ -2701,37 +2684,6 @@ private function getOverridingThrowPoints(Node\Stmt $statement, MutatingScope $s return null; } - /** - * Properties unset() anywhere in the class body cannot be assumed initialized in the - * other methods just because the constructor assigned them - unset() can run in between. - * - * @return array - */ - private function getUnsetPropertiesInCurrentClassLike(): array - { - if ($this->unsetPropertiesInCurrentClassLike !== null) { - return $this->unsetPropertiesInCurrentClassLike; - } - - $propertyNames = []; - foreach ((new NodeFinder())->findInstanceOf($this->currentClassLikeStatements, Unset_::class) as $unset) { - foreach ($unset->vars as $var) { - if ( - !$var instanceof PropertyFetch - || !$var->name instanceof Node\Identifier - || !$var->var instanceof Variable - || $var->var->name !== 'this' - ) { - continue; - } - - $propertyNames[$var->name->toString()] = true; - } - } - - return $this->unsetPropertiesInCurrentClassLike = $propertyNames; - } - private function getCurrentClassReflection(Node\Stmt\ClassLike $stmt, string $className, Scope $scope): ClassReflection { if (!$this->reflectionProvider->hasClass($className)) { diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index 4ef814d848..1f7ad2be15 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -274,17 +274,4 @@ public function testPropertyInitializationCustomSerialization(): void ]); } - #[RequiresPhp('>= 8.2')] - public function testPropertyInitializationUnset(): void - { - $this->treatPhpDocTypesAsCertain = true; - - $this->analyse([__DIR__ . '/data/property-initialization-unset.php'], [ - [ - 'Property PropertyInitializationUnset\NoUnset::$true in empty() is not falsy nor uninitialized.', - 21, - ], - ]); - } - } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index cda49c340a..e660eb2bcf 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -535,19 +535,6 @@ public function testPropertyInitializationCustomSerialization(): void ]); } - #[RequiresPhp('>= 8.2')] - public function testPropertyInitializationUnset(): void - { - $this->treatPhpDocTypesAsCertain = true; - - $this->analyse([__DIR__ . '/data/property-initialization-unset.php'], [ - [ - 'Property PropertyInitializationUnset\NoUnset::$string in isset() is not nullable nor uninitialized.', - 19, - ], - ]); - } - public function testPr4374(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 637967d010..79e82d25c4 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -625,17 +625,6 @@ public function testPropertyInitializationCustomSerialization(): void ]); } - #[RequiresPhp('>= 8.2')] - public function testPropertyInitializationUnset(): void - { - $this->analyse([__DIR__ . '/data/property-initialization-unset.php'], [ - [ - 'Property PropertyInitializationUnset\NoUnset::$string on left side of ?? is not nullable nor uninitialized.', - 18, - ], - ]); - } - public function testBug15046(): void { $this->analyse([__DIR__ . '/data/bug-15046.php'], [ diff --git a/tests/PHPStan/Rules/Variables/data/property-initialization-unset.php b/tests/PHPStan/Rules/Variables/data/property-initialization-unset.php deleted file mode 100644 index 07cb616bd0..0000000000 --- a/tests/PHPStan/Rules/Variables/data/property-initialization-unset.php +++ /dev/null @@ -1,72 +0,0 @@ -= 8.2 - -namespace PropertyInitializationUnset; - -class NoUnset -{ - private string $string; - private true $true; - - public function __construct() - { - $this->string = 'foo'; - $this->true = true; - } - - public function doFoo(): void - { - echo $this->string ?? 'default'; - if (isset($this->string)) { - } - if (empty($this->true)) { - } - } -} - -class UnsetInSameMethod -{ - private string $string; - private true $true; - - public function __construct() - { - $this->string = 'foo'; - $this->true = true; - } - - public function doFoo(): void - { - unset($this->string, $this->true); - echo $this->string ?? 'default'; - if (isset($this->string)) { - } - if (empty($this->true)) { - } - } -} - -class UnsetInAnotherMethod -{ - private string $string; - private true $true; - - public function __construct() - { - $this->string = 'foo'; - $this->true = true; - } - - public function reset(): void - { - unset($this->string, $this->true); - } - - public function doFoo(): void - { - echo $this->string ?? 'default'; - if (isset($this->string)) { - } - if (empty($this->true)) { - } - } -}