Skip to content

Commit f1cf75f

Browse files
committed
Suppress always-true loop warnings when loop body contains yield
- Generator suspension points (yield/yield from) act as break points, so while(true) { yield $x; } is valid and not a true infinite loop - Added hasYield flag to BreaklessWhileLoopNode and DoWhileLoopConditionNode - WhileLoopAlwaysTrueConditionRule and DoWhileLoopConstantConditionRule now skip the warning when the loop body contains a yield - Loops without yield in a generator still correctly report the warning Closes phpstan/phpstan#6189
1 parent d462113 commit f1cf75f

File tree

8 files changed

+99
-4
lines changed

8 files changed

+99
-4
lines changed

src/Analyser/NodeScopeResolver.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,7 +1525,7 @@ private function processStmtNode(
15251525
}
15261526

15271527
$isIterableAtLeastOnce = $beforeCondBooleanType->isTrue()->yes();
1528-
$this->callNodeCallback($nodeCallback, new BreaklessWhileLoopNode($stmt, $finalScopeResult->toPublic()->getExitPoints()), $bodyScopeMaybeRan, $storage);
1528+
$this->callNodeCallback($nodeCallback, new BreaklessWhileLoopNode($stmt, $finalScopeResult->toPublic()->getExitPoints(), $finalScopeResult->hasYield()), $bodyScopeMaybeRan, $storage);
15291529

15301530
if ($alwaysIterates) {
15311531
$isAlwaysTerminating = count($finalScopeResult->getExitPointsByType(Break_::class)) === 0;
@@ -1607,7 +1607,7 @@ private function processStmtNode(
16071607
$alwaysIterates = $condBooleanType->isTrue()->yes();
16081608
}
16091609

1610-
$this->callNodeCallback($nodeCallback, new DoWhileLoopConditionNode($stmt->cond, $bodyScopeResult->toPublic()->getExitPoints()), $bodyScope, $storage);
1610+
$this->callNodeCallback($nodeCallback, new DoWhileLoopConditionNode($stmt->cond, $bodyScopeResult->toPublic()->getExitPoints(), $bodyScopeResult->hasYield()), $bodyScope, $storage);
16111611

16121612
if ($alwaysIterates) {
16131613
$alwaysTerminating = count($bodyScopeResult->getExitPointsByType(Break_::class)) === 0;

src/Node/BreaklessWhileLoopNode.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ final class BreaklessWhileLoopNode extends NodeAbstract implements VirtualNode
1616
/**
1717
* @param StatementExitPoint[] $exitPoints
1818
*/
19-
public function __construct(private While_ $originalNode, private array $exitPoints)
19+
public function __construct(private While_ $originalNode, private array $exitPoints, private bool $hasYield = false)
2020
{
2121
parent::__construct($originalNode->getAttributes());
2222
}
@@ -34,6 +34,11 @@ public function getExitPoints(): array
3434
return $this->exitPoints;
3535
}
3636

37+
public function hasYield(): bool
38+
{
39+
return $this->hasYield;
40+
}
41+
3742
#[Override]
3843
public function getType(): string
3944
{

src/Node/DoWhileLoopConditionNode.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ final class DoWhileLoopConditionNode extends NodeAbstract implements VirtualNode
1313
/**
1414
* @param StatementExitPoint[] $exitPoints
1515
*/
16-
public function __construct(private Expr $cond, private array $exitPoints)
16+
public function __construct(private Expr $cond, private array $exitPoints, private bool $hasYield = false)
1717
{
1818
parent::__construct($cond->getAttributes());
1919
}
@@ -31,6 +31,11 @@ public function getExitPoints(): array
3131
return $this->exitPoints;
3232
}
3333

34+
public function hasYield(): bool
35+
{
36+
return $this->hasYield;
37+
}
38+
3439
#[Override]
3540
public function getType(): string
3641
{

src/Rules/Comparison/DoWhileLoopConstantConditionRule.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ public function processNode(Node $node, Scope $scope): array
4242
$exprType = $this->helper->getBooleanType($scope, $node->getCond());
4343
if ($exprType instanceof ConstantBooleanType) {
4444
if ($exprType->getValue()) {
45+
if ($node->hasYield()) {
46+
return [];
47+
}
4548
foreach ($node->getExitPoints() as $exitPoint) {
4649
$statement = $exitPoint->getStatement();
4750
if (!$statement instanceof Continue_) {

src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ public function processNode(
6868
$originalNode = $node->getOriginalNode();
6969
$exprType = $this->helper->getBooleanType($scope, $originalNode->cond);
7070
if ($exprType->isTrue()->yes()) {
71+
if ($node->hasYield()) {
72+
return [];
73+
}
74+
7175
$ref = $scope->getFunction() ?? $scope->getAnonymousFunctionReflection();
7276

7377
if ($ref !== null && $ref->getReturnType() instanceof NeverType) {

tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ protected function getRule(): Rule
2828
);
2929
}
3030

31+
public function testBug6189(): void
32+
{
33+
$this->analyse([__DIR__ . '/data/bug-6189.php'], []);
34+
}
35+
3136
public function testRule(): void
3237
{
3338
$this->analyse([__DIR__ . '/data/do-while-loop.php'], [

tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,18 @@ public function testRulePHP81(): void
5454
$this->analyse([__DIR__ . '/data/while-loop-true-php81.php'], []);
5555
}
5656

57+
public function testBug6189(): void
58+
{
59+
$this->analyse([__DIR__ . '/data/bug-6189.php'], [
60+
[
61+
'While loop condition is always true.',
62+
33,
63+
],
64+
[
65+
'While loop condition is always true.',
66+
44,
67+
],
68+
]);
69+
}
70+
5771
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug6189;
4+
5+
use Generator;
6+
7+
class Foo
8+
{
9+
10+
/**
11+
* @return Generator<int, int, null, void>
12+
*/
13+
public function generatorWithYield(): Generator
14+
{
15+
while (true) {
16+
yield 1;
17+
}
18+
}
19+
20+
/**
21+
* @return Generator<int, int, null, void>
22+
*/
23+
public function generatorWithYieldFrom(): Generator
24+
{
25+
while (true) {
26+
yield from [1, 2, 3];
27+
}
28+
}
29+
30+
/** Still an infinite loop - no yield inside the while body */
31+
public function noYieldInLoop(): void
32+
{
33+
while (true) {
34+
35+
}
36+
}
37+
38+
/**
39+
* @return Generator<int, int, null, void>
40+
*/
41+
public function generatorWithYieldOutsideLoop(): Generator
42+
{
43+
yield 0;
44+
while (true) {
45+
// yield is outside the loop, so this is still an infinite loop
46+
}
47+
}
48+
49+
/**
50+
* @return Generator<int, int, null, void>
51+
*/
52+
public function generatorDoWhileWithYield(): Generator
53+
{
54+
do {
55+
yield 1;
56+
} while (true);
57+
}
58+
59+
}

0 commit comments

Comments
 (0)