Skip to content

Commit 295d850

Browse files
VincentLangletVincentLanglet
authored
Fix while-true-if-break: Variable might not be defined (#5118)
1 parent 661babe commit 295d850

File tree

8 files changed

+304
-6
lines changed

8 files changed

+304
-6
lines changed

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,8 +1522,12 @@ private function processStmtNode(
15221522
}
15231523

15241524
$breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class);
1525-
foreach ($breakExitPoints as $breakExitPoint) {
1526-
$finalScope = $finalScope->mergeWith($breakExitPoint->getScope());
1525+
if (count($breakExitPoints) > 0) {
1526+
$breakScope = $alwaysIterates ? null : $finalScope;
1527+
foreach ($breakExitPoints as $breakExitPoint) {
1528+
$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
1529+
}
1530+
$finalScope = $breakScope;
15271531
}
15281532

15291533
$isIterableAtLeastOnce = $beforeCondBooleanType->isTrue()->yes();
@@ -1629,8 +1633,14 @@ private function processStmtNode(
16291633
} else {
16301634
$this->processExprNode($stmt, $stmt->cond, $bodyScope, $storage, $nodeCallback, ExpressionContext::createDeep());
16311635
}
1632-
foreach ($bodyScopeResult->getExitPointsByType(Break_::class) as $breakExitPoint) {
1633-
$finalScope = $breakExitPoint->getScope()->mergeWith($finalScope);
1636+
1637+
$breakExitPoints = $bodyScopeResult->getExitPointsByType(Break_::class);
1638+
if (count($breakExitPoints) > 0) {
1639+
$breakScope = $alwaysIterates ? null : $finalScope;
1640+
foreach ($breakExitPoints as $breakExitPoint) {
1641+
$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
1642+
}
1643+
$finalScope = $breakScope;
16341644
}
16351645

16361646
return new InternalStatementResult(
@@ -1741,8 +1751,13 @@ private function processStmtNode(
17411751
$finalScope = $finalScope->filterByFalseyValue($lastCondExpr);
17421752
}
17431753

1744-
foreach ($finalScopeResult->getExitPointsByType(Break_::class) as $breakExitPoint) {
1745-
$finalScope = $breakExitPoint->getScope()->mergeWith($finalScope);
1754+
$breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class);
1755+
if (count($breakExitPoints) > 0) {
1756+
$breakScope = $alwaysIterates->yes() ? null : $finalScope;
1757+
foreach ($breakExitPoints as $breakExitPoint) {
1758+
$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
1759+
}
1760+
$finalScope = $breakScope;
17461761
}
17471762

17481763
if ($isIterableAtLeastOnce->no() || $finalScopeResult->isAlwaysTerminating()) {

‎tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,4 +1257,80 @@ public function testBug12944(): void
12571257
$this->analyse([__DIR__ . '/data/bug-12944.php'], []);
12581258
}
12591259

1260+
public function testBug9023(): void
1261+
{
1262+
$this->cliArgumentsVariablesRegistered = true;
1263+
$this->polluteScopeWithLoopInitialAssignments = false;
1264+
$this->checkMaybeUndefinedVariables = true;
1265+
$this->polluteScopeWithAlwaysIterableForeach = true;
1266+
1267+
$this->analyse([__DIR__ . '/data/bug-9023.php'], []);
1268+
}
1269+
1270+
public function testBug11984(): void
1271+
{
1272+
$this->cliArgumentsVariablesRegistered = true;
1273+
$this->polluteScopeWithLoopInitialAssignments = false;
1274+
$this->checkMaybeUndefinedVariables = true;
1275+
$this->polluteScopeWithAlwaysIterableForeach = true;
1276+
1277+
$this->analyse([__DIR__ . '/data/bug-11984.php'], []);
1278+
}
1279+
1280+
#[DataProvider('dataBug11545')]
1281+
public function testBug11545(bool $polluteScopeWithLoopInitialAssignments): void
1282+
{
1283+
$this->cliArgumentsVariablesRegistered = true;
1284+
$this->polluteScopeWithLoopInitialAssignments = $polluteScopeWithLoopInitialAssignments;
1285+
$this->checkMaybeUndefinedVariables = true;
1286+
$this->polluteScopeWithAlwaysIterableForeach = true;
1287+
1288+
$errors = [];
1289+
if (!$polluteScopeWithLoopInitialAssignments) {
1290+
$errors[] = [
1291+
'Variable $result might not be defined.',
1292+
24,
1293+
];
1294+
}
1295+
1296+
$this->analyse([__DIR__ . '/data/bug-11545.php'], $errors);
1297+
}
1298+
1299+
/** @return iterable<array{bool}> */
1300+
public static function dataBug11545(): iterable
1301+
{
1302+
yield [false];
1303+
yield [true];
1304+
}
1305+
1306+
public function testBug10245(): void
1307+
{
1308+
$this->cliArgumentsVariablesRegistered = true;
1309+
$this->polluteScopeWithLoopInitialAssignments = false;
1310+
$this->checkMaybeUndefinedVariables = true;
1311+
$this->polluteScopeWithAlwaysIterableForeach = true;
1312+
1313+
$this->analyse([__DIR__ . '/data/bug-10245.php'], []);
1314+
}
1315+
1316+
public function testBug5919(): void
1317+
{
1318+
$this->cliArgumentsVariablesRegistered = true;
1319+
$this->polluteScopeWithLoopInitialAssignments = false;
1320+
$this->checkMaybeUndefinedVariables = true;
1321+
$this->polluteScopeWithAlwaysIterableForeach = true;
1322+
1323+
$this->analyse([__DIR__ . '/data/bug-5919.php'], []);
1324+
}
1325+
1326+
public function testBug5477(): void
1327+
{
1328+
$this->cliArgumentsVariablesRegistered = true;
1329+
$this->polluteScopeWithLoopInitialAssignments = false;
1330+
$this->checkMaybeUndefinedVariables = true;
1331+
$this->polluteScopeWithAlwaysIterableForeach = true;
1332+
1333+
$this->analyse([__DIR__ . '/data/bug-5477.php'], []);
1334+
}
1335+
12601336
}

‎tests/PHPStan/Rules/Variables/data/bug-10245.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types = 1); // lint >= 8.0
2+
3+
namespace Bug10245;
4+
5+
/**
6+
* @throws \Exception
7+
*/
8+
function produceInt(): int
9+
{
10+
return 1;
11+
}
12+
13+
function testTryCatchInWhileTrue(): void
14+
{
15+
while (true) {
16+
try {
17+
$a = produceInt();
18+
break;
19+
} catch (\Throwable) {}
20+
}
21+
22+
echo $a;
23+
}
24+
25+
function testIfBreakInWhileTrue(int $max): void
26+
{
27+
$i = 0;
28+
while (true) {
29+
if ($i > $max) {
30+
$result = 'done';
31+
break;
32+
}
33+
++$i;
34+
}
35+
print $result;
36+
}

‎tests/PHPStan/Rules/Variables/data/bug-11545.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug11545;
4+
5+
function foo(int $max): void {
6+
$i = 0;
7+
while (true) {
8+
if ($i > $max) {
9+
$result = 'done';
10+
break;
11+
}
12+
++$i;
13+
}
14+
print $result;
15+
}
16+
17+
function foo_for(int $max): void {
18+
for ($i = 0;; ++$i) {
19+
if ($i > $max) {
20+
$result = 'done';
21+
break;
22+
}
23+
}
24+
print $result;
25+
}
26+
27+
function foo_do_while(int $max): void {
28+
$i = 0;
29+
do {
30+
if ($i > $max) {
31+
$result = 'done';
32+
break;
33+
}
34+
++$i;
35+
} while (true);
36+
print $result;
37+
}
38+
39+
function bar(int $max): void {
40+
while (true) {
41+
$result = 'done';
42+
break;
43+
}
44+
print $result;
45+
}

‎tests/PHPStan/Rules/Variables/data/bug-11984.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);
2+
3+
namespace Bug11984;
4+
5+
class Foo
6+
{
7+
/**
8+
* @return array<string, mixed>
9+
*/
10+
public function loadFromFile(): array
11+
{
12+
return ['x' => 1];
13+
}
14+
15+
/**
16+
* @return array<string, mixed>
17+
*/
18+
public function test(): array
19+
{
20+
while (true) {
21+
try {
22+
$data = $this->loadFromFile();
23+
24+
break;
25+
} catch (\Exception $ex) {
26+
}
27+
}
28+
29+
return $data;
30+
}
31+
}

‎tests/PHPStan/Rules/Variables/data/bug-5477.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);
2+
3+
namespace Bug5477;
4+
5+
function foo(): int {
6+
while (true) {
7+
try {
8+
$transaction = 1;
9+
break;
10+
} catch (\Throwable $e) {
11+
continue;
12+
}
13+
}
14+
15+
return $transaction;
16+
}

‎tests/PHPStan/Rules/Variables/data/bug-5919.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Bug5919;
6+
7+
/**
8+
* @return mixed[]
9+
*/
10+
function queryApi(): array
11+
{
12+
return [5];
13+
}
14+
15+
function isTimeout(): bool
16+
{
17+
return true;
18+
}
19+
20+
function testSimple(): void
21+
{
22+
while (true) {
23+
try {
24+
$s = queryApi();
25+
break;
26+
} catch (\Exception $e) {
27+
if (false) {
28+
throw $e;
29+
}
30+
}
31+
}
32+
33+
var_dump(count($s));
34+
}
35+
36+
function testWithIsTimeout(): void
37+
{
38+
while (true) {
39+
try {
40+
$s = queryApi();
41+
break;
42+
} catch (\Exception $e) {
43+
if (isTimeout()) {
44+
throw $e;
45+
}
46+
}
47+
}
48+
49+
var_dump(count($s));
50+
}

‎tests/PHPStan/Rules/Variables/data/bug-9023.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug9023;
4+
5+
use Exception;
6+
7+
function unreliableFunc(): string
8+
{
9+
if (random_int(1, 5) === 1) {
10+
return 'something';
11+
} else {
12+
throw new Exception('Just demonstrating');
13+
}
14+
}
15+
16+
function testWhileTrueBreakWithTryCatch(): void
17+
{
18+
while (true) {
19+
try {
20+
$defined = unreliableFunc();
21+
break;
22+
} catch (Exception $e) {
23+
sleep(10);
24+
continue;
25+
}
26+
}
27+
28+
echo $defined;
29+
}

0 commit comments

Comments
 (0)