Skip to content

Commit fe3288c

Browse files
VincentLangletVincentLanglet
committed
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
1 parent 06ea1e1 commit fe3288c

File tree

5 files changed

+130
-20
lines changed

5 files changed

+130
-20
lines changed

‎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: 17 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,26 @@ 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+
if (
73+
$isOutsideDeclaringClass
74+
&& $declaringClass->isReadOnly()
75+
&& $nativeReflection->isPublic()
76+
&& !$nativeReflection->isPrivateSet()
77+
) {
78+
$errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of its declaring class.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
79+
->line($propertyFetch->name->getStartLine())
80+
->identifier('property.readOnlyAssignOutOfClass')
81+
->build();
82+
}
7583
continue;
7684
}
7785

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

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,4 +234,23 @@ public function testCloneWith(): void
234234
]);
235235
}
236236

237+
#[RequiresPhp('>= 8.5')]
238+
public function testBug14063(): void
239+
{
240+
$this->analyse([__DIR__ . '/data/bug-14063.php'], [
241+
[
242+
'Assign to protected(set) property Bug14063\Bar::$value.',
243+
54,
244+
],
245+
[
246+
'Access to protected property Bug14063\Baz::$prot.',
247+
57,
248+
],
249+
[
250+
'Access to private property Bug14063\Baz::$priv.',
251+
57,
252+
],
253+
]);
254+
}
255+
237256
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,19 @@ public function testCloneWith(): void
180180
$this->analyse([__DIR__ . '/data/readonly-property-assign-clone-with.php'], []);
181181
}
182182

183+
#[RequiresPhp('>= 8.5')]
184+
public function testBug14063(): void
185+
{
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+
]);
196+
}
197+
183198
}

‎tests/PHPStan/Rules/Properties/data/bug-14063.php‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php // lint >= 8.5
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug14063;
6+
7+
final readonly class Obj
8+
{
9+
public function __construct(public string $value) {}
10+
11+
public function doFoo(): void
12+
{
13+
clone($this, ['value' => 'newVal']);
14+
}
15+
}
16+
17+
class Bar
18+
{
19+
public readonly string $value;
20+
21+
public function __construct(string $value)
22+
{
23+
$this->value = $value;
24+
}
25+
26+
public function doFoo(): void
27+
{
28+
clone($this, ['value' => 'newVal']);
29+
}
30+
}
31+
32+
readonly class Baz
33+
{
34+
public function __construct(
35+
public string $pub,
36+
protected string $prot,
37+
private string $priv,
38+
) {}
39+
40+
public function doFoo(): void
41+
{
42+
clone($this, [
43+
'pub' => 'newVal',
44+
'prot' => 'newVal',
45+
'priv' => 'newVal',
46+
]);
47+
}
48+
}
49+
50+
$obj = new Obj('val');
51+
$newObj = clone($obj, ['value' => 'newVal']);
52+
53+
$bar = new Bar('val');
54+
$newBar = clone($bar, ['value' => 'newVal']);
55+
56+
function (Baz $baz): void {
57+
clone($baz, [
58+
'pub' => 'newVal',
59+
'prot' => 'newVal',
60+
'priv' => 'newVal',
61+
]);
62+
};

0 commit comments

Comments
 (0)