Fix phpstan/phpstan#14328: Unreachable statement information gets lost#5253
Open
phpstan-bot wants to merge 6 commits intophpstan:2.1.xfrom
Open
Fix phpstan/phpstan#14328: Unreachable statement information gets lost#5253phpstan-bot wants to merge 6 commits intophpstan:2.1.xfrom
phpstan-bot wants to merge 6 commits intophpstan:2.1.xfrom
Conversation
- MethodCallHandler and StaticCallHandler overwrote $isAlwaysTerminating instead of OR-ing with the existing value when checking the return type - This caused never-returning arguments in chained calls to not propagate the always-terminating status to the outer call - Added regression test for phpstan/phpstan#14328 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Use the syntax (new Foo()) to solve lint issues.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Already retrieved this output earlier — all 11609 tests passed. Everything is done and pushed. |
VincentLanglet
approved these changes
Mar 19, 2026
Collaborator
Author
|
The review from VincentLanglet is an APPROVED state with no new change requests. The previous comment about wrapping There are no outstanding changes to make — the PR is approved and ready to merge. |
staabm
approved these changes
Mar 19, 2026
staabm
reviewed
Mar 19, 2026
Contributor
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Please add a similar test for static method
…ning arg Addresses review feedback from staabm: the StaticCallHandler fix was missing a test. Restructured the test data into separate functions so each scenario (method call chain vs static call) is independently tested. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Already retrieved — all 11609 tests passed. Everything is done and pushed. |
Add a test case where a static call with a never-returning argument is chained to a method call, testing the interaction between StaticCallHandler and MethodCallHandler isAlwaysTerminating propagation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous tests covered static calls chained with instance method calls (->), but not static calls chained with other static calls (::). This new test exercises the StaticCallHandler fix where the class expression is itself a static call with a never-returning argument. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
All done. All four background tasks (two test suite runs and two PHPStan analyses) completed successfully, and the commit is pushed. |
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
3 participants
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
never-returning function call was used as an argument in a chained method call (e.g.new Foo()->returnThis($callback())->returnThis('x')), PHPStan failed to report the next statement as unreachable.Changes
src/Analyser/ExprHandler/MethodCallHandler.php:139— changed$isAlwaysTerminating = ...to$isAlwaysTerminating = $isAlwaysTerminating || (...)to preserve the terminating status from the var expressionsrc/Analyser/ExprHandler/StaticCallHandler.php:195— same fix for static method callstests/PHPStan/Rules/DeadCode/data/bug-14328.phpRoot cause
In
MethodCallHandlerandStaticCallHandler, when checking whether the method's return type isnever, the code overwrote$isAlwaysTerminatinginstead of OR-ing it with the existing value. This meant that if a previous part of the expression (such as anever-returning argument in an inner chained call) had already set$isAlwaysTerminating = true, it was reset tofalseby the outer method call's return type check.FuncCallHandleralready used the correct||pattern at the equivalent line.Test
Added
tests/PHPStan/Rules/DeadCode/data/bug-14328.phpwhich reproduces the exact scenario from the issue: anever-returning callback passed as argument in a chained method call, with the next line expected to be reported as unreachable.Fixes phpstan/phpstan#14328