[Templating] assertig that a string is non-empty-string using a generic validator results in a *false* redundant condition error

[azjezz]

example: https://psalm.dev/r/ae244c6b02

If we replace Type<non-empty-string> with a concrete class, we get no issues: https://psalm.dev/r/515024a452

investigating further, this seems to be related to covariant templating: https://psalm.dev/r/b0278b0493

whatever psalm changed in 4.10, completely broke PSL Type component, we now have the choice to either go back to using @template and not be able to create a union type, and any other array type, or keep as is and remove matches validator ( both options are BC breaking, currently down stream users are getting a type error either in the constructor ( for PSL < 1.8 ), or when using matches ( PSL >= 1.8, which uses covariant )

ref: azjezz/psl#227
ref: azjezz/psl#231
ref: azjezz/psl#212
ref: azjezz/psl#214

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/ae244c6b02

<?php

/**
 * @template-covariant T
 */
interface Type {
    /**
     * @psalm-assert-if-true T $value
     */
    public function matches(mixed $value): bool;
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string_type(): Type { exit(0); }

$f = 'hello';

assert(non_empty_string_type()->matches($f));

Psalm output (using commit 2e7e343):

ERROR: RedundantCondition - 20:1 - Type "hello" for $f is always string

https://psalm.dev/r/515024a452

<?php

interface Type {
    /**
     * @psalm-assert-if-true non-empty-string $value
     */
    public function matches(mixed $value): bool;
}

/**
 * @return Type
 */
function non_empty_string_type(): Type { exit(0); }

$f = 'hello';

assert(non_empty_string_type()->matches($f));

Psalm output (using commit 2e7e343):

No issues!

https://psalm.dev/r/b0278b0493

<?php

/**
 * @template T
 */
interface Type {
    /**
     * @psalm-assert-if-true T $value
     */
    public function matches(mixed $value): bool;
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string_type(): Type { exit(0); }

$f = 'hello';

assert(non_empty_string_type()->matches($f));

Psalm output (using commit 2e7e343):

ERROR: RedundantCondition - 20:1 - Type "hello" for $f is always string

[bendavies]

Nice investigation. Thanks!

[orklah]

I went back to january but this snippet still produced the same error with Psalm's code.

Are you sure this is related with Psalm 4.10 specifically?

[azjezz]

@orklah not sure which psalm version exactly broke this, but in February when matches() was introduced, and we where using @template, psalm didn't result in any errors: https://github.com/azjezz/psl/tree/feature/type/vec%2Bdict ( i restored the branch if you would like to test it )

[orklah]

Not sure what I should look at, there's a lot of Psalm errors in the CI for this branch.
However, if you say this was working in February, could you create a snippet that would fail in 4.10 and that would pass in 4.4.1 (release from January)?

[azjezz]

This works in 4.4.1, and fails in 4.10: https://psalm.dev/r/5aee8f5518

to fix this in 4.10, we have to replace @template with @template-covariant, however, that's when ->matches($x) starts to fail.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/5aee8f5518

<?php

/**
 * @template T
 */
interface Type {
    /**
     * @psalm-assert-if-true T $value
     */
    public function matches(mixed $value): bool;
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string_type(): Type { exit(0); }

/**
 * @return Type<int>
 */
function int_type(): Type { exit(0); }

/**
 * @template T
 * @param list<Type<T>> $_types
 * @return Type<list<T>>
 */
function list_type(array $_types): Type { exit(0); }

$a = non_empty_string_type();
$b = int_type();
$c = list_type([$a, $b]);
$_d = list_type([$a, $b, $c]);

Psalm output (using commit 2e7e343):

ERROR: InvalidArgument - 32:6 - Incompatible types found for T

ERROR: InvalidArgument - 33:7 - Incompatible types found for T

[orklah]

Oh, so this is related to the 'Incompatible types found for" error.

For reference, here are similar issues:
#6277
#6288
#6212

and it seems it was introduced by Matt refactoring some code here: acc7ee2

He made some fix for one case here: a205a23

and there was some explanation here: #6073 (comment)

It's well above my current knowledge of Psalm though...

[azjezz]

I think the covariant issue is not a problem, however, i also don't think matches should fail in this case, i will try to give it a try and fix it :)

[azjezz]

just spent 2 hours trying to fix this issue, and come up with nothing :)

[azjezz]

not sure why psalm thinks we are matching $value for string when doing Type\non_empty_string()->matches($value), even tho Type\non_empty_string() returns Type\TypeInterface<non-empty-string>, if Type\non_empty_string() return type is declared as Psl\Type\Internal\NonEmptyStringType, it works, this is because NonEmptyStringType::matches has @psalm-assert-if-true non-empty-string $value, however, TypeInterface::matches has @psalm-assert-if-true T $value, when replacing T, for some unknown reason, psalm translates it as @psalm-assert-if-true string $value, even tho T is not a string, but a non-empty-string.

[azjezz]

If it helps, here's the script i'm using for testing/debugging:

<?php

/**
 * @template-covariant T
 */
interface Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert-if-true T $value
     */
    public function matches($value): bool;
}

/**
 * @template-implements Type<non-empty-string>
 */
final class NonEmptyStringType implements Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert-if-true non-empty-string $value
     */
    public function matches($value): bool
    {
        return is_string($value) && $value !== '';
    }
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string(): Type {
    return new NonEmptyStringType();
}

function bar(): string { return 'hello'; }

function main(): void
{
    $value = bar();

    $type = non_empty_string();

    if ($type->matches($value)) {
        echo 'foo';
    } else {
        echo 'bar';
    }
}

this shouldn't fail.

[azjezz]

and something that is even more weird, is this: https://psalm.dev/r/cb32f3e4cc

here psalm asserts that it is a non-empty-string, not that it is a string, and no redundant condition error is trigger ( for asserting that a string, is already a string )

---

unrelated, psalm doesn't verify assertation functions: https://psalm.dev/r/9d9005ffee

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/cb32f3e4cc

<?php

/**
 * @template-covariant T
 */
interface Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert T $value
     */
    public function matches($value): bool;
}

/**
 * @template-implements Type<non-empty-string>
 */
final class NonEmptyStringType implements Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert non-empty-string $value
     */
    public function matches($value): bool
    {
        return is_string($value) && $value !== '';
    }
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string(): Type {
    return new NonEmptyStringType();
}

function bar(): string { return 'hello'; }

function main(): void
{
    $value = bar();

    $type = non_empty_string();
    $type->matches($value);
    
    if ($value === '') {
        echo 'foo';
    } else {
        echo 'bar';
    }
}

Psalm output (using commit e53827e):

ERROR: DocblockTypeContradiction - 46:9 - "" does not contain non-empty-string

https://psalm.dev/r/9d9005ffee

<?php

/**
 * @param mixed $value
 * @psalm-assert non-empty-string $value
 */
function assertNonEmpty($value): bool {  return false; }

Psalm output (using commit e53827e):

INFO: UnusedParam - 7:25 - Param $value is never referenced in this method

[orklah]

In https://psalm.dev/r/cb32f3e4cc I think you should have typed @psalm-assert-if-true instead of @psalm-assert

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/cb32f3e4cc

<?php

/**
 * @template-covariant T
 */
interface Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert T $value
     */
    public function matches($value): bool;
}

/**
 * @template-implements Type<non-empty-string>
 */
final class NonEmptyStringType implements Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert non-empty-string $value
     */
    public function matches($value): bool
    {
        return is_string($value) && $value !== '';
    }
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string(): Type {
    return new NonEmptyStringType();
}

function bar(): string { return 'hello'; }

function main(): void
{
    $value = bar();

    $type = non_empty_string();
    $type->matches($value);
    
    if ($value === '') {
        echo 'foo';
    } else {
        echo 'bar';
    }
}

Psalm output (using commit 71798b4):

ERROR: DocblockTypeContradiction - 46:9 - "" does not contain non-empty-string

[azjezz]

In https://psalm.dev/r/cb32f3e4cc I think you should have typed @psalm-assert-if-true instead of @psalm-assert

no, i used assert on purpose, just to show that the issue only happens with assert-if-*, but not with assert.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/cb32f3e4cc

<?php

/**
 * @template-covariant T
 */
interface Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert T $value
     */
    public function matches($value): bool;
}

/**
 * @template-implements Type<non-empty-string>
 */
final class NonEmptyStringType implements Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert non-empty-string $value
     */
    public function matches($value): bool
    {
        return is_string($value) && $value !== '';
    }
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string(): Type {
    return new NonEmptyStringType();
}

function bar(): string { return 'hello'; }

function main(): void
{
    $value = bar();

    $type = non_empty_string();
    $type->matches($value);
    
    if ($value === '') {
        echo 'foo';
    } else {
        echo 'bar';
    }
}

Psalm output (using commit f35df42):

ERROR: DocblockTypeContradiction - 46:9 - "" does not contain non-empty-string

[orklah]

Can you check that one again @azjezz ? I think underlying issues have been solved here

[azjezz]

Yes 🎉 my test case ( https://psalm.dev/r/507963421d ) is now passing, thank you!

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/507963421d

<?php

/**
 * @template-covariant T
 */
interface Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert-if-true T $value
     */
    public function matches($value): bool;
}

/**
 * @template-implements Type<non-empty-string>
 */
final class NonEmptyStringType implements Type {
    /**
     * @param mixed $value
     *
     * @psalm-assert-if-true non-empty-string $value
     */
    public function matches($value): bool
    {
        return is_string($value) && $value !== '';
    }
}

/**
 * @return Type<non-empty-string>
 */
function non_empty_string(): Type {
    return new NonEmptyStringType();
}

function bar(): string { return 'hello'; }

function main(): void
{
    $value = bar();

    $type = non_empty_string();

    if ($type->matches($value)) {
        echo 'foo';
    } else {
        echo 'bar';
    }
}

Psalm output (using commit 2e01e9b):

No issues!