From 9ec0cbdb429a761c2673134cf7c8fddcab7cfca2 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 1/3] 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 5f47c928b57..59f5e82c816 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 76416d567d7..89de47d38eb 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 00000000000..766a083cf97 --- /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; + } +} From 7f6add5733d4825032cf2ae27060e0eadfd40732 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Thu, 26 Mar 2026 14:28:03 +0000 Subject: [PATCH 2/3] Also check PropertyInitializationExpr scope for additional constructors The scope check for PropertyInitializationExpr was restricted to only __construct, but additional constructors (configured via additionalConstructors or AdditionalConstructorsExtension) should also consult the scope since PropertyInitializationExpr from __construct correctly reflects promoted property initialization state for them. Co-Authored-By: Claude Opus 4.6 --- src/Node/ClassPropertiesNode.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php index 59f5e82c816..f29a7cd9620 100644 --- a/src/Node/ClassPropertiesNode.php +++ b/src/Node/ClassPropertiesNode.php @@ -211,7 +211,7 @@ public function getUninitializedProperties( if ($usage instanceof PropertyWrite) { if (array_key_exists($propertyName, $initializedPropertiesMap)) { $hasInitialization = $initializedPropertiesMap[$propertyName]; - if (strtolower($function->getName()) === '__construct') { + if (in_array($function->getName(), $constructors, true)) { $hasInitialization = $hasInitialization->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); } if ( From 6d4a2d099a7f872a92dd0ca280bbbd98d6e6e895 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 28 Mar 2026 17:23:02 +0000 Subject: [PATCH 3/3] Add test for bug-12253 with additionalConstructors configured Adds a PayloadWithAdditionalConstructor test class that reproduces the same readonly property false positive scenario but with setUp() configured as an additional constructor via ConstructorsHelper. Co-Authored-By: Claude Opus 4.6 --- .../MissingReadOnlyPropertyAssignRuleTest.php | 1 + .../Rules/Properties/data/bug-12253.php | 44 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index 89de47d38eb..2fc14f808da 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -26,6 +26,7 @@ protected function getRule(): Rule 'Bug10523\\Controller::init', 'Bug10523\\MultipleWrites::init', 'Bug10523\\SingleWriteInConstructorCalledMethod::init', + 'Bug12253\\PayloadWithAdditionalConstructor::setUp', ], ), ); diff --git a/tests/PHPStan/Rules/Properties/data/bug-12253.php b/tests/PHPStan/Rules/Properties/data/bug-12253.php index 766a083cf97..54ad0fb9584 100644 --- a/tests/PHPStan/Rules/Properties/data/bug-12253.php +++ b/tests/PHPStan/Rules/Properties/data/bug-12253.php @@ -46,6 +46,50 @@ private function validationValue(mixed $value): mixed } } +class PayloadWithAdditionalConstructor +{ + /** @var array> */ + private(set) readonly array $validation; + + /** @var array */ + private array $ids = []; + + public function __construct(private readonly stdClass $payload) + { + } + + public function setUp(): void + { + $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> */