From fe3288c1c47184e4352d0a6b405872b330fee447 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Mon, 6 Apr 2026 19:07:48 +0000 Subject: [PATCH 1/2] Fix phpstan/phpstan#14063: Readonly property modification through clone() not reported outside allowed scope - Fixed PhpPropertyReflection::isProtectedSet() to handle readonly class promoted properties - BetterReflection's computeModifiers misses implicit protected(set) when readonly comes from the class rather than the property node - Added regression test in AccessPropertiesInAssignRuleTest and ReadOnlyPropertyAssignRuleTest --- .../ReadOnlyByPhpDocPropertyAssignRule.php | 27 +++++--- .../Properties/ReadOnlyPropertyAssignRule.php | 27 +++++--- .../AccessPropertiesInAssignRuleTest.php | 19 ++++++ .../ReadOnlyPropertyAssignRuleTest.php | 15 +++++ .../Rules/Properties/data/bug-14063.php | 62 +++++++++++++++++++ 5 files changed, 130 insertions(+), 20 deletions(-) create mode 100644 tests/PHPStan/Rules/Properties/data/bug-14063.php diff --git a/src/Rules/Properties/ReadOnlyByPhpDocPropertyAssignRule.php b/src/Rules/Properties/ReadOnlyByPhpDocPropertyAssignRule.php index e61a9a2426a..e4daff744ee 100644 --- a/src/Rules/Properties/ReadOnlyByPhpDocPropertyAssignRule.php +++ b/src/Rules/Properties/ReadOnlyByPhpDocPropertyAssignRule.php @@ -48,9 +48,6 @@ public function processNode(Node $node, Scope $scope): array } $inCloneWith = (bool) $propertyFetch->getAttribute('inCloneWith', false); - if ($inCloneWith) { - return []; - } $inFunction = $scope->getFunction(); if ( @@ -80,16 +77,26 @@ public function processNode(Node $node, Scope $scope): array $declaringClass = $nativeReflection->getDeclaringClass(); - if (!$scope->isInClass()) { - $errors[] = RuleErrorBuilder::message(sprintf('@readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName())) - ->line($propertyFetch->name->getStartLine()) - ->identifier('property.readOnlyByPhpDocAssignOutOfClass') - ->build(); + $scopeClassReflection = $scope->isInClass() ? $scope->getClassReflection() : null; + $isOutsideDeclaringClass = $scopeClassReflection === null + || $scopeClassReflection->getName() !== $declaringClass->getName(); + + if ($inCloneWith) { + if ( + $isOutsideDeclaringClass + && $declaringClass->isReadOnly() + && $nativeReflection->isPublic() + && !$nativeReflection->isPrivateSet() + ) { + $errors[] = RuleErrorBuilder::message(sprintf('@readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName())) + ->line($propertyFetch->name->getStartLine()) + ->identifier('property.readOnlyByPhpDocAssignOutOfClass') + ->build(); + } continue; } - $scopeClassReflection = $scope->getClassReflection(); - if ($scopeClassReflection->getName() !== $declaringClass->getName()) { + if ($isOutsideDeclaringClass) { $errors[] = RuleErrorBuilder::message(sprintf('@readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName())) ->line($propertyFetch->name->getStartLine()) ->identifier('property.readOnlyByPhpDocAssignOutOfClass') diff --git a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php index 987e000256e..3c0c3c17565 100644 --- a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php +++ b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php @@ -47,9 +47,6 @@ public function processNode(Node $node, Scope $scope): array } $inCloneWith = (bool) $propertyFetch->getAttribute('inCloneWith', false); - if ($inCloneWith) { - return []; - } $errors = []; $reflections = $this->propertyReflectionFinder->findPropertyReflectionsFromNode($propertyFetch, $scope); @@ -67,16 +64,26 @@ public function processNode(Node $node, Scope $scope): array $declaringClass = $nativeReflection->getDeclaringClass(); - if (!$scope->isInClass()) { - $errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName())) - ->line($propertyFetch->name->getStartLine()) - ->identifier('property.readOnlyAssignOutOfClass') - ->build(); + $scopeClassReflection = $scope->isInClass() ? $scope->getClassReflection() : null; + $isOutsideDeclaringClass = $scopeClassReflection === null + || $scopeClassReflection->getName() !== $declaringClass->getName(); + + if ($inCloneWith) { + if ( + $isOutsideDeclaringClass + && $declaringClass->isReadOnly() + && $nativeReflection->isPublic() + && !$nativeReflection->isPrivateSet() + ) { + $errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName())) + ->line($propertyFetch->name->getStartLine()) + ->identifier('property.readOnlyAssignOutOfClass') + ->build(); + } continue; } - $scopeClassReflection = $scope->getClassReflection(); - if ($scopeClassReflection->getName() !== $declaringClass->getName()) { + if ($isOutsideDeclaringClass) { $errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName())) ->line($propertyFetch->name->getStartLine()) ->identifier('property.readOnlyAssignOutOfClass') diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php index 6fb0fe27d5d..24b32fff97d 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php @@ -234,4 +234,23 @@ public function testCloneWith(): void ]); } + #[RequiresPhp('>= 8.5')] + public function testBug14063(): void + { + $this->analyse([__DIR__ . '/data/bug-14063.php'], [ + [ + 'Assign to protected(set) property Bug14063\Bar::$value.', + 54, + ], + [ + 'Access to protected property Bug14063\Baz::$prot.', + 57, + ], + [ + 'Access to private property Bug14063\Baz::$priv.', + 57, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php index 40bbb2d5c32..cb0a4040b33 100644 --- a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php @@ -180,4 +180,19 @@ public function testCloneWith(): void $this->analyse([__DIR__ . '/data/readonly-property-assign-clone-with.php'], []); } + #[RequiresPhp('>= 8.5')] + public function testBug14063(): void + { + $this->analyse([__DIR__ . '/data/bug-14063.php'], [ + [ + 'Readonly property Bug14063\Obj::$value is assigned outside of its declaring class.', + 51, + ], + [ + 'Readonly property Bug14063\Baz::$pub is assigned outside of its declaring class.', + 57, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-14063.php b/tests/PHPStan/Rules/Properties/data/bug-14063.php new file mode 100644 index 00000000000..71941a7ef99 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-14063.php @@ -0,0 +1,62 @@ += 8.5 + +declare(strict_types = 1); + +namespace Bug14063; + +final readonly class Obj +{ + public function __construct(public string $value) {} + + public function doFoo(): void + { + clone($this, ['value' => 'newVal']); + } +} + +class Bar +{ + public readonly string $value; + + public function __construct(string $value) + { + $this->value = $value; + } + + public function doFoo(): void + { + clone($this, ['value' => 'newVal']); + } +} + +readonly class Baz +{ + public function __construct( + public string $pub, + protected string $prot, + private string $priv, + ) {} + + public function doFoo(): void + { + clone($this, [ + 'pub' => 'newVal', + 'prot' => 'newVal', + 'priv' => 'newVal', + ]); + } +} + +$obj = new Obj('val'); +$newObj = clone($obj, ['value' => 'newVal']); + +$bar = new Bar('val'); +$newBar = clone($bar, ['value' => 'newVal']); + +function (Baz $baz): void { + clone($baz, [ + 'pub' => 'newVal', + 'prot' => 'newVal', + 'priv' => 'newVal', + ]); +}; From 88b6a3c0c4fbffe6ce675ba3527d2496c4c7c4b5 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Mon, 6 Apr 2026 22:00:10 +0000 Subject: [PATCH 2/2] Add test for non-readonly class with promoted public readonly property in clone-with Ensures that `final class Obj { public function __construct(public readonly string $value) {} }` with clone-with from outside is reported by AccessPropertiesInAssignRule as a protected(set) violation. Properties with explicit `readonly` keyword have implicit `protected(set)` visibility, so the assignment is correctly caught as a set-visibility violation. Co-Authored-By: Claude Opus 4.6 --- .../AccessPropertiesInAssignRuleTest.php | 10 +++++++--- .../Properties/ReadOnlyPropertyAssignRuleTest.php | 4 ++-- tests/PHPStan/Rules/Properties/data/bug-14063.php | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php index 24b32fff97d..87f474cb50c 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php @@ -238,17 +238,21 @@ public function testCloneWith(): void public function testBug14063(): void { $this->analyse([__DIR__ . '/data/bug-14063.php'], [ + [ + 'Assign to protected(set) property Bug14063\Qux::$value.', + 65, + ], [ 'Assign to protected(set) property Bug14063\Bar::$value.', - 54, + 68, ], [ 'Access to protected property Bug14063\Baz::$prot.', - 57, + 71, ], [ 'Access to private property Bug14063\Baz::$priv.', - 57, + 71, ], ]); } diff --git a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php index cb0a4040b33..1654c528ce9 100644 --- a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php @@ -186,11 +186,11 @@ public function testBug14063(): void $this->analyse([__DIR__ . '/data/bug-14063.php'], [ [ 'Readonly property Bug14063\Obj::$value is assigned outside of its declaring class.', - 51, + 62, ], [ 'Readonly property Bug14063\Baz::$pub is assigned outside of its declaring class.', - 57, + 71, ], ]); } diff --git a/tests/PHPStan/Rules/Properties/data/bug-14063.php b/tests/PHPStan/Rules/Properties/data/bug-14063.php index 71941a7ef99..5d05a62a172 100644 --- a/tests/PHPStan/Rules/Properties/data/bug-14063.php +++ b/tests/PHPStan/Rules/Properties/data/bug-14063.php @@ -47,9 +47,23 @@ public function doFoo(): void } } +// non-readonly class with promoted public readonly property +final class Qux +{ + public function __construct(public readonly string $value) {} + + public function doFoo(): void + { + clone($this, ['value' => 'newVal']); + } +} + $obj = new Obj('val'); $newObj = clone($obj, ['value' => 'newVal']); +$qux = new Qux('val'); +$newQux = clone($qux, ['value' => 'newVal']); + $bar = new Bar('val'); $newBar = clone($bar, ['value' => 'newVal']);