diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index f616b644ab3..b832bb30e8b 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -194,6 +194,9 @@ class NodeScopeResolver /** @var array */ private array $earlyTerminatingMethodNames; + /** @var array */ + private array $alwaysTerminatingCallables = []; + /** @var array */ private array $calledMethodStack = []; @@ -660,6 +663,15 @@ public function processStmtNode( array_merge($statementResult->getImpurePoints(), $functionImpurePoints), $functionReflection, ), $functionScope, $storage); + + if ( + count($gatheredReturnStatements) === 0 + && count($gatheredYieldStatements) === 0 + && $this->isBodyAlwaysTerminating($executionEnds) + ) { + $this->alwaysTerminatingCallables[strtolower($functionReflection->getName())] = true; + } + if (!$scope->isInAnonymousFunction()) { $this->processPendingFibers($storage); } @@ -825,6 +837,14 @@ public function processStmtNode( $methodReflection, ), $methodScope, $storage); + if ( + count($gatheredReturnStatements) === 0 + && count($gatheredYieldStatements) === 0 + && $this->isBodyAlwaysTerminating($executionEnds) + ) { + $this->alwaysTerminatingCallables[strtolower($classReflection->getName() . '::' . $methodReflection->getName())] = true; + } + if ($isConstructor) { $finalScope = null; @@ -2454,12 +2474,23 @@ private function findEarlyTerminatingExpr(Expr $expr, Scope $scope): ?Expr } } } + + if (!$expr->isFirstClassCallable() && $this->isMethodCallAlwaysTerminating($expr, $scope)) { + return $expr; + } } if ($expr instanceof FuncCall && $expr->name instanceof Name) { if (in_array((string) $expr->name, $this->earlyTerminatingFunctionCalls, true)) { return $expr; } + + if (!$expr->isFirstClassCallable()) { + $resolvedFunctionName = $this->reflectionProvider->resolveFunctionName($expr->name, $scope); + if ($resolvedFunctionName !== null && isset($this->alwaysTerminatingCallables[strtolower($resolvedFunctionName)])) { + return $expr; + } + } } if ($expr instanceof Expr\Exit_ || $expr instanceof Expr\Throw_) { @@ -2474,6 +2505,50 @@ private function findEarlyTerminatingExpr(Expr $expr, Scope $scope): ?Expr return null; } + private function isMethodCallAlwaysTerminating(MethodCall|Expr\StaticCall $expr, Scope $scope): bool + { + if (!$expr->name instanceof Node\Identifier) { + return false; + } + + if ($expr instanceof MethodCall) { + $methodCalledOnType = $scope->getType($expr->var); + } else { + if ($expr->class instanceof Name) { + $methodCalledOnType = $scope->resolveTypeByName($expr->class); + } else { + $methodCalledOnType = $scope->getType($expr->class); + } + } + + foreach ($methodCalledOnType->getObjectClassNames() as $referencedClass) { + $key = strtolower($referencedClass . '::' . $expr->name->toLowerString()); + if (isset($this->alwaysTerminatingCallables[$key])) { + return true; + } + } + + return false; + } + + /** + * @param ExecutionEndNode[] $executionEnds + */ + private function isBodyAlwaysTerminating(array $executionEnds): bool + { + if ($executionEnds === []) { + return false; + } + + foreach ($executionEnds as $executionEnd) { + if (!$executionEnd->getStatementResult()->isAlwaysTerminating()) { + return false; + } + } + + return true; + } + /** * @param callable(Node $node, Scope $scope): void $nodeCallback */ diff --git a/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php index 97b3d705baf..4d0ec5a08c4 100644 --- a/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php @@ -29,6 +29,11 @@ protected function getRule(): Rule ); } + public function testBug5865(): void + { + $this->analyse([__DIR__ . '/data/bug-5865.php'], []); + } + public function testBug6189(): void { $this->analyse([__DIR__ . '/data/bug-6189.php'], []); diff --git a/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php index 31f3abbf5e5..0cd79d5c576 100644 --- a/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php @@ -60,6 +60,11 @@ public function testBug10054(): void $this->analyse([__DIR__ . '/data/bug-10054.php'], []); } + public function testBug5865(): void + { + $this->analyse([__DIR__ . '/data/bug-5865-while.php'], []); + } + public function testBug6189(): void { $this->analyse([__DIR__ . '/data/bug-6189.php'], [ diff --git a/tests/PHPStan/Rules/Comparison/data/bug-5865-while.php b/tests/PHPStan/Rules/Comparison/data/bug-5865-while.php new file mode 100644 index 00000000000..f77966a7bf8 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-5865-while.php @@ -0,0 +1,44 @@ +alwaysThrows(); + } + } + + public static function staticAlwaysThrows(): void + { + throw new \Exception(); + } + + public function whileStaticCall(): void + { + while (true) { + self::staticAlwaysThrows(); + } + } + +} + +function alwaysThrowsFunction(): void +{ + throw new \Exception(); +} + +function whileFunctionCall(): void +{ + while (true) { + alwaysThrowsFunction(); + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-5865.php b/tests/PHPStan/Rules/Comparison/data/bug-5865.php new file mode 100644 index 00000000000..533bdb29b7a --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-5865.php @@ -0,0 +1,65 @@ +alwaysThrows(); + } while (true); // should not report - body always terminates + } + + public function doWhileVariableCondition(): void + { + $a = true; + do { + $this->alwaysThrows(); + $a = false; + } while ($a); // should not report - body always terminates + } + + public function neverReturnType(): never + { + throw new \Exception(); + } + + public function doWhileNeverReturnType(): void + { + do { + $this->neverReturnType(); + } while (true); // already works - no error + } + + public static function staticAlwaysThrows(): void + { + throw new \Exception(); + } + + public function doWhileStaticCall(): void + { + do { + self::staticAlwaysThrows(); + } while (true); // should not report - body always terminates + } + +} + +function alwaysThrowsFunction(): void +{ + throw new \Exception(); +} + +function doWhileFunctionCall(): void +{ + do { + alwaysThrowsFunction(); + } while (true); // should not report - body always terminates +}