Skip to content

Commit 2d7d93d

Browse files
ondrejmirtesphpstan-bot
authored andcommitted
Fix false positive for return overwritten by finally when return is in catch
- Only emit FinallyExitPointsNode when the finally block always terminates, not just when it has any exit points - A return inside a catch within finally doesn't always execute, so it shouldn't be reported as unconditionally overwriting the try-catch return - New regression test in tests/PHPStan/Rules/Exceptions/data/bug-6670.php Closes phpstan/phpstan#6670
1 parent d32efcb commit 2d7d93d

File tree

3 files changed

+30
-1
lines changed

3 files changed

+30
-1
lines changed

src/Analyser/NodeScopeResolver.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2028,7 +2028,7 @@ public function processStmtNode(
20282028
$impurePoints = array_merge($impurePoints, $finallyResult->getImpurePoints());
20292029
$finallyScope = $finallyResult->getScope();
20302030
$finalScope = $finallyResult->isAlwaysTerminating() ? $finalScope : $finalScope->processFinallyScope($finallyScope, $originalFinallyScope);
2031-
if (count($finallyResult->getExitPoints()) > 0) {
2031+
if (count($finallyResult->getExitPoints()) > 0 && $finallyResult->isAlwaysTerminating()) {
20322032
$this->callNodeCallback($nodeCallback, new FinallyExitPointsNode(
20332033
$finallyResult->toPublic()->getExitPoints(),
20342034
$finallyExitPoints,

tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public function testRule(): void
3838
]);
3939
}
4040

41+
public function testBug6670(): void
42+
{
43+
$this->analyse([__DIR__ . '/data/bug-6670.php'], []);
44+
}
45+
4146
public function testBug5627(): void
4247
{
4348
$this->analyse([__DIR__ . '/data/bug-5627.php'], [
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug6670;
4+
5+
class HelloWorld
6+
{
7+
public function sayHello(): string
8+
{
9+
try {
10+
return 'string';
11+
} finally {
12+
try {
13+
$this->clearCache();
14+
} catch(\Exception $e) {
15+
return 'same string';
16+
}
17+
}
18+
}
19+
20+
private function clearCache(): void
21+
{
22+
// do...
23+
}
24+
}

0 commit comments

Comments
 (0)