Skip to content
Closed
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
75 changes: 75 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ class NodeScopeResolver
/** @var array<string, true> */
private array $earlyTerminatingMethodNames;

/** @var array<string, true> */
private array $alwaysTerminatingCallables = [];

/** @var array<string, true> */
private array $calledMethodStack = [];

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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_) {
Expand All @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'], []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'], [
Expand Down
44 changes: 44 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-5865-while.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

namespace Bug5865While;

final class HelloWorld
{

public function alwaysThrows(): void
{
throw new \Exception();
}

public function whileLiteralTrue(): void
{
while (true) {
$this->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();
}
}
65 changes: 65 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-5865.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

namespace Bug5865;

final class HelloWorld
{

public function alwaysThrows(): void
{
throw new \Exception();
}

public function doWhileLiteralTrue(): void
{
do {
$this->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
}
Loading