InvalidReturnType are reported for stubs

[kkmuffme]

I have a project where a function exists both in the stubs AND in the code to analyze (basically: overwriting a function/method, but also for cases where psalm is unable to infer/load files due to how paths are loaded)

In my file I have this (just a mock example):
https://psalm.dev/r/e7f20deb65

Problem:
as this function exists in my stubs too, Psalm now reports this error for the stubs file - which gets suppressed (since errors in stubs file paths are suppressed by psalm by default)
Therefore this error never shows up.

Why this is a problem?
Psalm always takes the phpdoc from the stubs as truth (this is fine/good). It then uses the code (not stubs) to check if the code matches the phpdoc.
In this case however, it would make much more sense to report the error not for the stubs but for the file where it checks the code in.

I remember I had a similar issue a long time ago, when I got started with psalm, so I think this is not an isolated issue with InvalidReturnType

EDIT: this affects all (but only) PHPDoc issues for functions. While most of them get reported for both the stubs + the file the issue is in, InvalidReturnType and MissingParamType get only reported for the stubs.
But e.g. InvalidDocblock is reported for both (stubs and the file)

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/e7f20deb65

<?php

/**
 * @return never
 */
function foo() {
   if (rand(1,2) === 2 ) {
     exit;  
   }
}

Psalm output (using commit afe85fa):

ERROR: InvalidReturnType - 4:12 - foo is not expected to return any values but it does, either implicitly or explicitly

[AndrolGenhald]

Just to make sure I understand the issue, you're saying because foo() is stubbed with @return never, the actual foo() declaration in your code has InvalidReturnType suppressed?

[kkmuffme]

Yes (kind of).

Technically correct reply:
It's not suppressed but it reports the error for the stubs (and stubs errors are suppressed), instead of the actual foo().
This issue affects ONLY @return errors, e.g. InvalidNullableReturnType, InvalidReturnType,...

What seems to be happening internally:
Psalm takes the stubs @return, but uses it for the actual function (ignoring the @return declared in the phpdoc of the actual function - this is fine, this is how psalm works, as it always gives higher priority to phpdoc of stubs) - therefore if the actual function returns something other than what was declared in the stubs, it will report these errors for the stubbed function instead of the actual function.

So the bug is NOT that it takes the phpdoc of the stubs. The bug is that it reports the error on the stubs (where it took the phpdoc from) instead of the actual function.

[AndrolGenhald]

I think the proper way to fix this would be to re-parse the docblock and return signature when analyzing a function, and use that instead of whatever exists in storage. That way if the function declaration conflicts with the stub, the function itself will still be analyzed correctly, but the rest of the codebase will assume the function works as declared in the stub.

[kkmuffme]

Is this a bug or an enhancement?

[AndrolGenhald]

🤷 I just picked one because it's not clear to me if it was originally intentional behavior.

[kkmuffme]

This also seems to affect @var in class variables, e.g.

class Foo {
    /** @var MyBar */
    public $bar;
}

will report an error in the stubs for UndefinedDocblockClass

[kkmuffme]

For now I just added the stubs to projectFiles explicitly, so it will report errors from the stubs instead of ignoring those (and just manually check the correct line/file)