Psalm: Type narrowing with `instanceof` behaves differently than with assertion

[zerkms]

https://psalm.dev/r/f28a0ad134

<?php

interface One {
  function callme(string $data): void;
}

interface Two {
  function callme(int $data): void;
}

/**
 * @psalm-assert One $v
 * @param mixed $v
 */
function isOne($v): void {}

/** @var One|Two $value */
$value = null;

if ($value instanceof One) {
    $value->callme('1');
}

if ($value instanceof One) {
    isOne($value);
    $value->callme('1');
}

I think first case should also type check.

[weirdan]

I'd say neither should typecheck, because narrowing does not rule out a possibility of $value being the intersection of One and Two (unless Psalm can figure out they are incompatible and there are no possible intersections, and I don't think it can at the moment).

[zerkms]

@weirdan

being the intersection of One and Two

that was my initial thought. But after thinking a bit more: in the php's type system is it possible to implement a class that would be typed as One & Two (for this particular case)?

Or more generally: is it possible to implement a case when narrowing One | Two (for arbitrary One and Two) to either of those while having One & Two value can lead to problems?

[weirdan]

that was my initial thought. But after thinking a bit more: in the php's type system is it possible to implement a class that would be typed as One & Two (for this particular case)?

https://3v4l.org/doUaY

class C implements One, Two {
  /** @param mixed $_p */
  public function callme($_p): void {}
}

Here C is One & Two (so I was wrong about them being incompatible).

[muglug]

yeah, the assertion should behave as the instanceof

[zerkms]

@muglug so, both of them should fail?

[muglug]

Yes

[muglug]

simplified, this should be prohibited by Psalm, but it currently is not:

interface One {}
interface Two {}

/** @param One|Two $value */
function foo($value) : void {
  if ($value instanceof One) {
      if ($value instanceof One) {}
  }
}

[zerkms]

Yes

I'm not sure I'm following.

if ($value instanceof One) in php's type system _gurantees_ $value implements One's protocol. Why would psalm think otherwise on it?

simplified, this should be prohibited by Psalm, but it currently is not:

and could you clarify that as well? Why nesting instanceof is "wrong"?

[muglug]

if if ($value instanceof One) { is true, checking a second time is redundant. Psalm should warn about that, but the same bug is causing a change in type.

[zerkms]

Sorry for being so chatty, but to be entirely sure: after this bug is addressed should https://psalm.dev/r/5ab344d417 type-check successfully or fail the type-check?

[muglug]

it will (still) fail - it will act like https://psalm.dev/r/eafebfdfdb

[zerkms]

I cannot agree with it:

if ($value instanceof One) {
    $value->callme('1');
}

this must type-check: php's type system guarantees it's safe to make that call.

[muglug]

I cannot agree with it

Yeah, I agree it’s tough. Instead, you can assert !$value instanceof Two and Psalm will be happy.

[zerkms]

But it triggers false-positives on valid code: https://psalm.dev/r/605d15e248 https://3v4l.org/e4tsG

Instead, you can assert !$value instanceof Two and Psalm will be happy.

It would change semantics of code (in general case) and could lead to bugs: now you rely not on a contract, but on a lack of thereof.

[muglug]

But it triggers false-positives on valid code

I've ticketed that (separate) issue here: https://github.com/vimeo/psalm/issues/2237