Skip to content

Commit 566b101

Browse files
github-actions[bot]phpstan-bot
authored andcommitted
Fix phpstan/phpstan#5865: Do-while loop broken by exception not supported
Detect void methods/functions that always terminate (throw/exit) during body analysis and suppress false "always true/false" loop condition reports when such calls appear in loop bodies.
1 parent 04a99c1 commit 566b101

File tree

5 files changed

+194
-0
lines changed

5 files changed

+194
-0
lines changed

src/Analyser/NodeScopeResolver.php

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ class NodeScopeResolver
194194
/** @var array<string, true> */
195195
private array $earlyTerminatingMethodNames;
196196

197+
/** @var array<string, true> */
198+
private array $alwaysTerminatingCallables = [];
199+
197200
/** @var array<string, true> */
198201
private array $calledMethodStack = [];
199202

@@ -660,6 +663,15 @@ public function processStmtNode(
660663
array_merge($statementResult->getImpurePoints(), $functionImpurePoints),
661664
$functionReflection,
662665
), $functionScope, $storage);
666+
667+
if (
668+
count($gatheredReturnStatements) === 0
669+
&& count($gatheredYieldStatements) === 0
670+
&& $this->isBodyAlwaysTerminating($executionEnds)
671+
) {
672+
$this->alwaysTerminatingCallables[strtolower($functionReflection->getName())] = true;
673+
}
674+
663675
if (!$scope->isInAnonymousFunction()) {
664676
$this->processPendingFibers($storage);
665677
}
@@ -825,6 +837,14 @@ public function processStmtNode(
825837
$methodReflection,
826838
), $methodScope, $storage);
827839

840+
if (
841+
count($gatheredReturnStatements) === 0
842+
&& count($gatheredYieldStatements) === 0
843+
&& $this->isBodyAlwaysTerminating($executionEnds)
844+
) {
845+
$this->alwaysTerminatingCallables[strtolower($classReflection->getName() . '::' . $methodReflection->getName())] = true;
846+
}
847+
828848
if ($isConstructor) {
829849
$finalScope = null;
830850

@@ -2454,12 +2474,23 @@ private function findEarlyTerminatingExpr(Expr $expr, Scope $scope): ?Expr
24542474
}
24552475
}
24562476
}
2477+
2478+
if (!$expr->isFirstClassCallable() && $this->isMethodCallAlwaysTerminating($expr, $scope)) {
2479+
return $expr;
2480+
}
24572481
}
24582482

24592483
if ($expr instanceof FuncCall && $expr->name instanceof Name) {
24602484
if (in_array((string) $expr->name, $this->earlyTerminatingFunctionCalls, true)) {
24612485
return $expr;
24622486
}
2487+
2488+
if (!$expr->isFirstClassCallable()) {
2489+
$resolvedFunctionName = $this->reflectionProvider->resolveFunctionName($expr->name, $scope);
2490+
if ($resolvedFunctionName !== null && isset($this->alwaysTerminatingCallables[strtolower($resolvedFunctionName)])) {
2491+
return $expr;
2492+
}
2493+
}
24632494
}
24642495

24652496
if ($expr instanceof Expr\Exit_ || $expr instanceof Expr\Throw_) {
@@ -2474,6 +2505,50 @@ private function findEarlyTerminatingExpr(Expr $expr, Scope $scope): ?Expr
24742505
return null;
24752506
}
24762507

2508+
private function isMethodCallAlwaysTerminating(MethodCall|Expr\StaticCall $expr, Scope $scope): bool
2509+
{
2510+
if (!$expr->name instanceof Node\Identifier) {
2511+
return false;
2512+
}
2513+
2514+
if ($expr instanceof MethodCall) {
2515+
$methodCalledOnType = $scope->getType($expr->var);
2516+
} else {
2517+
if ($expr->class instanceof Name) {
2518+
$methodCalledOnType = $scope->resolveTypeByName($expr->class);
2519+
} else {
2520+
$methodCalledOnType = $scope->getType($expr->class);
2521+
}
2522+
}
2523+
2524+
foreach ($methodCalledOnType->getObjectClassNames() as $referencedClass) {
2525+
$key = strtolower($referencedClass . '::' . $expr->name->toLowerString());
2526+
if (isset($this->alwaysTerminatingCallables[$key])) {
2527+
return true;
2528+
}
2529+
}
2530+
2531+
return false;
2532+
}
2533+
2534+
/**
2535+
* @param ExecutionEndNode[] $executionEnds
2536+
*/
2537+
private function isBodyAlwaysTerminating(array $executionEnds): bool
2538+
{
2539+
if ($executionEnds === []) {
2540+
return false;
2541+
}
2542+
2543+
foreach ($executionEnds as $executionEnd) {
2544+
if (!$executionEnd->getStatementResult()->isAlwaysTerminating()) {
2545+
return false;
2546+
}
2547+
}
2548+
2549+
return true;
2550+
}
2551+
24772552
/**
24782553
* @param callable(Node $node, Scope $scope): void $nodeCallback
24792554
*/

tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php

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

32+
public function testBug5865(): void
33+
{
34+
$this->analyse([__DIR__ . '/data/bug-5865.php'], []);
35+
}
36+
3237
public function testBug6189(): void
3338
{
3439
$this->analyse([__DIR__ . '/data/bug-6189.php'], []);

tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ public function testBug10054(): void
6060
$this->analyse([__DIR__ . '/data/bug-10054.php'], []);
6161
}
6262

63+
public function testBug5865(): void
64+
{
65+
$this->analyse([__DIR__ . '/data/bug-5865-while.php'], []);
66+
}
67+
6368
public function testBug6189(): void
6469
{
6570
$this->analyse([__DIR__ . '/data/bug-6189.php'], [
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
namespace Bug5865While;
4+
5+
final class HelloWorld
6+
{
7+
8+
public function alwaysThrows(): void
9+
{
10+
throw new \Exception();
11+
}
12+
13+
public function whileLiteralTrue(): void
14+
{
15+
while (true) {
16+
$this->alwaysThrows();
17+
}
18+
}
19+
20+
public static function staticAlwaysThrows(): void
21+
{
22+
throw new \Exception();
23+
}
24+
25+
public function whileStaticCall(): void
26+
{
27+
while (true) {
28+
self::staticAlwaysThrows();
29+
}
30+
}
31+
32+
}
33+
34+
function alwaysThrowsFunction(): void
35+
{
36+
throw new \Exception();
37+
}
38+
39+
function whileFunctionCall(): void
40+
{
41+
while (true) {
42+
alwaysThrowsFunction();
43+
}
44+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
namespace Bug5865;
4+
5+
final class HelloWorld
6+
{
7+
8+
public function alwaysThrows(): void
9+
{
10+
throw new \Exception();
11+
}
12+
13+
public function doWhileLiteralTrue(): void
14+
{
15+
do {
16+
$this->alwaysThrows();
17+
} while (true); // should not report - body always terminates
18+
}
19+
20+
public function doWhileVariableCondition(): void
21+
{
22+
$a = true;
23+
do {
24+
$this->alwaysThrows();
25+
$a = false;
26+
} while ($a); // should not report - body always terminates
27+
}
28+
29+
public function neverReturnType(): never
30+
{
31+
throw new \Exception();
32+
}
33+
34+
public function doWhileNeverReturnType(): void
35+
{
36+
do {
37+
$this->neverReturnType();
38+
} while (true); // already works - no error
39+
}
40+
41+
public static function staticAlwaysThrows(): void
42+
{
43+
throw new \Exception();
44+
}
45+
46+
public function doWhileStaticCall(): void
47+
{
48+
do {
49+
self::staticAlwaysThrows();
50+
} while (true); // should not report - body always terminates
51+
}
52+
53+
}
54+
55+
function alwaysThrowsFunction(): void
56+
{
57+
throw new \Exception();
58+
}
59+
60+
function doWhileFunctionCall(): void
61+
{
62+
do {
63+
alwaysThrowsFunction();
64+
} while (true); // should not report - body always terminates
65+
}

0 commit comments

Comments
 (0)