Fix #9349: Throw point are not properly recognized#5068
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Fix #9349: Throw point are not properly recognized#5068phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
- Always include implicit throw points when matching to catch clauses, regardless of whether explicit non-throw-statement matches exist - Previously, when a method with @throws matched a catch clause, implicit throw points from other method calls were excluded from the catch scope - This caused variables assigned between the two calls to be incorrectly reported as "Undefined variable" instead of "might not be defined" - Removed unused $onlyExplicitIsThrow flag and InternalThrowPoint::getNode() - Updated test expectations in bug-4821.php and explicit-throws.php to reflect the corrected (more sound) behavior Closes phpstan/phpstan#9349
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 try block contains a method with
@throws RuntimeExceptionfollowed by variable assignments and another method call that can implicitly throw, the catch block forPDOException(which extendsRuntimeException) incorrectly reported variables as "Undefined" instead of "might not be defined".The root cause was that the try/catch throw point matching algorithm had a flag (
$onlyExplicitIsThrow) that prevented implicit throw points from being matched to a catch clause whenever an explicit non-throw-statement match already existed. This meant that when@throws RuntimeExceptionon a method call matchedcatch(PDOException)(because PDOException extends RuntimeException), implicit throw points from later code — where variables were already assigned — were excluded from the catch scope entirely.Changes
$onlyExplicitIsThrowflag and made implicit throw point matching unconditional insrc/Analyser/NodeScopeResolver.php(lines ~1917-1957)InternalThrowPoint::getNode()method fromsrc/Analyser/InternalThrowPoint.phptests/PHPStan/Analyser/nsrt/bug-4821.php—$methodcertainty changed fromNotoMaybe(correct:$method->invoke()can throw after$methodis assigned)tests/PHPStan/Analyser/nsrt/explicit-throws.php—$acertainty changed fromYestoMaybe(correct: unknown functiondoFoo()can throwInvalidArgumentExceptionbefore$ais assigned)tests/PHPStan/Analyser/nsrt/throw-points/bug-9349.phpRoot cause
The try/catch matching algorithm in
NodeScopeResolverprocesses throw points in three phases:Throwable, match everything@throwsannotations andthrowstatements)@throws)Phase 3 was guarded by a condition: only run if no explicit matches exist, OR all explicit matches are literal
throwstatements ($onlyExplicitIsThrow). When an explicit match came from a method call's@throwsannotation (not athrowstatement), the flag was set tofalse, causing phase 3 to be skipped entirely.This was incorrect because implicit throw points from different method calls (later in the try block) provide scopes where additional variables are defined. Skipping them meant the catch scope only reflected the state at the first matching throw point, missing variables assigned afterwards.
The fix removes this guard, making implicit throw point matching unconditional. This is sound because implicit throw points represent real possibilities — any method call without
@throws voidcan throw anyThrowable.Test
Added
tests/PHPStan/Analyser/nsrt/throw-points/bug-9349.phpwith two test cases:@throws RuntimeException+ implicit throw +catch(PDOException)— verifies$sqlisMaybe(notNo)@throws LogicException+ implicit throw +catch(PDOException)— verifies$sqlisYes(LogicException is unrelated to PDOException, so only implicit throws match)Fixes phpstan/phpstan#9349