Fix phpstan/phpstan#6119: False positive in try/catch/finally when throwing exception inside try block#5340
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Conversation
…rowing inside try block - Throw exit points from try body that are caught by catch blocks are no longer reported as overwritten by finally - Deferred adding try body throw exit points to finallyExitPoints until after catch processing - After catches are processed, only uncaught throws are added to finallyExitPoints - Updated existing test expectations in testRule and testBug5627 to reflect correct behavior - New regression test in tests/PHPStan/Rules/Exceptions/data/bug-6119.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a
throwinside atryblock is caught by acatchblock, PHPStan incorrectly reported it as "overwritten by a different one in the finally block below." The throw is handled by the catch, so it is not an exit point that the finally block overwrites.Changes
src/Analyser/NodeScopeResolver.php: deferred adding try body throw exit points to$finallyExitPointsuntil after catch blocks are processed. Only uncaught throws (those still present in$throwPointsafter catch processing) are added.tests/PHPStan/Rules/Exceptions/OverwrittenExitPointByFinallyRuleTest.phpfortestRuleandtestBug5627to remove caught throw exit points that are no longer reported.tests/PHPStan/Rules/Exceptions/data/bug-6119.php.Root cause
In
NodeScopeResolver.php, try body throw exit points were unconditionally added to$finallyExitPointsbefore catch blocks were processed. This meant that even throws fully caught by catch blocks were reported as overwritten by the finally block. The fix collects try body throw exit points separately, processes catch blocks first, then checks whether each throw point was consumed (caught) during catch processing. Only uncaught throws are added to$finallyExitPoints.Test
Added
tests/PHPStan/Rules/Exceptions/data/bug-6119.phpwhich reproduces the original issue: a throw inside a try block caught bycatch(\Throwable)with afinally { return 'OK'; }. The test expects no errors (empty array), confirming the false positive is eliminated.Fixes phpstan/phpstan#6119