-
-
Notifications
You must be signed in to change notification settings - Fork 372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Php56][Php70] Handle infinite loop on TernaryToNullCoalescingRector+AddDefaultValueForUndefinedVariableRector #3639
Conversation
Fixed 🎉 /cc @nerones |
All checks have passed 🎉 @TomasVotruba I think it is ready. |
@TomasVotruba I am merging it ;) |
return true; | ||
} | ||
|
||
if ($this->isAsCoalesceLeftOrAssignOpCoalesceVar($parentNode, $variable)) { | ||
if ($this->issetOrUnsetOrEmptyParent($parentNode)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samsonasik I think it could make sense to re-order the skipchecking methods so e.g. doing AST traversing only after cheaper checks have been done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch is to ensure no infinite loop by check coalesce early, which check coalesce later cause infinite loop, see https://getrector.com/demo/289e4c81-adae-4062-8a7a-2ad51c53c711
feel free to provide improvement PR 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing job, thanks. I really like this tool I hope that at some point I will able to contribute something. |
…AddDefaultValueForUndefinedVariableRector (#3639) * reproduce hang * Closes rectorphp/rector#7889 * clean up * [ci-review] Rector Rectify --------- Co-authored-by: GitHub Action <actions@github.com>
Closes rectorphp/rector#7889
Ref https://getrector.com/demo/289e4c81-adae-4062-8a7a-2ad51c53c711