Mapping closed inheritance to union during assertion

[klimick]

I expected to see this in #9687, but unfortunately it was missed.
It's also closes #5248.

[orklah]

Thanks!

[kkmuffme]

Original works fine: https://psalm.dev/r/91745556ee
However when using === false instead of ! it's still broken: https://psalm.dev/r/42ad2737c6 (same for !== true: https://psalm.dev/r/be43229bf9)

EDIT: the 3rd example from your issue #5248 still doesn't work: https://psalm.dev/r/408248f46f

@klimick would you be so kind to also fix that too?

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/91745556ee

<?php

/**
 * @psalm-assert-if-false string $value
 * @psalm-assert-if-true int $value
 */
function isRight(string|int $value): bool
{
	return is_int($value);
}

function testRight(int $v): void {}
function testLeft(string $v): void {}

/** @var int|string */
$value = 0;

isRight($value)
    ? testRight($value)
    : testLeft($value);

if (isRight($value)) {
    testRight($value);
} else {
    testLeft($value);
}

!isRight($value)
    ? testLeft($value)
    : testRight($value);

if (!isRight($value)) {
    testLeft($value);
} else {
    testRight($value);
}

Psalm output (using commit bebce06):

No issues!

https://psalm.dev/r/42ad2737c6

<?php

/**
 * @psalm-assert-if-false string $value
 * @psalm-assert-if-true int $value
 */
function isRight(string|int $value): bool
{
	return is_int($value);
}

function testRight(int $v): void {}
function testLeft(string $v): void {}

/** @var int|string */
$value = 0;

isRight($value)
    ? testRight($value)
    : testLeft($value);

if (isRight($value)) {
    testRight($value);
} else {
    testLeft($value);
}

!isRight($value)
    ? testLeft($value)
    : testRight($value);

if (isRight($value) === false) {
    testLeft($value);
} else {
    testRight($value);
}

Psalm output (using commit bebce06):

ERROR: InvalidScalarArgument - 33:14 - Argument 1 of testLeft expects string, but int|string provided

ERROR: InvalidScalarArgument - 35:15 - Argument 1 of testRight expects int, but int|string provided

https://psalm.dev/r/be43229bf9

<?php

/**
 * @psalm-assert-if-false string $value
 * @psalm-assert-if-true int $value
 */
function isRight(string|int $value): bool
{
	return is_int($value);
}

function testRight(int $v): void {}
function testLeft(string $v): void {}

/** @var int|string */
$value = 0;

isRight($value)
    ? testRight($value)
    : testLeft($value);

if (isRight($value)) {
    testRight($value);
} else {
    testLeft($value);
}

!isRight($value)
    ? testLeft($value)
    : testRight($value);

if (isRight($value) !== true) {
    testLeft($value);
} else {
    testRight($value);
}

Psalm output (using commit bebce06):

ERROR: InvalidScalarArgument - 33:14 - Argument 1 of testLeft expects string, but int|string provided

ERROR: InvalidScalarArgument - 35:15 - Argument 1 of testRight expects int, but int|string provided

[klimick]

I described the original problem rather badly.
Added a lot of unnecessary examples that poorly describe the essence of the problem.

The problem was that the psalm could not certain that the Either (or any other abstract class/interface) in the if branch is one value and the else branch is another. And this behavior of the psalm was quite safe.

Look at this example:

/**
 * @template L
 * @template R
 */
interface Either {}

/**
 * @template R
 * @implements Either<empty, R>
 */
final class Right implements Either {}

/**
 * @template L
 * @implements Either<L, empty>
 */
final class Left implements Either {}

/**
 * @template L
 * @template R
 *
 * @param Either<L, R> $either
 * @return bool
 *
 * @psalm-assert-if-true Left<L> $either
 */
function isLeft(Either $either): bool
{
     throw new RuntimeException('???');
}

/**
 * @param Either<string, int>
 */
function test(Either $e): void {
    if (isLeft($e)) {
         // exactly Left<string> 
    } else {
        // Either<string, int> here.
        // Because we can extend/implement the Either at any point of the program (even at runtime)
    }
}

In the else branch should be Right<int>. But how achieve it? Needs somehow to tell Psalm that the Either can be extended/implemented by closed list of types. And @psalm-inheritors can do it: https://psalm.dev/r/408248f46f https://psalm.dev/r/d0e5cb2891.

Thanks to @psalm-inheritors psalm can negates complex types same as for simple scalars: https://psalm.dev/r/c848406bc5

Regarding the multiple definition of the psalm-assert-if-true/false:
I don't know why psalm maintainers did it. I think this is bad design.
I always use only one @psalm-assert-if-true annotation. Due to the psalm assertion system it should be enough (I'm referring to the examples above).

@orklah what do you think about multiple assertion annotation?
I can't remember any compiler where it would be.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/408248f46f

<?php

/**
 * @template L
 * @template R
 */
interface Either {}

/**
 * @template R
 * @implements Either<empty, R>
 */
final class Right implements Either {}

/**
 * @template L
 * @implements Either<L, empty>
 */
final class Left implements Either {}

/**
 * @template L
 * @template R
 *
 * @param Either<L, R> $either
 * @return bool
 *
 * @psalm-assert-if-true Left<L> $either
 * @psalm-assert-if-false Right<L> $either
 */
function isLeft(Either $either): bool
{
	throw new RuntimeException('???');
}

/** @var Either<int, string> $either */
$either = null;

if (!isLeft($either)) {
    /** @psalm-trace $left */
	$left = $either;
} else {
	/** @psalm-trace $right */
	$right = $either;
}

Psalm output (using commit bebce06):

INFO: Trace - 41:2 - $left: Either<int, string>&Right<int>

INFO: Trace - 44:2 - $right: Left<int>

https://psalm.dev/r/c848406bc5

<?php

function getIntOrStringOrBool(): int|string|bool
{
    throw new RuntimeException('???');
}

function first(): void
{
    $intOrStringOrBool = getIntOrStringOrBool();

    if (is_int($intOrStringOrBool)) {
        /** @psalm-check-type-exact $intOrStringOrBool = int */
    } else {
        /** @psalm-check-type-exact $intOrStringOrBool = string|bool */
    }
}

function second(): void
{
    $intOrStringOrBool = getIntOrStringOrBool();

    if (is_int($intOrStringOrBool)) {
        /** @psalm-check-type-exact $intOrStringOrBool = int */
    } elseif (is_string($intOrStringOrBool)) {
        /** @psalm-check-type-exact $intOrStringOrBool = string */
    } else {
        /** @psalm-check-type-exact $intOrStringOrBool = bool */
    }
}

function third(): void
{
    $intOrStringOrBool = getIntOrStringOrBool();

    if (is_int($intOrStringOrBool)) {
        /** @psalm-check-type-exact $intOrStringOrBool = int */
    } elseif (is_string($intOrStringOrBool)) {
        /** @psalm-check-type-exact $intOrStringOrBool = string */
    } elseif (is_bool($intOrStringOrBool)) { // of course it's redundant)
        /** @psalm-check-type-exact $intOrStringOrBool = bool */
    }
}

Psalm output (using commit bebce06):

ERROR: RedundantCondition - 40:15 - Type bool for $intOrStringOrBool is always bool

[klimick]

@kkmuffme all you examples works If you leave only one assertion dockblock. Why is that? I don't know)

[kkmuffme]

Ok, then I'll fix it. It's just really confusing bc from what the issue title is and what you state is fixed isn't actually fixed.

Multiple assertions can make sense, e.g. if you have array/list or multiple variables.

[klimick]

Multiple assertions can make sense, e.g. if you have array/list or multiple variables.

For example type array<string, string> | list<string>.
Currently, Psalm can't assert that some times a variable is array<string, string> and some times is list<string>, because types will be collapsed to array<int | string, string>: https://psalm.dev/r/d23a4022d9
For me it's looks like a bug. It's completely unrelated types. If the types would not collapse into one, then one assertion would be enough.

Can you provide your examples? I am curious.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/d23a4022d9

<?php

/**
 * @param array<string, string>|list<string> $_
 */
function test(array $_): void
{
    /** @psalm-trace $_ */;
}

test([23 => '']);

Psalm output (using commit bebce06):

INFO: Trace - 8:27 - $_: array<int<0, max>|string, string>

[kkmuffme]

#9037

Yeah it's a "bug" aka not a feature (there is an issue where muglug explains why, e.g. typescript has that feature since 2-3 years, but it's really complex so it hasn't been added in psalm, as no other php static analysis tools have it either)

I actually have a PR to fix it, but I have to redo it now bc muglug found an internal issue that now breaks my PR. Alas, I will PR it, that should also fix that issue.