Regression: `@return ($iterable is list ? never : list<T>)` no longer works

[MidnightDesign]

The following code didn't produce any errors prior to 6.13.0:

<?php

/**
 * Returns `never` when given a list to make sure it's not called unnecessarily
 *
 * @template T
 * @param iterable<mixed, T> $iterable
 * @return ($iterable is list ? never : list<T>)
 */
function iterableToList(iterable $iterable): array
{
    return array_values(iterator_to_array($iterable));
}

/**
 * @return iterable<int, string>
 */
function returnsIterable(): iterable
{
    yield "foo";
}

$list = iterableToList(returnsIterable());

https://psalm.dev/r/f22bcf3289

In 6.13.0 it errors with

ERROR: NoValue - 23:1 - All possible types for this assignment were invalidated - This may be dead code

I'm pretty sure https://github.com/vimeo/psalm/blob/e56a34dd190e5a05cf1bdaf8d93ba47f0b381561/src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php#L249 introduced the issue.

In my fork, I've got a branch with a failing test case. If you want, I can open a PR. No idea how to fix it though.

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/f22bcf3289

<?php

/**
 * Returns `never` when given a list to make sure it's not called unnecessarily
 *
 * @template T
 * @param iterable<mixed, T> $iterable
 * @return ($iterable is list ? never : list<T>)
 */
function iterableToList(iterable $iterable): array
{
    return array_values(iterator_to_array($iterable));
}

/**
 * @return iterable<int, string>
 */
function returnsIterable(): iterable
{
    yield "foo";
}

$list = iterableToList(returnsIterable());

Psalm output (using commit cdceda0):

ERROR: NoValue - 23:1 - All possible types for this assignment were invalidated - This may be dead code

INFO: UnusedVariable - 23:1 - $list is never referenced or the value is not used

[ro0NL]

Maybe related

InvalidArrayAccess: Cannot access array value on non-array variable $some of type iterable<mixed, Some>

since 6.13

In our case we override iterable return type with array, as allowed by PHP: https://3v4l.org/S1fMX#v8.4.10

[andrew-demb]

I'm not sure why this should work at all.

When list is passed to $iterable - function will return a list, it won't break the PHP process with die/exit. Psalm can complain about the wrong conditional return type, but I'm not sure whether this should work as it worked before.

/**
 * Returns `never` when given a list to make sure it's not called unnecessarily
 *
 * @template T
 * @param iterable<mixed, T> $iterable
 * @return ($iterable is list ? never : list<T>)
 */
function iterableToList(iterable $iterable): array
{
    return array_values(iterator_to_array($iterable));
}

[MidnightDesign]

@andrew-demb We can argue about that specific function, but there's definitely a bug. Here's a more clear-cut case:

/**
 * @param iterable<mixed, mixed> $i
 * @return ($i is list ? 'list' : 'iter')
 */
function describe(iterable $i) {
    return is_array($i) && array_is_list($i) ? 'list' : 'iter';
}

/**
 * @return iterable<mixed, mixed>
 */
function getIterable(): iterable {
   return new ArrayObject(['x' => 23]);
}

/**
 * @param 'list' | 'iter' $l
 */
function output(string $l): void {
    echo $l;
}

$value = describe(getIterable()); // Should be 'list' | 'iter'
// ERROR: [DocblockTypeContradiction](https://psalm.dev/155) - 26:6 - 'iter' does not contain 'list'
echo $value === 'iter' ? 'Iterable!' : 'List!';

https://psalm.dev/r/6d495ac9d2

As I understand it, a conditional return type should behave like this:

1. If the left operand ($i) of the is operand is a subset of the right operand (list), the expression should evaluate to the "consequent" (the "?" part) expression
2. If the left operand is disjoint from the right operand, it should evaluate to the "alternate" (the ":" part) expression
3. Otherwise it should evaluate to the union of the consequent and the alternate

In this case, we should be dealing with (3):

1. iterable is not a subset of list
2. iterable is not disjoint from list
3. So it should evaluate to 'list' | 'iter'

Here's the same concept with strings instead of iterables (which works):

/**
 * @return ($s is non-empty-string ? 'non-empty' : 'empty')
 */
function describe(string $s) {
    return $s === '' ? 'empty' : 'non-empty';
}

/**
 * @param 'empty' | 'non-empty' $s
 */
function output(string $s): void {
    echo $s;
}

function getString(): string {
    return '';
}

$value = describe(getString());
echo $value === 'empty' ? 'Empty!' : 'Not empty!'; // No issues

https://psalm.dev/r/f310b995a7

---

I also threw this comment against a reasoning model, which might put my argument into better words.

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/6d495ac9d2

<?php

/**
 * @param iterable<mixed, mixed> $i
 * @return ($i is list ? 'list' : 'iter')
 */
function describe(iterable $i) {
    return is_array($i) && array_is_list($i) ? 'list' : 'iter';
}

/**
 * @return iterable<mixed, mixed>
 */
function getIterable(): iterable {
   return new ArrayObject(['x' => 23]);
}

/**
 * @param 'list' | 'iter' $l
 */
function output(string $l): void {
    echo $l;
}

$value = describe(getIterable()); // Should be 'list' | 'iter'
echo $value === 'iter' ? 'Iterable!' : 'List!';

Psalm output (using commit cdceda0):

ERROR: DocblockTypeContradiction - 26:6 - 'iter' does not contain 'list'

ERROR: DocblockTypeContradiction - 26:6 - Docblock-defined type 'list' for $value is never =string(iter)

ERROR: RedundantConditionGivenDocblockType - 26:40 - Docblock-defined type 'list' for $value is never =string(iter)

https://psalm.dev/r/f310b995a7

<?php

/**
 * @return ($s is non-empty-string ? 'non-empty' : 'empty')
 */
function describe(string $s) {
    return $s === '' ? 'empty' : 'non-empty';
}

/**
 * @param 'empty' | 'non-empty' $s
 */
function output(string $s): void {
    echo $s;
}

function getString(): string {
    return '';
}

$value = describe(getString());
echo $value === 'empty' ? 'Empty!' : 'Not empty!'; // No issues

Psalm output (using commit cdceda0):

No issues!

[MidnightDesign]

@danog Sorry for pinging you, but I really think I've provided a solid foundation for someone familiar with the codebase to tackle this bug. I've tried fixing it myself, but the code goes over my head, unfortunately.

Do you think this valid after my latest comment?

[andrew-demb]

Please check this PR https://github.com/vimeo/psalm/pull/11558