False positive InvalidReturnType for method that always throws

[derrabus]

Given the following code:

class Logger
{
    public static function logCall(string $method): void
    {
    }
}

abstract class MyAbstractClass
{    
    /**
     * @return string
     */
    public function doSomething()
    {
        Logger::logCall(__METHOD__);

        throw new Exception('Not supported.');
    }
}

https://psalm.dev/r/b8245d5f63

Psalm reports:

ERROR: InvalidReturnType - 13:16 - The declared return type 'string' for MyAbstractClass::doSomething is incorrect, got 'never'

I would argue that the return type is still correct, it's just that the return type inferred by Psalm is more specific. The problem here is that this method is overridden by child classes and those implementations actually return a string. If I would document @return never here, the child implementation would illegally widen the return type from never to string.

Note that the error goes away if I…

• … set the native return type to string.
• … remove the Logger::logCall(__METHOD__); line from the method.

Both should be pretty much irrelevant for the issue. 🤔

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/b8245d5f63

<?php

class Logger
{
    public static function logCall(string $method): void
    {
    }
}

abstract class MyAbstractClass
{    
    /**
     * @return string
     */
    public function doSomething()
    {
        Logger::logCall(__METHOD__);

        throw new Exception('Not supported.');
    }
}

Psalm output (using commit 1f72597):

ERROR: InvalidReturnType - 13:16 - The declared return type 'string' for MyAbstractClass::doSomething is incorrect, got 'never'

[orklah]

ping @kkmuffme I believe this is linked to your changes around never

[kkmuffme]

Similar to/Inverse of #10377

The underlying problem is, that for @return we use explicit never to indicate a function exits, as there is no other way to annotate this. Therefore here the inverse happens - it reports it, as the function always exits.
I'm aware of the problem with extending classes, but there isn't a way around it
Fyi if you only had the throw https://psalm.dev/r/70d79e581f I already implemented that it won't report an error.

I suggest to just suppress the error, as this cannot be fixed.
Fixing this, would mean that in case you have a function where you actually always want to return never, would not get reported in extending classes - which would be a bigger problem.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/70d79e581f

<?php

class Logger
{
    public static function logCall(string $method): void
    {
    }
}

abstract class MyAbstractClass
{    
    /**
     * @return string
     */
    public function doSomething()
    {
        throw new Exception('Not supported.');
    }
}

Psalm output (using commit efdf425):

No issues!

[derrabus]

Thank you for your answer. That's of course not very satisfying, but I think I can live with ignoring this error.