phpstan / phpstan

PHP Static Analysis Tool - discover bugs in your code without running it!
https://phpstan.org/
MIT License
12.89k stars 879 forks source link

Modifying a parameter passed by reference in Immediately Invoked Function Expression results in mixed on param out #11824

Open ruudk opened 2 weeks ago

ruudk commented 2 weeks ago

Bug report

PHPStan already handles Immediately Invoked Function Expression quite well.

After https://github.com/phpstan/phpstan-src/pull/1628 it also understands the passed in types.

Even when the parameter is passed by reference, within the function, the type information is correct.

But as soon as you modify the variable that is passed by reference, the outer scope will get mixed.

$context['firstname'] = 'John';

(function(&$context) {
    \PHPStan\Testing\assertType("array{firstname: 'John'}", $context);
    $context['lastname'] = 'Doe';
})($context);

\PHPStan\Testing\assertType("array{firstname: 'John', lastname: 'Doe'}", $context); // mixed

I feel this is very similar to what the ParameterOutTypeExtension try to do. Can we use this to fix this type of problem?

I'd be happy to create the PR if someone can guide me in the right direction.

/cc @staabm

Code snippet that reproduces the problem

https://phpstan.org/r/d9f35dbf-4c9c-411a-9b08-0e3bfb971561

Expected output

See assert in the playground.

ruudk commented 1 week ago

Alternative, where the variable is named differently for clarification: https://phpstan.org/r/a147b3bb-2074-4a21-a0b0-7c75e8d49eb4

ruudk commented 1 week ago

I spent a few hours on trying to solve this myself but I'm stuck.

I believe this type of thing is similar to param out, yet it works differently.

For the ParamOut functionality, it can be done in NodeScopeResolver::processArgs which is called when it is processing a FuncCall expression.

But when working with an immediately invoked function expression, we first have to process the body of the function, get the scope, and then update the outer scope depending on if the parameter was byRef: true.

No idea how to do this....

staabm commented 1 week ago

I can look into it in case its important enough for you to sponsor the time

ruudk commented 1 week ago

Thanks. I'm a small sponsor through TicketSwap. But the issue I'm trying to solve in this issue is for TwigStan.