Skip to content

Commit 1004481

Browse files
schlndhschlndh
authored
Check redeclared promoted readonly properties
1 parent 1f95482 commit 1004481

9 files changed

+568
-0
lines changed

‎src/Analyser/NodeScopeResolver.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@
177177
use PHPStan\Type\TypeTraverser;
178178
use PHPStan\Type\TypeUtils;
179179
use PHPStan\Type\UnionType;
180+
use ReflectionProperty;
180181
use Throwable;
181182
use Traversable;
182183
use TypeError;
@@ -2722,6 +2723,26 @@ static function (): void {
27222723
$scope = $scope->invalidateExpression(new Variable('this'), true);
27232724
}
27242725

2726+
if (
2727+
$methodReflection !== null
2728+
&& !$methodReflection->isStatic()
2729+
&& $methodReflection->getName() === '__construct'
2730+
&& $scopeFunction instanceof MethodReflection
2731+
&& !$scopeFunction->isStatic()
2732+
&& $scope->isInClass()
2733+
&& $scope->getClassReflection()->isSubclassOf($methodReflection->getDeclaringClass()->getName())
2734+
) {
2735+
$thisType = $scope->getType(new Variable('this'));
2736+
$methodClassReflection = $methodReflection->getDeclaringClass();
2737+
foreach ($methodClassReflection->getNativeReflection()->getProperties(ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED) as $property) {
2738+
if (!$property->isPromoted() || $property->getDeclaringClass()->getName() !== $methodClassReflection->getName()) {
2739+
continue;
2740+
}
2741+
2742+
$scope = $scope->assignInitializedProperty($thisType, $property->getName());
2743+
}
2744+
}
2745+
27252746
$hasYield = $hasYield || $result->hasYield();
27262747
$throwPoints = array_merge($throwPoints, $result->getThrowPoints());
27272748
$impurePoints = array_merge($impurePoints, $result->getImpurePoints());

‎src/Node/ClassStatementsGatherer.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use PHPStan\Reflection\ClassReflection;
1919
use PHPStan\ShouldNotHappenException;
2020
use PHPStan\Type\TypeUtils;
21+
use ReflectionProperty;
2122
use function count;
2223
use function in_array;
2324
use function strtolower;
@@ -157,6 +158,9 @@ private function gatherNodes(Node $node, Scope $scope): void
157158
}
158159
if ($node instanceof MethodCall || $node instanceof StaticCall) {
159160
$this->methodCalls[] = new \PHPStan\Node\Method\MethodCall($node, $scope);
161+
if ($node instanceof StaticCall && $node->name instanceof Identifier && $node->name->toLowerString() === '__construct') {
162+
$this->tryToApplyPropertyWritesFromAncestorConstructor($node, $scope);
163+
}
160164
return;
161165
}
162166
if ($node instanceof MethodCallableNode || $node instanceof StaticMethodCallableNode) {
@@ -252,4 +256,28 @@ private function tryToApplyPropertyReads(Expr\FuncCall $node, Scope $scope): voi
252256
}
253257
}
254258

259+
private function tryToApplyPropertyWritesFromAncestorConstructor(StaticCall $ancestorConstructorCall, Scope $scope): void
260+
{
261+
if (!$ancestorConstructorCall->class instanceof Node\Name) {
262+
return;
263+
}
264+
265+
$calledOnType = $scope->resolveTypeByName($ancestorConstructorCall->class);
266+
if ($calledOnType->getClassReflection() === null || TypeUtils::findThisType($calledOnType) === null) {
267+
return;
268+
}
269+
270+
$classReflection = $calledOnType->getClassReflection()->getNativeReflection();
271+
foreach ($classReflection->getProperties(ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED) as $property) {
272+
if (!$property->isPromoted() || $property->getDeclaringClass()->getName() !== $classReflection->getName()) {
273+
continue;
274+
}
275+
$this->propertyUsages[] = new PropertyWrite(
276+
new PropertyFetch(new Expr\Variable('this'), new Identifier($property->getName()), $ancestorConstructorCall->getAttributes()),
277+
$scope,
278+
false,
279+
);
280+
}
281+
}
282+
255283
}

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,4 +287,101 @@ public function testBug10822(): void
287287
$this->analyse([__DIR__ . '/data/bug-10822.php'], []);
288288
}
289289

290+
public function testRedeclaredReadonlyProperties(): void
291+
{
292+
if (PHP_VERSION_ID < 80100) {
293+
$this->markTestSkipped('Test requires PHP 8.1.');
294+
}
295+
296+
$this->analyse([__DIR__ . '/data/redeclare-readonly-property.php'], [
297+
[
298+
'Readonly property RedeclareReadonlyProperty\B1::$myProp is already assigned.',
299+
16,
300+
],
301+
[
302+
'Readonly property RedeclareReadonlyProperty\B5::$myProp is already assigned.',
303+
50,
304+
],
305+
[
306+
'Readonly property RedeclareReadonlyProperty\B7::$myProp is already assigned.',
307+
70,
308+
],
309+
[
310+
'Readonly property class@anonymous/tests/PHPStan/Rules/Properties/data/redeclare-readonly-property.php:117::$myProp is already assigned.',
311+
121,
312+
],
313+
[
314+
'Class RedeclareReadonlyProperty\B16 has an uninitialized readonly property $myProp. Assign it in the constructor.',
315+
195,
316+
],
317+
[
318+
'Class RedeclareReadonlyProperty\C17 has an uninitialized readonly property $aProp. Assign it in the constructor.',
319+
218,
320+
],
321+
[
322+
'Class RedeclareReadonlyProperty\B18 has an uninitialized readonly property $aProp. Assign it in the constructor.',
323+
233,
324+
],
325+
]);
326+
}
327+
328+
public function testRedeclaredPropertiesOfReadonlyClass(): void
329+
{
330+
if (PHP_VERSION_ID < 80200) {
331+
$this->markTestSkipped('Test requires PHP 8.2.');
332+
}
333+
334+
$this->analyse([__DIR__ . '/data/redeclare-property-of-readonly-class.php'], [
335+
[
336+
'Readonly property RedeclarePropertyOfReadonlyClass\B1::$promotedProp is already assigned.',
337+
15,
338+
],
339+
]);
340+
}
341+
342+
public function testBug8101(): void
343+
{
344+
if (PHP_VERSION_ID < 80100) {
345+
$this->markTestSkipped('Test requires PHP 8.1.');
346+
}
347+
348+
$this->analyse([__DIR__ . '/data/bug-8101.php'], [
349+
[
350+
'Readonly property Bug8101\B::$myProp is already assigned.',
351+
12,
352+
],
353+
]);
354+
}
355+
356+
public function testBug9863(): void
357+
{
358+
if (PHP_VERSION_ID < 80100) {
359+
$this->markTestSkipped('Test requires PHP 8.1.');
360+
}
361+
362+
$this->analyse([__DIR__ . '/data/bug-9863.php'], [
363+
[
364+
'Readonly property Bug9863\ReadonlyChildWithoutIsset::$foo is already assigned.',
365+
17,
366+
],
367+
[
368+
'Class Bug9863\ReadonlyParentWithIsset has an uninitialized readonly property $foo. Assign it in the constructor.',
369+
23,
370+
],
371+
[
372+
'Access to an uninitialized readonly property Bug9863\ReadonlyParentWithIsset::$foo.',
373+
28,
374+
],
375+
]);
376+
}
377+
378+
public function testBug9864(): void
379+
{
380+
if (PHP_VERSION_ID < 80100) {
381+
$this->markTestSkipped('Test requires PHP 8.1.');
382+
}
383+
384+
$this->analyse([__DIR__ . '/data/bug-9864.php'], []);
385+
}
386+
290387
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,18 @@ public function testBug9831(): void
202202
]);
203203
}
204204

205+
public function testRedeclareReadonlyProperties(): void
206+
{
207+
$this->analyse([__DIR__ . '/data/redeclare-readonly-property.php'], [
208+
[
209+
'Class RedeclareReadonlyProperty\B19 has an uninitialized property $prop2. Give it default value or assign it in the constructor.',
210+
249,
211+
],
212+
[
213+
'Access to an uninitialized property RedeclareReadonlyProperty\B19::$prop2.',
214+
260,
215+
],
216+
]);
217+
}
218+
205219
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php declare(strict_types = 1); // lint >= 8.1
2+
3+
namespace Bug8101;
4+
5+
class A {
6+
public function __construct(public readonly int $myProp) {}
7+
}
8+
9+
class B extends A {
10+
// This should be reported as an error, as a readonly prop cannot be redeclared.
11+
public function __construct(public readonly int $myProp) {
12+
parent::__construct($myProp);
13+
}
14+
}
15+
16+
$foo = new B(7);

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php declare(strict_types = 1); // lint >= 8.1
2+
3+
namespace Bug9863;
4+
5+
class ReadonlyParentWithoutIsset
6+
{
7+
public function __construct(
8+
public readonly int $foo
9+
) {}
10+
}
11+
12+
class ReadonlyChildWithoutIsset extends ReadonlyParentWithoutIsset
13+
{
14+
public function __construct(
15+
public readonly int $foo = 42
16+
) {
17+
parent::__construct($foo);
18+
}
19+
}
20+
21+
class ReadonlyParentWithIsset
22+
{
23+
public readonly int $foo;
24+
25+
public function __construct(
26+
int $foo
27+
) {
28+
if (! isset($this->foo)) {
29+
$this->foo = $foo;
30+
}
31+
}
32+
}
33+
34+
class ReadonlyChildWithIsset extends ReadonlyParentWithIsset
35+
{
36+
public function __construct(
37+
public readonly int $foo = 42
38+
) {
39+
parent::__construct($foo);
40+
}
41+
}
42+
43+
$a = new ReadonlyParentWithoutIsset(0);
44+
$b = new ReadonlyChildWithoutIsset();
45+
$c = new ReadonlyChildWithoutIsset(1);
46+
47+
$x = new ReadonlyParentWithIsset(2);
48+
$y = new ReadonlyChildWithIsset();
49+
$z = new ReadonlyChildWithIsset(3);

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php declare(strict_types = 1); // lint >= 8.2
2+
3+
namespace Bug9864;
4+
5+
readonly abstract class UuidValueObject
6+
{
7+
public function __construct(public string $value)
8+
{
9+
$this->ensureIsValidUuid($value);
10+
}
11+
12+
private function ensureIsValidUuid(string $value): void
13+
{
14+
}
15+
}
16+
17+
18+
final readonly class ProductId extends UuidValueObject
19+
{
20+
public string $value;
21+
22+
public function __construct(
23+
string $value
24+
) {
25+
parent::__construct($value);
26+
}
27+
}
28+
29+
var_dump(new ProductId('test'));
30+
31+
// property is assigned on parent class, no need to reassing, specially for readonly properties

‎tests/PHPStan/Rules/Properties/data/redeclare-property-of-readonly-class.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php declare(strict_types = 1); // lint >= 8.2
2+
3+
namespace RedeclarePropertyOfReadonlyClass;
4+
5+
readonly class A {
6+
public function __construct(public int $promotedProp)
7+
{
8+
}
9+
}
10+
11+
readonly class B1 extends A {
12+
// $promotedProp is written twice
13+
public function __construct(public int $promotedProp)
14+
{
15+
parent::__construct(5);
16+
}
17+
}
18+
19+
readonly class B2 extends A {
20+
// Don't get confused by standard parameter with same name
21+
public function __construct(int $promotedProp)
22+
{
23+
parent::__construct($promotedProp);
24+
}
25+
}
26+
27+
readonly class B3 extends A {
28+
// This is allowed, because we don't write to the property.
29+
public int $promotedProp;
30+
31+
public function __construct()
32+
{
33+
parent::__construct(7);
34+
}
35+
}
36+
37+
readonly class B4 extends A {
38+
// The second write is not from the constructor. It is an error, but it is handled by different rule.
39+
public int $promotedProp;
40+
41+
public function __construct()
42+
{
43+
parent::__construct(7);
44+
}
45+
46+
public function set(): void
47+
{
48+
$this->promotedProp = 7;
49+
}
50+
}

0 commit comments

Comments
 (0)