Allow process multiple UnreachableStatementNode on NodeScopeResolver::processNodes()#3745
Conversation
|
I don't want to spam the user with more of these messages, one of them is enough. But I'd accept a PR that adds this information to UnreachableStatementNode as a new getter, something like |
|
I see, I will try |
|
@ondrejmirtes implemented 👍 , I added |
|
@ondrejmirtes I am not sure how unit test error is related: could you help verify? thank you. |
tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php
Outdated
Show resolved
Hide resolved
|
@ondrejmirtes it seems working ok now, Ready to merge 👍 |
ondrejmirtes
left a comment
There was a problem hiding this comment.
You're not actually testing getNextStatements anywhere.
Please create a new RuleTestCase inside tests/ and write an anonymous class in getRule() that would verify this information, for example by putting the next statements into reported errors.
|
I will try |
4ee2951 to
ebc0855
Compare
|
I've added test for |
|
@ondrejmirtes test added, Ready to merge 👍 |
|
@ondrejmirtes mergeable? |
747ca1c to
2d7efbc
Compare
|
@ondrejmirtes Rebased 👍 |
|
I just pushed some test adjustments and a failing test for you. The output for testRuleTopLevel is currently wrong. See: https://phpstan.org/r/7a2aafcd-10b3-405d-9402-90037f780059 - the top level function is not considered unreachable currently by PHPStan. But it's included in your "next statements" which is wrong. |
|
@ondrejmirtes I've fixed handling top level function 2427d86 👍 |
|
Thank you. |
|
Thank you @ondrejmirtes I will test on rector-src 👍 |
|
linked PR on rector-src for future reference, tested the |
/cc @TomasVotruba if this is accepted, this should hopefully will allow us to remove
UnreachableStatementNodeVisitoron rector side.https://github.com/rectorphp/rector-src/blob/e5d221e40d2c5acc1a92b1ded501321ee690be06/src/PHPStan/NodeVisitor/UnreachableStatementNodeVisitor.php#L17
which used if found a first
UnreachableStatementNode, as phpstan only process firstUnreachableStatementNode