Incorrect PossiblyUndefinedVariable issue reporting

[boesing]

It seems that psalm somehow does not realize that due to the try-catch block, the variable $indicesByAlias is always set in case there are no errors.

Somehow, the catch is not parsed properly as we do use continue in there. Using continue should continue the loop and thus, there should be no codepath which actually leads to the $indicesByAlias being undefined.

https://psalm.dev/r/ef08112a04

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/ef08112a04

<?php


final class Foo
{
    /**
     * @template T of non-empty-string
     * @param non-empty-list<T> $aliases
     * @return non-empty-array<T,list<non-empty-string>>
     */
    private function detectIndicesForAliases(array $aliases): array
    {
        /** @var non-empty-array<T,list<non-empty-string>> $indices */
        $indices = array_combine(
            $aliases,
            array_fill(0, count($aliases), [])
        );

        foreach ($aliases as $alias) {
            try {
                /** @var list<non-empty-string> $indicesByAlias */
                $indicesByAlias = array_keys([]);
            } catch (Throwable $throwable) {
                if ($throwable->getCode() === 123) {
                    continue;
                }

                throw $throwable;
            }

            $indices[$alias] = $indicesByAlias;
        }

        return $indices;
    }
}

Psalm output (using commit 24f7920):

INFO: PossiblyUndefinedVariable - 31:32 - Possibly undefined variable $indicesByAlias defined in try block

[AndrolGenhald]

Will almost certainly be fixed by #7688, but I need to find some time to get back to it. The last I checked I had to also completely redo how we're handling variables defined in ifs to get it to work, and there were like 3-5 tests failing, but the failing cases were really annoying to figure out.

[alcaeus]

I had a similar issue regarding a try-catch block: https://psalm.dev/r/b1102ead61. In this example, psalm does not correctly track the value of $string, which will always be a string unless an exception was caught (which is then re-thrown). Not sure if it's the same issue, I can open a new ticket for this if necessary.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/b1102ead61

<?php

function returnsString(): string
{
    $exception = null;
    $string = null;
    
    try {
        $string = generatesStringOrThrows();
    } catch (Exception $exception) {
    }
    
    // Imagine we're doing other stuff here
    
    if ($exception !== null) {
        throw $exception;
    }
    
    return $string;
}

function generatesStringOrThrows(): string
{
    if (rand(0, 1) == 1) {
        return 'foo';
    }
    
    throw new Exception('nope');
}

Psalm output (using commit d957ff2):

ERROR: NullableReturnStatement - 19:12 - The declared return type 'string' for returnsString is not nullable, but the function returns 'null|string'

ERROR: InvalidNullableReturnType - 3:27 - The declared return type 'string' for returnsString is not nullable, but 'null|string' contains null

[AndrolGenhald]

@alcaeus That's a separate issue that comes up now and then, having types of variables be dependent on type checks of other variables is really complicated and not likely to be supported any time soon: https://psalm.dev/r/f6d94dc39e

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/f6d94dc39e

<?php

/** @var mixed */
$foo = null;
$isString = false;

if (is_string($foo)) {
    $isString = true;
}

if ($isString) {
    /** @psalm-trace $foo */; // Technically should be `string`
}

Psalm output (using commit d957ff2):

INFO: Trace - 12:29 - $foo: mixed