Skip to content

Commit 8245617

Browse files
phpstan-botclaude
phpstan-bot
and
claude
committed
Move clone-with readonly check from isProtectedSet() hack to readonly rules
Instead of working around BetterReflection's missing implicit protected(set) detection in PhpPropertyReflection::isProtectedSet(), handle the clone-with scope restriction directly in ReadOnlyPropertyAssignRule and ReadOnlyByPhpDocPropertyAssignRule. The inCloneWith early return is moved inside the per-property loop so that clone-with from outside a readonly class correctly reports "Readonly property assigned outside of its declaring class" for public properties that have implicit protected(set). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 14044bc commit 8245617

File tree

5 files changed

+50
-47
lines changed

5 files changed

+50
-47
lines changed

‎src/Reflection/Php/PhpPropertyReflection.php‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -312,24 +312,7 @@ public function getHook(string $hookType): ExtendedMethodReflection
312312

313313
public function isProtectedSet(): bool
314314
{
315-
if ($this->reflection->isProtectedSet()) {
316-
return true;
317-
}
318-
319-
// Workaround: BetterReflection's computeModifiers only checks the property
320-
// node's readonly flag when adding implicit protected(set) for public readonly
321-
// properties. For promoted properties in readonly classes, the readonly flag
322-
// is on the class, not the property node, so the implicit protected(set) is missed.
323-
if (
324-
$this->declaringClass->isReadOnly()
325-
&& $this->reflection->isPublic()
326-
&& !$this->reflection->isPrivateSet()
327-
&& ($this->reflection->getModifiers() & ReflectionProperty::IS_READONLY_COMPATIBILITY) === 0
328-
) {
329-
return true;
330-
}
331-
332-
return false;
315+
return $this->reflection->isProtectedSet();
333316
}
334317

335318
public function isPrivateSet(): bool

‎src/Rules/Properties/ReadOnlyByPhpDocPropertyAssignRule.php‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ public function processNode(Node $node, Scope $scope): array
4848
}
4949

5050
$inCloneWith = (bool) $propertyFetch->getAttribute('inCloneWith', false);
51-
if ($inCloneWith) {
52-
return [];
53-
}
5451

5552
$inFunction = $scope->getFunction();
5653
if (
@@ -80,16 +77,26 @@ public function processNode(Node $node, Scope $scope): array
8077

8178
$declaringClass = $nativeReflection->getDeclaringClass();
8279

83-
if (!$scope->isInClass()) {
84-
$errors[] = RuleErrorBuilder::message(sprintf('@readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
85-
->line($propertyFetch->name->getStartLine())
86-
->identifier('property.readOnlyByPhpDocAssignOutOfClass')
87-
->build();
80+
$scopeClassReflection = $scope->isInClass() ? $scope->getClassReflection() : null;
81+
$isOutsideDeclaringClass = $scopeClassReflection === null
82+
|| $scopeClassReflection->getName() !== $declaringClass->getName();
83+
84+
if ($inCloneWith) {
85+
if (
86+
$isOutsideDeclaringClass
87+
&& $declaringClass->isReadOnly()
88+
&& $nativeReflection->isPublic()
89+
&& !$nativeReflection->isPrivateSet()
90+
) {
91+
$errors[] = RuleErrorBuilder::message(sprintf('@readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
92+
->line($propertyFetch->name->getStartLine())
93+
->identifier('property.readOnlyByPhpDocAssignOutOfClass')
94+
->build();
95+
}
8896
continue;
8997
}
9098

91-
$scopeClassReflection = $scope->getClassReflection();
92-
if ($scopeClassReflection->getName() !== $declaringClass->getName()) {
99+
if ($isOutsideDeclaringClass) {
93100
$errors[] = RuleErrorBuilder::message(sprintf('@readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
94101
->line($propertyFetch->name->getStartLine())
95102
->identifier('property.readOnlyByPhpDocAssignOutOfClass')

‎src/Rules/Properties/ReadOnlyPropertyAssignRule.php‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,6 @@ public function processNode(Node $node, Scope $scope): array
4747
}
4848

4949
$inCloneWith = (bool) $propertyFetch->getAttribute('inCloneWith', false);
50-
if ($inCloneWith) {
51-
return [];
52-
}
5350

5451
$errors = [];
5552
$reflections = $this->propertyReflectionFinder->findPropertyReflectionsFromNode($propertyFetch, $scope);
@@ -67,16 +64,31 @@ public function processNode(Node $node, Scope $scope): array
6764

6865
$declaringClass = $nativeReflection->getDeclaringClass();
6966

70-
if (!$scope->isInClass()) {
71-
$errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
72-
->line($propertyFetch->name->getStartLine())
73-
->identifier('property.readOnlyAssignOutOfClass')
74-
->build();
67+
$scopeClassReflection = $scope->isInClass() ? $scope->getClassReflection() : null;
68+
$isOutsideDeclaringClass = $scopeClassReflection === null
69+
|| $scopeClassReflection->getName() !== $declaringClass->getName();
70+
71+
if ($inCloneWith) {
72+
// Clone-with is allowed for readonly properties within the declaring class.
73+
// But public properties in readonly classes have implicit protected(set),
74+
// which restricts clone-with to the declaring class and subclasses.
75+
// BetterReflection misses this for promoted properties in readonly classes,
76+
// so we detect it here.
77+
if (
78+
$isOutsideDeclaringClass
79+
&& $declaringClass->isReadOnly()
80+
&& $nativeReflection->isPublic()
81+
&& !$nativeReflection->isPrivateSet()
82+
) {
83+
$errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
84+
->line($propertyFetch->name->getStartLine())
85+
->identifier('property.readOnlyAssignOutOfClass')
86+
->build();
87+
}
7588
continue;
7689
}
7790

78-
$scopeClassReflection = $scope->getClassReflection();
79-
if ($scopeClassReflection->getName() !== $declaringClass->getName()) {
91+
if ($isOutsideDeclaringClass) {
8092
$errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
8193
->line($propertyFetch->name->getStartLine())
8294
->identifier('property.readOnlyAssignOutOfClass')

‎tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -238,18 +238,10 @@ public function testCloneWith(): void
238238
public function testBug14063(): void
239239
{
240240
$this->analyse([__DIR__ . '/data/bug-14063.php'], [
241-
[
242-
'Assign to protected(set) property Bug14063\Obj::$value.',
243-
51,
244-
],
245241
[
246242
'Assign to protected(set) property Bug14063\Bar::$value.',
247243
54,
248244
],
249-
[
250-
'Assign to protected(set) property Bug14063\Baz::$pub.',
251-
57,
252-
],
253245
[
254246
'Access to protected property Bug14063\Baz::$prot.',
255247
57,

‎tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,16 @@ public function testCloneWith(): void
183183
#[RequiresPhp('>= 8.5')]
184184
public function testBug14063(): void
185185
{
186-
$this->analyse([__DIR__ . '/data/bug-14063.php'], []);
186+
$this->analyse([__DIR__ . '/data/bug-14063.php'], [
187+
[
188+
'Readonly property Bug14063\Obj::$value is assigned outside of its declaring class.',
189+
51,
190+
],
191+
[
192+
'Readonly property Bug14063\Baz::$pub is assigned outside of its declaring class.',
193+
57,
194+
],
195+
]);
187196
}
188197

189198
}

0 commit comments

Comments
 (0)