Fix type inference in assignment with side-effects#333
Fix type inference in assignment with side-effects#333ondrejmirtes merged 4 commits intophpstan:masterfrom
Conversation
The type of the assigned expression needs to be evaluated in the old scope, not the new scope.
|
Hi, I really like what you're doing here! Just interested - how much time did it take you to find your way around internals that you're able to fix bugs so effortlessly? :) I need one thing here - get the code from the original issue and test it as a new data provider for function foo(): void
{
$queue = ['asd'];
$list = [];
do {
$current = array_pop($queue);
assertType('string|null', $current);
if ($current === null) {
break;
}
$list[] = $current;
} while ($queue);If in doubt, look at the current |
|
The assert will probably be |
|
I'm really excited by your contributions so far, if you'd like to do more, here are two large lists: If some bug turns out not so easy to fix, let me know. It shouldn't take more than 30-50 lines to fix, otherwise it's not easy :) |
|
It's also possible that this PR fixes other issues related to Could you add test cases for those example that are fixed? You can test each issue in a separate commit (and add |
:) I wouldn't say effortlessly, it still took me a bit to understand the basics. But the code seems quite well structured, so it is not that hard to get around -- so good job there! The only things that make it slightly more difficult is the lack of comments to classes and interfaces. And the occasional 500+ lines methods. I make heavy use of xdebug so that helps a lot. I will hopefully get to add the tests and look at other potentially fixed issues in the evening. And I have another fix that is almost ready as well :) |
|
That's awesome! Thank you very much :) For what it's worth, at least the tests keep the 500+ line methods together :) |
|
FYI I'm sorry about the current master build failures, you can disregard them and just focus on the tests that you've written yourself :) I'm gonna try to fix them ASAP. |
|
Hi, this is awesome, thank you :) If you have more regression tests for already fixed bugs, you can send them in a new PR :) |
|
You're welcome! :) It just occurred to me: Shouldn't PHPStan report that the do-while-condition is always false? I can open a separate issue for that. |
|
Yeah, there could be a rule for that. The core is to have the rule for |
The type of the assigned expression needs to be evaluated in the old scope, not
the new scope.
Fixes phpstan/phpstan#3875