Skip to content

Commit 677afdd

Browse files
authored
Fix false positives about uninitialized properties
1 parent e606fbe commit 677afdd

File tree

4 files changed

+419
-5
lines changed

4 files changed

+419
-5
lines changed

src/Node/ClassPropertiesNode.php

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,15 +152,15 @@ public function getUninitializedProperties(
152152
return [$uninitializedProperties, [], []];
153153
}
154154

155-
$methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $constructors);
156-
$prematureAccess = [];
157-
$additionalAssigns = [];
158-
159155
$initializedInConstructor = [];
160156
if ($classReflection->hasConstructor()) {
161157
$initializedInConstructor = array_diff_key($uninitializedProperties, $this->collectUninitializedProperties([$classReflection->getConstructor()->getName()], $uninitializedProperties));
162158
}
163159

160+
$methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $constructors, $initializedInConstructor);
161+
$prematureAccess = [];
162+
$additionalAssigns = [];
163+
164164
foreach ($this->getPropertyUsages() as $usage) {
165165
$fetch = $usage->getFetch();
166166
if (!$fetch instanceof PropertyFetch) {
@@ -311,17 +311,21 @@ private function collectUninitializedProperties(array $constructors, array $unin
311311
* @param string[] $methods
312312
* @param array<string, TrinaryLogic> $initialInitializedProperties
313313
* @param array<string, array<string, TrinaryLogic>> $initializedProperties
314+
* @param array<string, ClassPropertyNode> $initializedInConstructorProperties
315+
*
314316
* @return array<string, array<string, TrinaryLogic>>
315317
*/
316318
private function getMethodsCalledFromConstructor(
317319
ClassReflection $classReflection,
318320
array $initialInitializedProperties,
319321
array $initializedProperties,
320322
array $methods,
323+
array $initializedInConstructorProperties,
321324
): array
322325
{
323326
$originalMap = $initializedProperties;
324327
$originalMethods = $methods;
328+
325329
foreach ($this->methodCalls as $methodCall) {
326330
$methodCallNode = $methodCall->getNode();
327331
if ($methodCallNode instanceof Array_) {
@@ -353,6 +357,12 @@ private function getMethodsCalledFromConstructor(
353357
continue;
354358
}
355359

360+
if ($inMethod->getName() !== '__construct') {
361+
foreach ($initializedInConstructorProperties as $propertyName => $propertyNode) {
362+
$initializedProperties[$inMethod->getName()][$propertyName] = TrinaryLogic::createYes();
363+
}
364+
}
365+
356366
$methodName = $methodCallNode->name->toString();
357367
if (array_key_exists($methodName, $initializedProperties)) {
358368
foreach ($this->getInitializedProperties($callScope, $initializedProperties[$inMethod->getName()] ?? $initialInitializedProperties) as $propertyName => $isInitialized) {
@@ -375,7 +385,7 @@ private function getMethodsCalledFromConstructor(
375385
return $initializedProperties;
376386
}
377387

378-
return $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $methods);
388+
return $this->getMethodsCalledFromConstructor($classReflection, $initialInitializedProperties, $initializedProperties, $methods, $initializedInConstructorProperties);
379389
}
380390

381391
/**

tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ protected function getRule(): Rule
2323
self::getContainer(),
2424
[
2525
'MissingReadOnlyPropertyAssign\\TestCase::setUp',
26+
'Bug10523\\Controller::init',
27+
'Bug10523\\MultipleWrites::init',
28+
'Bug10523\\SingleWriteInConstructorCalledMethod::init',
2629
],
2730
),
2831
);
@@ -261,4 +264,27 @@ public function testAnonymousReadonlyClass(): void
261264
]);
262265
}
263266

267+
public function testBug10523(): void
268+
{
269+
if (PHP_VERSION_ID < 80100) {
270+
$this->markTestSkipped('Test requires PHP 8.1.');
271+
}
272+
273+
$this->analyse([__DIR__ . '/data/bug-10523.php'], [
274+
[
275+
'Readonly property Bug10523\MultipleWrites::$userAccount is already assigned.',
276+
55,
277+
],
278+
]);
279+
}
280+
281+
public function testBug10822(): void
282+
{
283+
if (PHP_VERSION_ID < 80100) {
284+
$this->markTestSkipped('Test requires PHP 8.1.');
285+
}
286+
287+
$this->analyse([__DIR__ . '/data/bug-10822.php'], []);
288+
}
289+
264290
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug10523;
4+
5+
final class Controller
6+
{
7+
private readonly B $userAccount;
8+
9+
public function __construct()
10+
{
11+
$this->userAccount = new B();
12+
}
13+
14+
public function init(): void
15+
{
16+
$this->redirectIfNkdeCheckoutNotAllowed();
17+
$this->redirectIfNoShoppingBasketPresent();
18+
}
19+
20+
private function redirectIfNkdeCheckoutNotAllowed(): void
21+
{
22+
23+
}
24+
25+
private function redirectIfNoShoppingBasketPresent(): void
26+
{
27+
$x = $this->userAccount;
28+
}
29+
30+
}
31+
32+
class B {}
33+
34+
final class MultipleWrites
35+
{
36+
private readonly B $userAccount;
37+
38+
public function __construct()
39+
{
40+
$this->userAccount = new B();
41+
}
42+
43+
public function init(): void
44+
{
45+
$this->redirectIfNkdeCheckoutNotAllowed();
46+
$this->redirectIfNoShoppingBasketPresent();
47+
}
48+
49+
private function redirectIfNkdeCheckoutNotAllowed(): void
50+
{
51+
}
52+
53+
private function redirectIfNoShoppingBasketPresent(): void
54+
{
55+
$this->userAccount = new B();
56+
}
57+
58+
}
59+
60+
61+
final class SingleWriteInConstructorCalledMethod
62+
{
63+
private readonly B $userAccount;
64+
65+
public function __construct()
66+
{
67+
}
68+
69+
public function init(): void
70+
{
71+
$this->redirectIfNkdeCheckoutNotAllowed();
72+
$this->redirectIfNoShoppingBasketPresent();
73+
}
74+
75+
private function redirectIfNkdeCheckoutNotAllowed(): void
76+
{
77+
}
78+
79+
private function redirectIfNoShoppingBasketPresent(): void
80+
{
81+
$this->userAccount = new B();
82+
}
83+
84+
}

0 commit comments

Comments
 (0)