Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,7 @@ private function processStmtNode(
}

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

if ($alwaysIterates) {
$isAlwaysTerminating = count($finalScopeResult->getExitPointsByType(Break_::class)) === 0;
Expand Down Expand Up @@ -1607,7 +1607,7 @@ private function processStmtNode(
$alwaysIterates = $condBooleanType->isTrue()->yes();
}

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

if ($alwaysIterates) {
$alwaysTerminating = count($bodyScopeResult->getExitPointsByType(Break_::class)) === 0;
Expand Down
7 changes: 6 additions & 1 deletion src/Node/BreaklessWhileLoopNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ final class BreaklessWhileLoopNode extends NodeAbstract implements VirtualNode
/**
* @param StatementExitPoint[] $exitPoints
*/
public function __construct(private While_ $originalNode, private array $exitPoints)
public function __construct(private While_ $originalNode, private array $exitPoints, private bool $hasYield)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw: I was not sure whether the yields should be within the exitPoints

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but InternalStatementResult already differentiate $hasYield and $exitPoints so it seems consistent to me.

{
parent::__construct($originalNode->getAttributes());
}
Expand All @@ -34,6 +34,11 @@ public function getExitPoints(): array
return $this->exitPoints;
}

public function hasYield(): bool
{
return $this->hasYield;
}

#[Override]
public function getType(): string
{
Expand Down
7 changes: 6 additions & 1 deletion src/Node/DoWhileLoopConditionNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ final class DoWhileLoopConditionNode extends NodeAbstract implements VirtualNode
/**
* @param StatementExitPoint[] $exitPoints
*/
public function __construct(private Expr $cond, private array $exitPoints)
public function __construct(private Expr $cond, private array $exitPoints, private bool $hasYield)
{
parent::__construct($cond->getAttributes());
}
Expand All @@ -31,6 +31,11 @@ public function getExitPoints(): array
return $this->exitPoints;
}

public function hasYield(): bool
{
return $this->hasYield;
}

#[Override]
public function getType(): string
{
Expand Down
3 changes: 3 additions & 0 deletions src/Rules/Comparison/DoWhileLoopConstantConditionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public function processNode(Node $node, Scope $scope): array
$exprType = $this->helper->getBooleanType($scope, $node->getCond());
if ($exprType instanceof ConstantBooleanType) {
if ($exprType->getValue()) {
if ($node->hasYield()) {
return [];
}
foreach ($node->getExitPoints() as $exitPoint) {
$statement = $exitPoint->getStatement();
if (!$statement instanceof Continue_) {
Expand Down
4 changes: 4 additions & 0 deletions src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public function processNode(
$originalNode = $node->getOriginalNode();
$exprType = $this->helper->getBooleanType($scope, $originalNode->cond);
if ($exprType->isTrue()->yes()) {
if ($node->hasYield()) {
return [];
}

$ref = $scope->getFunction() ?? $scope->getAnonymousFunctionReflection();

if ($ref !== null && $ref->getReturnType() instanceof NeverType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ protected function getRule(): Rule
);
}

public function testBug6189(): void
{
$this->analyse([__DIR__ . '/data/bug-6189.php'], []);
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/do-while-loop.php'], [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,23 @@ public function testRulePHP81(): void
$this->analyse([__DIR__ . '/data/while-loop-true-php81.php'], []);
}

public function testBug10054(): void
{
$this->analyse([__DIR__ . '/data/bug-10054.php'], []);
}

public function testBug6189(): void
{
$this->analyse([__DIR__ . '/data/bug-6189.php'], [
[
'While loop condition is always true.',
33,
],
[
'While loop condition is always true.',
44,
],
]);
}

}
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-10054.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

namespace Bug10054;

function falsePositive(): \Generator {
while (true) {
$item = yield;

yield $item;
}
}

$generator = falsePositive();

var_dump($generator->send('foo'));
59 changes: 59 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-6189.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php declare(strict_types = 1);

namespace Bug6189;

use Generator;

class Foo
{

/**
* @return Generator<int, int, null, void>
*/
public function generatorWithYield(): Generator
{
while (true) {
yield 1;
}
}

/**
* @return Generator<int, int, null, void>
*/
public function generatorWithYieldFrom(): Generator
{
while (true) {
yield from [1, 2, 3];
}
}

/** Still an infinite loop - no yield inside the while body */
public function noYieldInLoop(): void
{
while (true) {

}
}

/**
* @return Generator<int, int, null, void>
*/
public function generatorWithYieldOutsideLoop(): Generator
{
yield 0;
while (true) {
// yield is outside the loop, so this is still an infinite loop
}
}

/**
* @return Generator<int, int, null, void>
*/
public function generatorDoWhileWithYield(): Generator
{
do {
yield 1;
} while (true);
}

}
Loading