Psalm: Type is lost after passing a reference to a lambda

[zerkms]

It might be a regression - it did not behave like that before, or just psalm became smarter.

<?php

function wantString(string $s): void {};

$str = 'foo';

wantString($str);

static function() use(&$str): void {};

wantString($str);

https://psalm.dev/r/ee5576fa42

Psalm output (using commit 4052e6d):

INFO: UnusedParam - 3:28 - Param $s is never referenced in this method

INFO: MixedArgument - 11:12 - Argument 1 of wantString cannot be mixed, expecting string

I agree that especially when an anonymous function is passed as a callback it's impossible to tell _when_ it will be invoked, but I don't think it should reset to mixed unconditionally.

[weirdan]

What would you suggest though? Delay type invalidation to where the closure leaves current scope?

[zerkms]

I suggest to apply the same inspections as if the anonymous function's body was in the current function scope.

So

<?php

function wantString(string $s): void {};

$str = 'foo';

wantString($str);

static function() use(&$str): void {};

wantString($str);

should behave as

<?php

function wantString(string $s): void {};

$str = 'foo';

wantString($str);

wantString($str);

And

<?php

function wantString(string $s): void {};

$str = 'foo';

wantString($str);

static function() use(&$str): void {
    $str = 42;
};

wantString($str);

should behave as if it was

<?php

function wantString(string $s): void {};

$str = 'foo';

wantString($str);

$str = 42;

wantString($str);

[muglug]

That last example doesn’t behave that way: https://3v4l.org/3aFL4

Basically there's no good way to deal with this pattern.

[zerkms]

That last example doesn’t behave that way

I thought it's obvious from the original post that I oversimplified it to demonstrate the problem.

https://3v4l.org/J1GeF

Basically there's no good way to deal with this pattern.

I just realised that I suggested above not what I think.

"I suggest to apply the same inspections as if the anonymous function's body was in the current function scope." --- this should be instead read as:

"The same inspections should be applied and the finally inferred type should be unionised".

So in the https://3v4l.org/J1GeF the final type should be string | (int): where string is the type before an anonymous function, and (int) is the type inferred from the anonymous function body.

I'm not sure if it's possible for psalm to do that, but logically it's the best way to treat it (and it's not wrong).

[muglug]

Yes, I just saw that PHPStan handles this more gracefully: https://phpstan.org/r/0acecf3a-8477-4aec-898c-35bf1a6f5770

Psalm should do the same - it will require analysing the closure body at least twice, but that's fine by me.

[muglug]

After playing a little more, PHPStan's behaviour is unsound (I've ticketed it there).

It gets confused by

$str = 'foo';

$a = static function() use(&$str): void {
  if (is_object($str)) {
      echo 'object';
  }
};

$str = new stdClass();

$a();

which leads me to realise that there is no sound solution that preserves type information, sadly.

[muglug]

Because it would union on a mixed type, which is most often meaningless.

By-reference vars are a pain, and complicate analysis no end.