From 1198766492fbf9e5ffe78fc564bd916ca2cde90a Mon Sep 17 00:00:00 2001 From: Brad Miller <28307684+mad-briller@users.noreply.github.com> Date: Wed, 3 Dec 2025 14:52:46 +0000 Subject: [PATCH 1/2] Don't report already assigned errors when setting or unsetting an offset on array access objects set as class properties. --- src/Node/ClassPropertiesNode.php | 17 +++++++++++ src/Node/ClassStatementsGatherer.php | 5 ++-- src/Node/Property/PropertyWrite.php | 10 ++++++- .../MissingReadOnlyPropertyAssignRuleTest.php | 11 +++++++ .../Rules/Properties/data/bug-13856.php | 29 +++++++++++++++++++ 5 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 tests/PHPStan/Rules/Properties/data/bug-13856.php diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php index 5f47c928b5..ce08d0d6b6 100644 --- a/src/Node/ClassPropertiesNode.php +++ b/src/Node/ClassPropertiesNode.php @@ -2,6 +2,7 @@ namespace PHPStan\Node; +use ArrayAccess; use Override; use PhpParser\Node; use PhpParser\Node\Expr\Array_; @@ -13,6 +14,8 @@ use PhpParser\NodeAbstract; use PHPStan\Analyser\Scope; use PHPStan\Node\Expr\PropertyInitializationExpr; +use PHPStan\Node\Expr\SetOffsetValueTypeExpr; +use PHPStan\Node\Expr\UnsetOffsetExpr; use PHPStan\Node\Method\MethodCall; use PHPStan\Node\Property\PropertyAssign; use PHPStan\Node\Property\PropertyRead; @@ -22,6 +25,7 @@ use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider; use PHPStan\TrinaryLogic; use PHPStan\Type\NeverType; +use PHPStan\Type\ObjectType; use PHPStan\Type\TypeUtils; use function array_diff_key; use function array_key_exists; @@ -211,6 +215,19 @@ public function getUninitializedProperties( if ($usage instanceof PropertyWrite) { if (array_key_exists($propertyName, $initializedPropertiesMap)) { + $originalNode = $usage->getOriginalNode(); + + if ($originalNode instanceof PropertyAssignNode) { + $assignedExpr = $originalNode->getAssignedExpr(); + + if ( + ($assignedExpr instanceof SetOffsetValueTypeExpr || $assignedExpr instanceof UnsetOffsetExpr) + && (new ObjectType(ArrayAccess::class))->isSuperTypeOf($scope->getType($assignedExpr->getVar()))->yes() + ) { + continue; + } + } + $hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); if ( !$hasInitialization->no() diff --git a/src/Node/ClassStatementsGatherer.php b/src/Node/ClassStatementsGatherer.php index b3cada2212..74b7ea2a56 100644 --- a/src/Node/ClassStatementsGatherer.php +++ b/src/Node/ClassStatementsGatherer.php @@ -150,6 +150,7 @@ private function gatherNodes(Node $node, Scope $scope): void new PropertyFetch(new Expr\Variable('this'), new Identifier($node->getName())), $scope, true, + $node, ); } return; @@ -194,7 +195,7 @@ private function gatherNodes(Node $node, Scope $scope): void return; } if ($node instanceof PropertyAssignNode) { - $this->propertyUsages[] = new PropertyWrite($node->getPropertyFetch(), $scope, false); + $this->propertyUsages[] = new PropertyWrite($node->getPropertyFetch(), $scope, false, $node); $this->propertyAssigns[] = new PropertyAssign($node, $scope); return; } @@ -212,7 +213,7 @@ private function gatherNodes(Node $node, Scope $scope): void } $this->propertyUsages[] = new PropertyRead($node->expr, $scope); - $this->propertyUsages[] = new PropertyWrite($node->expr, $scope, false); + $this->propertyUsages[] = new PropertyWrite($node->expr, $scope, false, $node); return; } if ($node instanceof FunctionCallableNode) { diff --git a/src/Node/Property/PropertyWrite.php b/src/Node/Property/PropertyWrite.php index df39b83d0b..dafd881663 100644 --- a/src/Node/Property/PropertyWrite.php +++ b/src/Node/Property/PropertyWrite.php @@ -2,9 +2,12 @@ namespace PHPStan\Node\Property; +use PhpParser\Node\Expr\AssignRef; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\StaticPropertyFetch; use PHPStan\Analyser\Scope; +use PHPStan\Node\ClassPropertyNode; +use PHPStan\Node\PropertyAssignNode; /** * @api @@ -12,7 +15,7 @@ final class PropertyWrite { - public function __construct(private PropertyFetch|StaticPropertyFetch $fetch, private Scope $scope, private bool $promotedPropertyWrite) + public function __construct(private PropertyFetch|StaticPropertyFetch $fetch, private Scope $scope, private bool $promotedPropertyWrite, private ClassPropertyNode|PropertyAssignNode|AssignRef|null $originalNode = null) { } @@ -34,4 +37,9 @@ public function isPromotedPropertyWrite(): bool return $this->promotedPropertyWrite; } + public function getOriginalNode(): ClassPropertyNode|PropertyAssignNode|AssignRef|null + { + return $this->originalNode; + } + } diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index 578f806989..e198047424 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -46,4 +46,15 @@ public static function getAdditionalConfigFiles(): array ); } + #[RequiresPhp('>= 8.1')] + public function testBug13856(): void + { + $this->analyse([__DIR__ . '/data/bug-13856.php'], [ + [ + 'Readonly property Bug13856\foo2::$store is already assigned.', + 27, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-13856.php b/tests/PHPStan/Rules/Properties/data/bug-13856.php new file mode 100644 index 0000000000..86ff7922c3 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-13856.php @@ -0,0 +1,29 @@ += 8.1 + +namespace Bug13856; + +use SplObjectStorage; + +class foo +{ + /** @var SplObjectStorage */ + private readonly SplObjectStorage $store; + + public function __construct() + { + $this->store = new SplObjectStorage(); + $this->store[(object) ['foo' => 'bar']] = true; + } +} + +class foo2 +{ + /** @var array */ + private readonly array $store; + + public function __construct() + { + $this->store[1] = true; + $this->store[2] = false; + } +} From ee9ba4154ce14cfe4ed038b4a82abc54e9949efd Mon Sep 17 00:00:00 2001 From: Brad Miller <28307684+mad-briller@users.noreply.github.com> Date: Thu, 4 Dec 2025 13:17:21 +0000 Subject: [PATCH 2/2] Add test for unset. --- .../Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php | 2 +- tests/PHPStan/Rules/Properties/data/bug-13856.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index e198047424..c41ec6c95d 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -52,7 +52,7 @@ public function testBug13856(): void $this->analyse([__DIR__ . '/data/bug-13856.php'], [ [ 'Readonly property Bug13856\foo2::$store is already assigned.', - 27, + 28, ], ]); } diff --git a/tests/PHPStan/Rules/Properties/data/bug-13856.php b/tests/PHPStan/Rules/Properties/data/bug-13856.php index 86ff7922c3..a0a0cd461d 100644 --- a/tests/PHPStan/Rules/Properties/data/bug-13856.php +++ b/tests/PHPStan/Rules/Properties/data/bug-13856.php @@ -13,6 +13,7 @@ public function __construct() { $this->store = new SplObjectStorage(); $this->store[(object) ['foo' => 'bar']] = true; + unset($this->store[(object) ['foo' => 'bar']]); } }