[NodeAnalyzer] Treat infinite loop without break as always terminated - #8348
Merged
Merged
Conversation
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.
Fixes rectorphp/rector#9852
TerminatedNodeAnalyzer::isAlwaysTerminated()did not recognize an infinite loop (while (true),do {} while (true),for (;;)) with nobreakas a terminating statement. Anything following such a loop is unreachable, but the analyzer reported the loop as fall-through.Consequence downstream:
ConsoleExecuteReturnIntRectorappended a redundantreturn Command::SUCCESS;after awhile (true)body whose exits allreturn, producing an unreachable statement that fails PHPStan at level 7+ (reported in the issue).RemoveUnreachableStatementRector(the core consumer) also failed to strip such dead code.Now an infinite loop with no escaping
break/gotois treated as always terminated:Behavior with
RemoveUnreachableStatementRector:while (true) { if ($a) { return 'A'; } $a = doSomething(); } -echo 'never executed';Conservative on purpose: a loop containing any
break/goto(which may exit it) is left as non-terminating, so no code that could still be reached is dropped. Only literal always-true conditions (true, non-zero int literal, emptyfor(;;)) qualify — dynamic conditions likewhile ($x)are untouched.Fixtures:
always_terminated_infinite_while.php.inc(removed dead code) andskip_while_true_with_break.php.inc(kept, has a break).