ExplicitReturnNullRector: skip on goto #6988
Conversation
rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php
Outdated
Show resolved
Hide resolved
| ----- | ||
| <?php | ||
|
|
||
| namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture; | ||
|
|
||
| final class DoWhileMaybeReturned4 | ||
| { | ||
| public function run(int $i) | ||
| { | ||
| do { | ||
| if (rand(0, 1)) { | ||
| goto execute; | ||
| } | ||
|
|
||
| return 2; | ||
| } while (++$i < 1); | ||
| execute: | ||
| echo 'test'; | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| ?> |
There was a problem hiding this comment.
hopefully its fine to skip this case, as I think it is not very useful
There was a problem hiding this comment.
add return null after echo 'test' is valid here, so probably verify if it has label.
It also probably more verifiable via TerminatedNodeAnalyzer service
There was a problem hiding this comment.
I agree it is technically allowed, but I think its not valuable to complicate the rector for goto cases.
I would just leave code with goto alone as it might have suprising logic
There was a problem hiding this comment.
I will check if that can be covered via TerminatedNodeAnalyzer service :), otherwise, I can just merge it later if that too complex to cover
There was a problem hiding this comment.
I created PR with utilize TerminatedNodeAnalyzer at:
There was a problem hiding this comment.
After some try, it indeed to complicated to tweak, I can merge this for now :)
There was a problem hiding this comment.
@staabm I created properly new PR to handle this:
which check on SilentVoidResolver instead.
|
Thank you @staabm |
|
Thx |
|
This pull request has been automatically locked because it has been closed for 150 days. Please open a new PR if you want to continue the work. |
2 participants
closes rectorphp/rector#9224