diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index ca1daa6f340..aa7f60d52da 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1830,11 +1830,13 @@ public function processStmtNode( } else { $finallyScope = null; } + $tryBodyThrowExitPoints = []; foreach ($branchScopeResult->getExitPoints() as $exitPoint) { - $finallyExitPoints[] = $exitPoint->toPublic(); if ($exitPoint->getStatement() instanceof Node\Stmt\Expression && $exitPoint->getStatement()->expr instanceof Expr\Throw_) { + $tryBodyThrowExitPoints[] = $exitPoint->toPublic(); continue; } + $finallyExitPoints[] = $exitPoint->toPublic(); if ($finallyScope !== null) { $finallyScope = $finallyScope->mergeWith($exitPoint->getScope()); } @@ -2009,6 +2011,27 @@ public function processStmtNode( } } + // Add try body throw exit points to finallyExitPoints only if they weren't caught + foreach ($tryBodyThrowExitPoints as $throwExitPoint) { + $throwLine = $throwExitPoint->getStatement()->getStartLine(); + $isCaught = true; + foreach ($throwPoints as $throwPoint) { + if (!$throwPoint->isExplicit()) { + continue; + } + if ($throwPoint->getNode()->getStartLine() !== $throwLine) { + continue; + } + $isCaught = false; + break; + } + if ($isCaught) { + continue; + } + + $finallyExitPoints[] = $throwExitPoint; + } + if ($finalScope === null) { $finalScope = $scope; } diff --git a/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php b/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php index 82437035fec..9b346ed1f78 100644 --- a/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php @@ -19,10 +19,6 @@ protected function getRule(): Rule public function testRule(): void { $this->analyse([__DIR__ . '/data/overwritten-exit-point.php'], [ - [ - 'This throw is overwritten by a different one in the finally block below.', - 8, - ], [ 'This return is overwritten by a different one in the finally block below.', 11, @@ -58,13 +54,14 @@ public function testBug7665(): void $this->analyse([__DIR__ . '/data/bug-7665.php'], []); } + public function testBug6119(): void + { + $this->analyse([__DIR__ . '/data/bug-6119.php'], []); + } + public function testBug5627(): void { $this->analyse([__DIR__ . '/data/bug-5627.php'], [ - [ - 'This throw is overwritten by a different one in the finally block below.', - 10, - ], [ 'This throw is overwritten by a different one in the finally block below.', 12, @@ -105,10 +102,6 @@ public function testBug5627(): void 'The overwriting return is on this line.', 51, ], - [ - 'This throw is overwritten by a different one in the finally block below.', - 62, - ], [ 'This throw is overwritten by a different one in the finally block below.', 64, @@ -149,10 +142,6 @@ public function testBug5627(): void 'The overwriting return is on this line.', 103, ], - [ - 'This throw is overwritten by a different one in the finally block below.', - 122, - ], [ 'This throw is overwritten by a different one in the finally block below.', 124, diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-6119.php b/tests/PHPStan/Rules/Exceptions/data/bug-6119.php new file mode 100644 index 00000000000..720ec4aec2f --- /dev/null +++ b/tests/PHPStan/Rules/Exceptions/data/bug-6119.php @@ -0,0 +1,45 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug6119; + +class Locker { + + public function __construct(private bool $locked = false) { + } + + public function acquireLock(object $obj): bool { + if(rand(0,10) > 5) { + return $this->locked = true; + } + return $this->locked = false; + } + + public function isLocked(): bool { + return $this->locked; + } +} + +class HelloWorld +{ + + public function __construct(private Locker $locker) { + } + + public function doStuff(object $obj): string + { + try { + + // code + if(!$this->locker->acquireLock($obj)) { + throw new \RuntimeException('Lock not acquired'); + } + // other stuff + } catch(\Throwable $e) { + // do some stuff to reset + } finally { + return 'OK'; + } + } +}