Fix type inference in assignment with side-effects

[jlherren]

The type of the assigned expression needs to be evaluated in the old scope, not
the new scope.

Fixes phpstan/phpstan#3875

[ondrejmirtes]

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 NodeScopeResolverTest::testFileAsserts. The tested file should look like this:

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 NodeScopeResolverTest::testFileAsserts data providers.

[ondrejmirtes]

The assert will probably be assertType('asd', $current);

[ondrejmirtes]

I'm really excited by your contributions so far, if you'd like to do more, here are two large lists:

• https://github.com/phpstan/phpstan/milestone/8
• https://github.com/phpstan/phpstan/milestone/17

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 :)

[ondrejmirtes]

It's also possible that this PR fixes other issues related to array_shift: https://github.com/phpstan/phpstan/issues?q=is%3Aissue+is%3Aopen+array_shift

Could you add test cases for those example that are fixed? You can test each issue in a separate commit (and add closes #123 in the commit messages of each one).

[jlherren]

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 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 :)

[ondrejmirtes]

That's awesome! Thank you very much :) For what it's worth, at least the tests keep the 500+ line methods together :)

[ondrejmirtes]

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.

[ondrejmirtes]

Hi, this is awesome, thank you :) If you have more regression tests for already fixed bugs, you can send them in a new PR :)

[jlherren]

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.

[ondrejmirtes]

Yeah, there could be a rule for that. The core is to have the rule for While_ node and ask whether $scope->getType($node->cond) is new ConstantBoolean(false). Feel free to implement it. No need to open an issue, I have it somewhere deep down in my personal todolist :)