Skip to content

Commit eea4e54

Browse files
grizzm0grizzm0
committed
Allow reinitialization of a readonly property in __clone since PHP8.3
Signed-off-by: Kristofer Karlsson <karlsson.kristofer@gmail.com>
1 parent c853aa2 commit eea4e54

7 files changed

Lines changed: 224 additions & 3 deletions

File tree

‎src/Analyser/MutatingScope.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use PHPStan\Node\Expr\OriginalForeachKeyExpr;
3333
use PHPStan\Node\Expr\OriginalForeachValueExpr;
3434
use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
35+
use PHPStan\Node\Expr\CloneReinitializationExpr;
3536
use PHPStan\Node\Expr\PossiblyImpureCallExpr;
3637
use PHPStan\Node\Expr\PropertyInitializationExpr;
3738
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
@@ -2928,7 +2929,18 @@ public function assignInitializedProperty(Type $fetchedOnType, string $propertyN
29282929
return $this;
29292930
}
29302931

2931-
return $this->assignExpression(new PropertyInitializationExpr($propertyName), new MixedType(), new MixedType());
2932+
$scope = $this->assignExpression(new PropertyInitializationExpr($propertyName), new MixedType(), new MixedType());
2933+
2934+
$function = $scope->getFunction();
2935+
if (
2936+
$function instanceof MethodReflection
2937+
&& strtolower($function->getName()) === '__clone'
2938+
&& $scope->phpVersion->supportsReadonlyPropertyReinitializationOnClone()
2939+
) {
2940+
$scope = $scope->assignExpression(new CloneReinitializationExpr($propertyName), new MixedType(), new MixedType());
2941+
}
2942+
2943+
return $scope;
29322944
}
29332945

29342946
public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self

‎src/Node/Expr/CloneReinitializationExpr.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node\Expr;
4+
5+
use Override;
6+
use PhpParser\Node\Expr;
7+
use PHPStan\Node\VirtualNode;
8+
9+
/**
10+
* Tracks that a readonly property has been re-assigned within the current __clone() body.
11+
*
12+
* Distinct from PropertyInitializationExpr because PHP 8.3+ allows readonly properties
13+
* to be re-initialized once inside __clone — but the property is already initialized at
14+
* __clone's entry (carried over from the post-construction class scope), so the standard
15+
* initialization tracker can't distinguish "first write inside __clone" from "no write yet
16+
* inside __clone". This expression is only set when an assignment actually happens inside
17+
* __clone, and is excluded from rememberConstructorExpressions() so it never leaks into
18+
* __clone's entry scope.
19+
*/
20+
final class CloneReinitializationExpr extends Expr implements VirtualNode
21+
{
22+
23+
public function __construct(private string $propertyName)
24+
{
25+
parent::__construct([]);
26+
}
27+
28+
public function getPropertyName(): string
29+
{
30+
return $this->propertyName;
31+
}
32+
33+
#[Override]
34+
public function getType(): string
35+
{
36+
return 'PHPStan_Node_CloneReinitializationExpr';
37+
}
38+
39+
/**
40+
* @return string[]
41+
*/
42+
#[Override]
43+
public function getSubNodeNames(): array
44+
{
45+
return [];
46+
}
47+
48+
}

‎src/Node/Printer/Printer.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PHPStan\Node\BooleanAndNode;
88
use PHPStan\Node\BooleanOrNode;
99
use PHPStan\Node\Expr\AlwaysRememberedExpr;
10+
use PHPStan\Node\Expr\CloneReinitializationExpr;
1011
use PHPStan\Node\Expr\ExistingArrayDimFetch;
1112
use PHPStan\Node\Expr\ForeachValueByRefExpr;
1213
use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
@@ -104,6 +105,11 @@ protected function pPHPStan_Node_PropertyInitializationExpr(PropertyInitializati
104105
return sprintf('__phpstanPropertyInitialization(%s)', $expr->getPropertyName());
105106
}
106107

108+
protected function pPHPStan_Node_CloneReinitializationExpr(CloneReinitializationExpr $expr): string // phpcs:ignore
109+
{
110+
return sprintf('__phpstanCloneReinitialization(%s)', $expr->getPropertyName());
111+
}
112+
107113
protected function pPHPStan_Node_ForeachValueByRefExpr(ForeachValueByRefExpr $expr): string // phpcs:ignore
108114
{
109115
return sprintf('__phpstanForeachValueByRef(%s)', $this->p($expr->getExpr()));

‎src/Php/PhpVersion.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,11 @@ public function supportsReadOnlyAnonymousClasses(): bool
342342
return $this->versionId >= 80300;
343343
}
344344

345+
public function supportsReadonlyPropertyReinitializationOnClone(): bool
346+
{
347+
return $this->versionId >= 80300;
348+
}
349+
345350
public function supportsNeverReturnTypeInArrowFunction(): bool
346351
{
347352
return $this->versionId >= 80200;

‎src/Rules/Properties/ReadOnlyPropertyAssignRule.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PhpParser\Node;
77
use PHPStan\Analyser\Scope;
88
use PHPStan\DependencyInjection\RegisteredRule;
9+
use PHPStan\Node\Expr\CloneReinitializationExpr;
910
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
1011
use PHPStan\Node\Expr\UnsetOffsetExpr;
1112
use PHPStan\Node\PropertyAssignNode;
@@ -96,15 +97,26 @@ public function processNode(Node $node, Scope $scope): array
9697
throw new ShouldNotHappenException();
9798
}
9899

100+
$methodName = $scopeMethod->getName();
101+
$inClone = $this->phpVersion->supportsReadonlyPropertyReinitializationOnClone() && strtolower($methodName) === '__clone';
99102
if (
100-
in_array($scopeMethod->getName(), $this->constructorsHelper->getConstructors($scopeClassReflection), true)
101-
|| strtolower($scopeMethod->getName()) === '__unserialize'
103+
in_array($methodName, $this->constructorsHelper->getConstructors($scopeClassReflection), true)
104+
|| strtolower($methodName) === '__unserialize'
105+
|| $inClone
102106
) {
103107
if (TypeUtils::findThisType($scope->getType($propertyFetch->var)) === null) {
104108
$errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is not assigned on $this.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
105109
->line($propertyFetch->name->getStartLine())
106110
->identifier('property.readOnlyAssignNotOnThis')
107111
->build();
112+
} elseif (
113+
$inClone
114+
&& !$scope->hasExpressionType(new CloneReinitializationExpr($propertyReflection->getName()))->no()
115+
) {
116+
$errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is already assigned.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
117+
->line($propertyFetch->name->getStartLine())
118+
->identifier('assign.readOnlyProperty')
119+
->build();
108120
}
109121

110122
continue;

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,64 @@ public function testCloneWith(): void
191191
$this->analyse([__DIR__ . '/data/readonly-property-assign-clone-with.php'], []);
192192
}
193193

194+
#[RequiresPhp('>= 8.1.0')]
195+
public function testBug11495(): void
196+
{
197+
if (PHP_VERSION_ID < 80300) {
198+
$errors = [
199+
[
200+
'Readonly property Bug11495\HelloWorld::$foo is assigned outside of the constructor.',
201+
17,
202+
],
203+
[
204+
'Readonly property Bug11495\HelloWorld::$foo is assigned outside of the constructor.',
205+
20,
206+
],
207+
[
208+
'Readonly property Bug11495\DoubleAssign::$foo is assigned outside of the constructor.',
209+
40,
210+
],
211+
[
212+
'Readonly property Bug11495\DoubleAssign::$foo is assigned outside of the constructor.',
213+
41,
214+
],
215+
[
216+
'Readonly property Bug11495\BranchedAssign::$foo is assigned outside of the constructor.',
217+
57,
218+
],
219+
[
220+
'Readonly property Bug11495\BranchedAssign::$foo is assigned outside of the constructor.',
221+
59,
222+
],
223+
[
224+
'Readonly property Bug11495\ConditionalThenAssign::$foo is assigned outside of the constructor.',
225+
76,
226+
],
227+
[
228+
'Readonly property Bug11495\ConditionalThenAssign::$foo is assigned outside of the constructor.',
229+
78,
230+
],
231+
];
232+
} else {
233+
$errors = [
234+
[
235+
'Readonly property Bug11495\HelloWorld::$foo is not assigned on $this.',
236+
20,
237+
],
238+
[
239+
'Readonly property Bug11495\DoubleAssign::$foo is already assigned.',
240+
41,
241+
],
242+
[
243+
'Readonly property Bug11495\ConditionalThenAssign::$foo is already assigned.',
244+
78,
245+
],
246+
];
247+
}
248+
249+
$this->analyse([__DIR__ . '/data/bug-11495.php'], $errors);
250+
}
251+
194252
#[RequiresPhp('>= 8.4.0')]
195253
public function testBug12871(): void
196254
{

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
<?php // lint >= 8.1
2+
declare(strict_types = 1);
3+
4+
namespace Bug11495;
5+
6+
class HelloWorld
7+
{
8+
private readonly string $foo;
9+
10+
public function __construct()
11+
{
12+
$this->foo = 'bar';
13+
}
14+
15+
public function __clone()
16+
{
17+
$this->foo = 'baz';
18+
19+
$s = new self();
20+
$s->foo = 'baz';
21+
}
22+
23+
public function getFoo(): string
24+
{
25+
return $this->foo;
26+
}
27+
}
28+
29+
class DoubleAssign
30+
{
31+
private readonly string $foo;
32+
33+
public function __construct()
34+
{
35+
$this->foo = 'bar';
36+
}
37+
38+
public function __clone()
39+
{
40+
$this->foo = 'baz';
41+
$this->foo = 'qux';
42+
}
43+
}
44+
45+
class BranchedAssign
46+
{
47+
private readonly string $foo;
48+
49+
public function __construct()
50+
{
51+
$this->foo = 'bar';
52+
}
53+
54+
public function __clone()
55+
{
56+
if (rand(0, 1)) {
57+
$this->foo = 'a';
58+
} else {
59+
$this->foo = 'b';
60+
}
61+
}
62+
}
63+
64+
class ConditionalThenAssign
65+
{
66+
private readonly string $foo;
67+
68+
public function __construct()
69+
{
70+
$this->foo = 'bar';
71+
}
72+
73+
public function __clone()
74+
{
75+
if (rand(0, 1)) {
76+
$this->foo = 'a';
77+
}
78+
$this->foo = 'b';
79+
}
80+
}

0 commit comments

Comments
 (0)