-
Notifications
You must be signed in to change notification settings - Fork 459
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
Support for
endless loops
#3573
base: 1.12.x
Are you sure you want to change the base?
Conversation
b1611ba
to
b43e2e3
Compare
b43e2e3
to
824abe6
Compare
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.
Please do not change anything about while
. Change only for
. After that is merged, you can send the while
change as another PR and then we can think about it.
It shouldn't change anything, just merges the elseif/else , so only refactor. But I might be overlooking something and can revert it again 😊 |
adapted. the failing test cases seem to be OK, since in https://github.com/nikic/PHP-Parser/blob/v3.1.5/lib/PhpParser/ParserAbstract.php#L175-L344 there are 2 endless loops and the outer one does not seem to have a break statement. Replacing them with a |
return new StatementResult( | ||
$finalScope, | ||
$finalScopeResult->hasYield() || $hasYield, | ||
false/* $finalScopeResult->isAlwaysTerminating() && $isAlwaysIterable*/, | ||
$isAlwaysTerminating, |
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.
I feel like I'm missing $finalScopeResult->isAlwaysTerminating()
here. It's related to the failing test we talk above.
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.
Also please update the E2E test so it's not failing.
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.
adapted I think. not sure if I did it the right way, but I manually checked-out PHP-Parser and manually run PHPStan like in the test and let it update the baseline there.
Saw this fixes some more issues, will add regression tests.. |
f0bd0f9
to
9d19c22
Compare
realized that I can make this better.. |
Closes phpstan/phpstan#6807
Closes phpstan/phpstan#8463
Closes phpstan/phpstan#9374
analogue to
while
. contains also a slight simplification in the setting of$isAlwaysTerminating
forwhile
loops to keep this more in sync.I had to switch the 2 test blocks in
tests/PHPStan/Rules/Comparison/data/strict-comparison.php
because the first one would be never exiting endless loop after this change, which would make the second one dead code and not testing anything useful any more.