Misused Psalm Assertion for isValid() method?

[LikeAJohny]

Do I misunderstand the @psalm-assert-if-true Assertion?

The Uuid::isValid method has a psalm assertion @psalm-assert-if-true non-empty-string $uuid set.
This assertion seems to lead Psalm to think that only non-empty-strings can be given to this method and any false validation means that the given string is empty.
At least that's what I'm guessing after receiving the following error
All possible types for this argument were invalidated - This may be dead code
in a project of mine.

Example code

Here's a simplified version of the problematic code. I've just mocked a Uuid::isValid implementation for demo purposes.

https://psalm.dev/r/764e8e9309

[ramsey]

My understanding is this assertion means that if the return value is true, then Psalm can assume $uuid is a non-empty string.

The following code from your example says, "if $id is not a valid UUID, then throw an exception and pass $id to the exception." However, at this point, Psalm is no longer sure what type $id might be. It could be an empty string or something else.

        if (!Uuid::isValid($id)) {
            throw new \Exception($id);
        }

Does this help?

[janedbal]

It fails even in phpstan: https://phpstan.org/r/48a003c1-7368-43ee-b453-5d6bc888892e

The assertion means that the result is true if you pass non empty string, which is wrong. PHPStan's docs here: https://phpstan.org/writing-php-code/narrowing-types#custom-type-checking-functions-and-methods (PHPStan reads even psalm annotation).

[ramsey]

@gsteel Can you weigh-in on this? You added this assertion in #410

[nocive]

Seeing the same issue, this is causing psalm to issue NoValue errors for my own code when using Uuid::isValid.

[gsteel]

Perhaps I'm missing something about how psalm assertions can be used, in which case it should be reverted. However, an additional @psalm-assert-if-false seems to clear things up for psalm:

https://psalm.dev/r/0167fd28fc

Also, the psalm docs appear to agree with what was done here so I'm not sure why this is misuse.

It appears to me that PHPStan is effectively saying "Why bother calling this method, you already have the correct type" which IMO is making a lot of assumptions about how your code works.

The whole point of asserting the non-empty-string is so that the asserted argument can be treated as non empty without any further massaging…

https://psalm.dev/r/1ecb8eb32b

If it causes more problems than it solves, then perhaps it should be dropped, otherwise, the negated assertion will certainly solve psalm issues (That have only begun to appear since the release of 5 perhaps).

[ondrejmirtes]

This is the solution to the problem: https://psalm.dev/r/d5610909df

But don't ask me why - I don't understand the syntax and the reasoning, I merely borrowed it into PHPStan :)

Here's the explanation: https://psalm.dev/docs/annotating_code/assertion_syntax/#equality-assertions

[gsteel]

Nice! Thanks @ondrejmirtes - Of course this makes PHPStan happy too :D

[ramsey]

I just released 4.7.2 with the fix for this. Thanks, @gsteel!