Fix #9601: Failed Determine type in match operator#5066
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Fix #9601: Failed Determine type in match operator#5066phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
- Set isAlwaysTerminating = true for Expr\Throw_ (was incorrectly inheriting from inner expression, unlike Exit_ which was already correct) - Skip always-terminating match arm body scopes when merging post-match scope, so throwing arms don't dilute narrowed types - New regression test in tests/PHPStan/Analyser/nsrt/bug-9601.php - Updated ScopeFunctionCallStack test expectations: code after a statement containing throw-as-argument is now correctly unreachable Closes phpstan/phpstan#9601
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
After a
match(true)expression where all non-throwing arms useinstanceofchecks and thedefaultarm throws, PHPStan failed to narrow the matched variable's type. The variable remainedmixedinstead of being narrowed to the union of matched types.Changes
src/Analyser/NodeScopeResolver.php: FixedExpr\Throw_handling to set$isAlwaysTerminating = true(was incorrectly inheriting from the inner expression result;Exit_already had this correct)src/Analyser/NodeScopeResolver.php: AddedisAlwaysTerminating()checks before adding match arm body scopes to the merge list, in all three code paths: default arms, non-default arms (general path), and non-default arms (enum fast path)tests/PHPStan/Analyser/nsrt/bug-9601.php: New regression testtests/PHPStan/Rules/ScopeFunctionCallStackRuleTest.php: Updated expectations — code after a statement withthrowas a function argument is now correctly identified as unreachabletests/PHPStan/Rules/ScopeFunctionCallStackWithParametersRuleTest.php: Same updateRoot cause
Two issues combined to cause this bug:
Expr\Throw_inprocessExprNode()set$isAlwaysTerminating = $result->isAlwaysTerminating()(inheriting from the inner expression likenew LogicException()), which returnedfalse. This should have beentrue, matching the behavior ofExit_.Match expression processing unconditionally added all arm body scopes to
$armBodyScopesfor the post-match scope merge. When the default arm throws, its scope (containing the un-narrowed variable type) was merged with the narrowed scopes from other arms, dilutingHelloWorld|HelloWorld2back tomixed. The fix follows the same pattern used by the ternary operator (lines 4133-4142) and if/else branches (lines 1168/1202), which already excluded always-terminating branches from scope merges.Test
The regression test creates a
match(true)with twoinstanceofconditions and a throwing default arm, then asserts the variable is narrowed to the union of the matched types after the match expression.Fixes phpstan/phpstan#9601