From 288ca283b2d850416001a7e45108afb7e2efd61e Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Fri, 27 Feb 2026 13:11:53 +0000 Subject: [PATCH] Fix false positive "readonly property already assigned" in method called from constructor When a constructor calls a helper method that assigns a readonly property and also calls other methods, the property was incorrectly reported as "already assigned". Two root causes: 1. In getMethodsCalledFromConstructor(), non-__construct methods that called other methods had all constructor-initialized properties marked as Yes. This was intended only for additional constructors (like setUp) but incorrectly applied to regular helper methods. Fixed by tracking which methods are original constructors vs discovered helper methods. 2. The additionalAssigns check used scope's hasExpressionType() for PropertyInitializationExpr, but this expression leaks from the constructor scope into all non-static method scopes via rememberConstructorScope(). For non-constructor methods, the scope check always returns Yes even for the first assignment. Fixed by only consulting the scope for __construct itself, and relying on the initializedPropertiesMap for other methods. Fixes https://github.com/phpstan/phpstan/issues/12253 --- src/Node/ClassPropertiesNode.php | 14 +-- .../MissingReadOnlyPropertyAssignRuleTest.php | 6 ++ .../Rules/Properties/data/bug-12253.php | 87 +++++++++++++++++++ 3 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 tests/PHPStan/Rules/Properties/data/bug-12253.php diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php index 5f47c928b5..59f5e82c81 100644 --- a/src/Node/ClassPropertiesNode.php +++ b/src/Node/ClassPropertiesNode.php @@ -166,10 +166,9 @@ public function getUninitializedProperties( $initializedInConstructor = array_diff_key($uninitializedProperties, $this->collectUninitializedProperties([$classReflection->getConstructor()->getName()], $uninitializedProperties)); } - $methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $constructors, $initializedInConstructor); + $methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $constructors, $initializedInConstructor, $constructors); $prematureAccess = []; $additionalAssigns = []; - foreach ($this->getPropertyUsages() as $usage) { $fetch = $usage->getFetch(); if (!$fetch instanceof PropertyFetch) { @@ -211,7 +210,10 @@ public function getUninitializedProperties( if ($usage instanceof PropertyWrite) { if (array_key_exists($propertyName, $initializedPropertiesMap)) { - $hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); + $hasInitialization = $initializedPropertiesMap[$propertyName]; + if (strtolower($function->getName()) === '__construct') { + $hasInitialization = $hasInitialization->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); + } if ( !$hasInitialization->no() && !$usage->isPromotedPropertyWrite() @@ -318,6 +320,7 @@ private function collectUninitializedProperties(array $constructors, array $unin * @param array $initialInitializedProperties * @param array> $initializedProperties * @param array $initializedInConstructorProperties + * @param string[] $originalConstructors * * @return array> */ @@ -327,6 +330,7 @@ private function getMethodsCalledFromConstructor( array $initializedProperties, array $methods, array $initializedInConstructorProperties, + array $originalConstructors, ): array { $originalMap = $initializedProperties; @@ -363,7 +367,7 @@ private function getMethodsCalledFromConstructor( continue; } - if ($inMethod->getName() !== '__construct') { + if ($inMethod->getName() !== '__construct' && in_array($inMethod->getName(), $originalConstructors, true)) { foreach (array_keys($initializedInConstructorProperties) as $propertyName) { $initializedProperties[$inMethod->getName()][$propertyName] = TrinaryLogic::createYes(); } @@ -391,7 +395,7 @@ private function getMethodsCalledFromConstructor( return $initializedProperties; } - return $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $methods, $initializedInConstructorProperties); + return $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $methods, $initializedInConstructorProperties, $originalConstructors); } /** diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index 76416d567d..89de47d38e 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -342,4 +342,10 @@ public function testBug11828(): void $this->analyse([__DIR__ . '/data/bug-11828.php'], []); } + #[RequiresPhp('>= 8.4')] + public function testBug12253(): void + { + $this->analyse([__DIR__ . '/data/bug-12253.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-12253.php b/tests/PHPStan/Rules/Properties/data/bug-12253.php new file mode 100644 index 0000000000..766a083cf9 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-12253.php @@ -0,0 +1,87 @@ += 8.4 + +declare(strict_types = 1); + +namespace Bug12253; + +use stdClass; + +class Payload +{ + /** @var array> */ + private(set) readonly array $validation; + + /** @var array */ + private array $ids = []; + + public function __construct(private readonly stdClass $payload) + { + $this->parseValidation(); + } + + private function parseValidation(): void + { + $validations = []; + + foreach ($this->payload->validation as $key => $validation) { + $validations[] = [ + 'id' => $key, + 'field_id' => $this->ids[$validation->field_id], + 'rule' => $validation->rule, + 'value' => $this->validationValue($validation->value), + 'message' => $validation->message, + ]; + } + + $this->validation = $validations; + } + + private function validationValue(mixed $value): mixed + { + if (is_null($value)) { + return null; + } + + return $this->ids[$value] ?? $value; + } +} + +class PayloadWithoutAsymmetricVisibility +{ + /** @var array> */ + private readonly array $validation; + + /** @var array */ + private array $ids = []; + + public function __construct(private readonly stdClass $payload) + { + $this->parseValidation(); + } + + private function parseValidation(): void + { + $validations = []; + + foreach ($this->payload->validation as $key => $validation) { + $validations[] = [ + 'id' => $key, + 'field_id' => $this->ids[$validation->field_id], + 'rule' => $validation->rule, + 'value' => $this->validationValue($validation->value), + 'message' => $validation->message, + ]; + } + + $this->validation = $validations; + } + + private function validationValue(mixed $value): mixed + { + if (is_null($value)) { + return null; + } + + return $this->ids[$value] ?? $value; + } +}