From f4c4157d5107d7573dc3ecfcbee86ddc3ffe6bcd Mon Sep 17 00:00:00 2001 From: Bellangelo Date: Wed, 13 Nov 2024 05:15:29 +0200 Subject: [PATCH 1/5] New test for bug --- .../OverwrittenExitPointByFinallyRuleTest.php | 5 +++++ .../PHPStan/Rules/Exceptions/data/bug-11906.php | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 tests/PHPStan/Rules/Exceptions/data/bug-11906.php diff --git a/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php b/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php index 1698ada07c..485043afdf 100644 --- a/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php @@ -176,4 +176,9 @@ public function testBug5627(): void ]); } + public function testBug11906(): void + { + $this->analyse([__DIR__ . '/data/bug-11906.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-11906.php b/tests/PHPStan/Rules/Exceptions/data/bug-11906.php new file mode 100644 index 0000000000..19bffb13e2 --- /dev/null +++ b/tests/PHPStan/Rules/Exceptions/data/bug-11906.php @@ -0,0 +1,17 @@ + Date: Wed, 13 Nov 2024 06:00:11 +0200 Subject: [PATCH 2/5] Remove from exit points catched exceptions --- src/Analyser/NodeScopeResolver.php | 7 +++++++ tests/PHPStan/Rules/Exceptions/data/bug-11906.php | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index f2ce6e9c2e..3261ba896f 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1638,6 +1638,13 @@ private function processStmtNode( } else { $catchScope = $catchScope->mergeWith($matchingThrowPoint->getScope()); } + + foreach ($finallyExitPoints as $key => $finallyExitPoint) { + if ($finallyExitPoint->getStatement()->expr === $matchingThrowPoint->getNode()) { + unset($finallyExitPoints[$key]); + break; + } + } } $variableName = null; diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-11906.php b/tests/PHPStan/Rules/Exceptions/data/bug-11906.php index 19bffb13e2..d6bb8a939c 100644 --- a/tests/PHPStan/Rules/Exceptions/data/bug-11906.php +++ b/tests/PHPStan/Rules/Exceptions/data/bug-11906.php @@ -8,7 +8,7 @@ function func(): void { try { throw new LogicException('test'); } catch (LogicException) { - // This catch-block should cause line 7 to not be treated as an exit point + // This catch-block should cause line 9 to not be treated as an exit point } finally { if (getenv('FOO')) { return; From a0df696b5449a331924b68b3ffc540e7d17f8055 Mon Sep 17 00:00:00 2001 From: Bellangelo Date: Wed, 13 Nov 2024 06:03:36 +0200 Subject: [PATCH 3/5] Update pre-existing tests --- .../OverwrittenExitPointByFinallyRuleTest.php | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php b/tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.php index 485043afdf..6fff563d78 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, @@ -41,10 +37,6 @@ public function testRule(): void 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, @@ -53,10 +45,6 @@ public function testBug5627(): void 'The overwriting return is on this line.', 14, ], - [ - 'This exit point is overwritten by a different one in the finally block below.', - 29, - ], [ 'This exit point is overwritten by a different one in the finally block below.', 31, @@ -65,10 +53,6 @@ public function testBug5627(): void 'The overwriting return is on this line.', 33, ], - [ - 'This exit point is overwritten by a different one in the finally block below.', - 39, - ], [ 'This exit point is overwritten by a different one in the finally block below.', 41, @@ -85,10 +69,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, @@ -97,10 +77,6 @@ public function testBug5627(): void 'The overwriting return is on this line.', 66, ], - [ - 'This exit point is overwritten by a different one in the finally block below.', - 81, - ], [ 'This exit point is overwritten by a different one in the finally block below.', 83, @@ -109,10 +85,6 @@ public function testBug5627(): void 'The overwriting return is on this line.', 85, ], - [ - 'This exit point is overwritten by a different one in the finally block below.', - 91, - ], [ 'This exit point is overwritten by a different one in the finally block below.', 93, @@ -129,10 +101,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, From 123f5dc36b8f9e2300cee845eb2b941b5ba3b69b Mon Sep 17 00:00:00 2001 From: Bellangelo Date: Wed, 13 Nov 2024 06:19:30 +0200 Subject: [PATCH 4/5] Fix code style --- tests/PHPStan/Rules/Exceptions/data/bug-11906.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-11906.php b/tests/PHPStan/Rules/Exceptions/data/bug-11906.php index d6bb8a939c..459daf13cf 100644 --- a/tests/PHPStan/Rules/Exceptions/data/bug-11906.php +++ b/tests/PHPStan/Rules/Exceptions/data/bug-11906.php @@ -1,13 +1,11 @@ - Date: Wed, 13 Nov 2024 06:19:45 +0200 Subject: [PATCH 5/5] Validate that an expression exist --- src/Analyser/NodeScopeResolver.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 3261ba896f..0d2b425fb8 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1640,7 +1640,10 @@ private function processStmtNode( } foreach ($finallyExitPoints as $key => $finallyExitPoint) { - if ($finallyExitPoint->getStatement()->expr === $matchingThrowPoint->getNode()) { + if ( + $finallyExitPoint->getStatement() instanceof Node\Stmt\Expression + && $finallyExitPoint->getStatement()->expr === $matchingThrowPoint->getNode() + ) { unset($finallyExitPoints[$key]); break; }