Skip to content

Commit 22f51ec

Browse files
committed
Fix execution branch confusion with multiple variable assignments in if/else
- Changed createConditionalExpressions to create pairwise conditional holders instead of one holder with all guards - In a binary branch merge, any single type guard uniquely identifies the branch, so individual guards are sufficient - Previously, 3+ type guards created holders requiring all-minus-one conditions, causing circular dependency in resolution - New regression test in tests/PHPStan/Analyser/nsrt/bug-5051.php Closes phpstan/phpstan#5051
1 parent d462113 commit 22f51ec

File tree

2 files changed

+77
-4
lines changed

2 files changed

+77
-4
lines changed

src/Analyser/MutatingScope.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4141,17 +4141,21 @@ private function createConditionalExpressions(
41414141
continue;
41424142
}
41434143

4144-
$conditionalExpression = new ConditionalExpressionHolder($variableTypeGuards, $holder);
4145-
$conditionalExpressions[$exprString][$conditionalExpression->getKey()] = $conditionalExpression;
4144+
foreach ($variableTypeGuards as $guardExprString => $guardHolder) {
4145+
$conditionalExpression = new ConditionalExpressionHolder([$guardExprString => $guardHolder], $holder);
4146+
$conditionalExpressions[$exprString][$conditionalExpression->getKey()] = $conditionalExpression;
4147+
}
41464148
}
41474149

41484150
foreach ($mergedExpressionTypes as $exprString => $mergedExprTypeHolder) {
41494151
if (array_key_exists($exprString, $ourExpressionTypes)) {
41504152
continue;
41514153
}
41524154

4153-
$conditionalExpression = new ConditionalExpressionHolder($typeGuards, new ExpressionTypeHolder($mergedExprTypeHolder->getExpr(), new ErrorType(), TrinaryLogic::createNo()));
4154-
$conditionalExpressions[$exprString][$conditionalExpression->getKey()] = $conditionalExpression;
4155+
foreach ($typeGuards as $guardExprString => $guardHolder) {
4156+
$conditionalExpression = new ConditionalExpressionHolder([$guardExprString => $guardHolder], new ExpressionTypeHolder($mergedExprTypeHolder->getExpr(), new ErrorType(), TrinaryLogic::createNo()));
4157+
$conditionalExpressions[$exprString][$conditionalExpression->getKey()] = $conditionalExpression;
4158+
}
41554159
}
41564160

41574161
return $conditionalExpressions;
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug5051;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class HelloWorld
8+
{
9+
public function test(?object $data): void
10+
{
11+
if ($data === null) {
12+
$foo = 'bar';
13+
$update = false;
14+
} else {
15+
$foo = 'baz';
16+
$update = true;
17+
}
18+
19+
assertType('object|null', $data);
20+
assertType("'bar'|'baz'", $foo);
21+
22+
if ($update) {
23+
assertType('object', $data);
24+
}
25+
}
26+
27+
public function testWithBooleans(?object $data): void
28+
{
29+
if ($data === null) {
30+
$update = false;
31+
$foo = false;
32+
} else {
33+
$update = true;
34+
$foo = true;
35+
}
36+
37+
if ($update) {
38+
assertType('object', $data);
39+
}
40+
}
41+
42+
public function testWithDifferentVariableNames(?object $data): void
43+
{
44+
if ($data === null) {
45+
$update = false;
46+
$foo = 'bar';
47+
} else {
48+
$update = true;
49+
$fuu = 'baz';
50+
}
51+
52+
if ($update) {
53+
assertType('object', $data);
54+
}
55+
}
56+
57+
public function testWithoutExtraAssignment(?object $data): void
58+
{
59+
if ($data === null) {
60+
$update = false;
61+
} else {
62+
$update = true;
63+
}
64+
65+
if ($update) {
66+
assertType('object', $data);
67+
}
68+
}
69+
}

0 commit comments

Comments
 (0)