Skip to content

Commit b2d2a9d

Browse files
committed
Evaluate uninitialized properties based on each execution end in constructor
Closes phpstan/phpstan#7649
1 parent 2d2cae1 commit b2d2a9d

9 files changed

+203
-5
lines changed

src/Analyser/NodeScopeResolver.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ private function processStmtNode(
691691
$this->processAttributeGroups($stmt->attrGroups, $classScope, $classStatementsGatherer);
692692

693693
$this->processStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context);
694-
$nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls()), $classScope);
694+
$nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes()), $classScope);
695695
$nodeCallback(new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls()), $classScope);
696696
$nodeCallback(new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches()), $classScope);
697697
$classReflection->evictPrivateSymbols();
@@ -3814,6 +3814,9 @@ private function processAssignVar(
38143814
} else {
38153815
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
38163816
$nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
3817+
if ($var instanceof PropertyFetch && $var->name instanceof Node\Identifier && !$isAssignOp) {
3818+
$scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString());
3819+
}
38173820
}
38183821
$scope = $scope->assignExpression(
38193822
$var,
@@ -3835,6 +3838,9 @@ private function processAssignVar(
38353838
} else {
38363839
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
38373840
$nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
3841+
if ($var instanceof PropertyFetch && $var->name instanceof Node\Identifier && !$isAssignOp) {
3842+
$scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString());
3843+
}
38383844
}
38393845
}
38403846

src/Node/ClassPropertiesNode.php

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@
2121
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
2222
use PHPStan\ShouldNotHappenException;
2323
use PHPStan\TrinaryLogic;
24+
use PHPStan\Type\NeverType;
2425
use PHPStan\Type\TypeUtils;
2526
use function array_key_exists;
2627
use function array_keys;
2728
use function in_array;
29+
use function strtolower;
2830

2931
/** @api */
3032
class ClassPropertiesNode extends NodeAbstract implements VirtualNode
@@ -34,13 +36,15 @@ class ClassPropertiesNode extends NodeAbstract implements VirtualNode
3436
* @param ClassPropertyNode[] $properties
3537
* @param array<int, PropertyRead|PropertyWrite> $propertyUsages
3638
* @param array<int, MethodCall> $methodCalls
39+
* @param array<string, MethodReturnStatementsNode> $returnStatementNodes
3740
*/
3841
public function __construct(
3942
private ClassLike $class,
4043
private ReadWritePropertiesExtensionProvider $readWritePropertiesExtensionProvider,
4144
private array $properties,
4245
private array $propertyUsages,
4346
private array $methodCalls,
47+
private array $returnStatementNodes,
4448
)
4549
{
4650
parent::__construct($class->getAttributes());
@@ -191,10 +195,6 @@ public function getUninitializedProperties(
191195
}
192196

193197
if ($usage instanceof PropertyWrite) {
194-
if (array_key_exists($propertyName, $uninitializedProperties)) {
195-
unset($uninitializedProperties[$propertyName]);
196-
}
197-
198198
if (array_key_exists($propertyName, $initializedPropertiesMap)) {
199199
$hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)));
200200
if (!$hasInitialization->no() && !$usage->isPromotedPropertyWrite()) {
@@ -217,6 +217,57 @@ public function getUninitializedProperties(
217217
}
218218
}
219219

220+
foreach (array_keys($methodsCalledFromConstructor) as $constructor) {
221+
$lowerConstructorName = strtolower($constructor);
222+
if (!array_key_exists($lowerConstructorName, $this->returnStatementNodes)) {
223+
continue;
224+
}
225+
226+
$returnStatementsNode = $this->returnStatementNodes[$lowerConstructorName];
227+
$methodScope = null;
228+
foreach ($returnStatementsNode->getExecutionEnds() as $executionEnd) {
229+
$statementResult = $executionEnd->getStatementResult();
230+
$endNode = $executionEnd->getNode();
231+
if ($statementResult->isAlwaysTerminating()) {
232+
if ($endNode instanceof Node\Stmt\Throw_) {
233+
continue;
234+
}
235+
if ($endNode instanceof Node\Stmt\Expression) {
236+
$exprType = $statementResult->getScope()->getType($endNode->expr);
237+
if ($exprType instanceof NeverType && $exprType->isExplicit()) {
238+
continue;
239+
}
240+
}
241+
}
242+
if ($methodScope === null) {
243+
$methodScope = $statementResult->getScope();
244+
continue;
245+
}
246+
247+
$methodScope = $methodScope->mergeWith($statementResult->getScope());
248+
}
249+
250+
foreach ($returnStatementsNode->getReturnStatements() as $returnStatement) {
251+
if ($methodScope === null) {
252+
$methodScope = $returnStatement->getScope();
253+
continue;
254+
}
255+
$methodScope = $methodScope->mergeWith($returnStatement->getScope());
256+
}
257+
258+
if ($methodScope === null) {
259+
continue;
260+
}
261+
262+
foreach (array_keys($uninitializedProperties) as $propertyName) {
263+
if (!$methodScope->hasExpressionType(new PropertyInitializationExpr($propertyName))->yes()) {
264+
continue;
265+
}
266+
267+
unset($uninitializedProperties[$propertyName]);
268+
}
269+
}
270+
220271
return [
221272
$uninitializedProperties,
222273
$prematureAccess,

src/Node/ClassStatementsGatherer.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use PHPStan\Type\TypeUtils;
2121
use function count;
2222
use function in_array;
23+
use function strtolower;
2324

2425
class ClassStatementsGatherer
2526
{
@@ -50,6 +51,9 @@ class ClassStatementsGatherer
5051
/** @var ClassConstantFetch[] */
5152
private array $constantFetches = [];
5253

54+
/** @var array<string, MethodReturnStatementsNode> */
55+
private array $returnStatementNodes = [];
56+
5357
/**
5458
* @param callable(Node $node, Scope $scope): void $nodeCallback
5559
*/
@@ -109,6 +113,14 @@ public function getConstantFetches(): array
109113
return $this->constantFetches;
110114
}
111115

116+
/**
117+
* @return array<string, MethodReturnStatementsNode>
118+
*/
119+
public function getReturnStatementsNodes(): array
120+
{
121+
return $this->returnStatementNodes;
122+
}
123+
112124
public function __invoke(Node $node, Scope $scope): void
113125
{
114126
$nodeCallback = $this->nodeCallback;
@@ -151,6 +163,10 @@ private function gatherNodes(Node $node, Scope $scope): void
151163
$this->methodCalls[] = new \PHPStan\Node\Method\MethodCall($node->getOriginalNode(), $scope);
152164
return;
153165
}
166+
if ($node instanceof MethodReturnStatementsNode) {
167+
$this->returnStatementNodes[strtolower($node->getMethodName())] = $node;
168+
return;
169+
}
154170
if (
155171
$node instanceof Expr\FuncCall
156172
&& $node->name instanceof Node\Name

src/Node/MethodReturnStatementsNode.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ public function hasNativeReturnTypehint(): bool
5757
return $this->classMethod->returnType !== null;
5858
}
5959

60+
public function getMethodName(): string
61+
{
62+
return $this->classMethod->name->toString();
63+
}
64+
6065
public function getYieldStatements(): array
6166
{
6267
return $this->yieldStatements;

tests/PHPStan/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRuleTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ public function testRule(): void
8484
'Class MissingReadOnlyPropertyAssignPhpDoc\BarDoubleAssignInSetter has an uninitialized @readonly property $foo. Assign it in the constructor.',
8585
57,
8686
],
87+
[
88+
'Class MissingReadOnlyPropertyAssignPhpDoc\AssignOp has an uninitialized @readonly property $foo. Assign it in the constructor.',
89+
85,
90+
],
8791
[
8892
'Access to an uninitialized @readonly property MissingReadOnlyPropertyAssignPhpDoc\AssignOp::$foo.',
8993
92,

tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ public function testRule(): void
8484
'Class MissingReadOnlyPropertyAssign\BarDoubleAssignInSetter has an uninitialized readonly property $foo. Assign it in the constructor.',
8585
53,
8686
],
87+
[
88+
'Class MissingReadOnlyPropertyAssign\AssignOp has an uninitialized readonly property $foo. Assign it in the constructor.',
89+
79,
90+
],
8791
[
8892
'Access to an uninitialized readonly property MissingReadOnlyPropertyAssign\AssignOp::$foo.',
8993
85,
@@ -195,4 +199,18 @@ public function testBug7198(): void
195199
$this->analyse([__DIR__ . '/data/bug-7198.php'], []);
196200
}
197201

202+
public function testBug7649(): void
203+
{
204+
if (PHP_VERSION_ID < 80100) {
205+
$this->markTestSkipped('Test requires PHP 8.1.');
206+
}
207+
208+
$this->analyse([__DIR__ . '/data/bug-7649.php'], [
209+
[
210+
'Class Bug7649\Foo has an uninitialized readonly property $bar. Assign it in the constructor.',
211+
7,
212+
],
213+
]);
214+
}
215+
198216
}

tests/PHPStan/Rules/Properties/UninitializedPropertyRuleTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,18 @@ public function testRule(): void
9191
'Access to an uninitialized property UninitializedProperty\InitializedInPublicSetterNonFinalClass::$foo.',
9292
278,
9393
],*/
94+
[
95+
'Class UninitializedProperty\SometimesInitializedInPrivateSetter has an uninitialized property $foo. Give it default value or assign it in the constructor.',
96+
286,
97+
],
9498
[
9599
'Access to an uninitialized property UninitializedProperty\SometimesInitializedInPrivateSetter::$foo.',
96100
303,
97101
],
102+
[
103+
'Class UninitializedProperty\EarlyReturn has an uninitialized property $foo. Give it default value or assign it in the constructor.',
104+
372,
105+
],
98106
]);
99107
}
100108

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
namespace Bug7649;
4+
5+
class Foo
6+
{
7+
public readonly string $bar;
8+
9+
public function __construct(bool $flag)
10+
{
11+
if ($flag) {
12+
$this->bar = 'baz';
13+
} else {
14+
}
15+
}
16+
}

tests/PHPStan/Rules/Properties/data/uninitialized-property.php

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,3 +332,77 @@ public function doSomething()
332332
}
333333

334334
}
335+
336+
class ThrowInConstructor1
337+
{
338+
339+
private int $foo;
340+
341+
public function __construct()
342+
{
343+
if (rand(0, 1)) {
344+
$this->foo = 1;
345+
return;
346+
}
347+
348+
throw new \Exception;
349+
}
350+
351+
}
352+
353+
class ThrowInConstructor2
354+
{
355+
356+
private int $foo;
357+
358+
public function __construct()
359+
{
360+
if (rand(0, 1)) {
361+
throw new \Exception;
362+
}
363+
364+
$this->foo = 1;
365+
}
366+
367+
}
368+
369+
class EarlyReturn
370+
{
371+
372+
private int $foo;
373+
374+
public function __construct()
375+
{
376+
if (rand(0, 1)) {
377+
return;
378+
}
379+
380+
$this->foo = 1;
381+
}
382+
383+
}
384+
385+
class NeverInConstructor
386+
{
387+
388+
private int $foo;
389+
390+
public function __construct()
391+
{
392+
if (rand(0, 1)) {
393+
$this->foo = 1;
394+
return;
395+
}
396+
397+
$this->returnNever();
398+
}
399+
400+
/**
401+
* @return never
402+
*/
403+
private function returnNever()
404+
{
405+
throw new \Exception();
406+
}
407+
408+
}

0 commit comments

Comments
 (0)