vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.54k stars 660 forks source link

Typehint at InvalidReturnStatement incorrectly states fully qualified class names #2654

Closed HenkPoley closed 4 years ago

HenkPoley commented 4 years ago

I've got an INFO error along the lines of:

INFO: InvalidReturnStatement - app/MyNamespace/SomeClass.php:115:16 
- The type 
'array{extras: Illuminate\Support\Collection}' 
does not match the declared return type 
'array{extras: App\MyNamespace\Illuminate\Support\Collection}'
for App\MyNamespace\SomeClass::toArray

Which I got after copy-pasting the auto-detected @psalm-return type for that method, from a previous psalm run.

The full qualified class name should be (notice the leading slash) \Illuminate\Support\Collection instead of Illuminate\Support\Collection or App\QDT\VO\Illuminate\Support\Collection. With the leading slash, psalm is happy "No errors found!".

(Additionally, it would be really nice if it could get pretty-printed what the mismatching parts are)

Psalm 3.8.3@389af1bfc739bfdff3f9e3dc7bd6499aee51a831

muglug commented 4 years ago

This isn't a bug (at least, I don't consider it one, and PHPStan has the same behaviour). If you want automatic fixes that populate docblocks appropriately, use Psalter: https://psalm.dev/docs/manipulating_code/fixing/#invalidreturntype

HenkPoley commented 4 years ago

At the moment psalter --issues=InvalidReturnType doesn't quite add @psalm-return docblocks with psalm's complex array syntax.

muglug commented 4 years ago

What does it do in your case?

HenkPoley commented 4 years ago

🤔

When I just run vendor/bin/psalter --dry-run --issues=InvalidReturnType --threads=4 on files where I set something like @psalm-return foo, it will impute a psalm return type.

My normal dry-run-psalter.sh script uses the following issue fixers string, which does not propose the fixes.

--issues=InvalidFalsableReturnType,InvalidNullableReturnType,InvalidReturnType,LessSpecificReturnType,MismatchingDocblockParamType,MismatchingDocblockReturnType,MissingParamType,MissingReturnType

Edit: the @psalm-return type also literally needs to be incorrect (not just missing). But that is okay.

zmitic commented 4 years ago

I can confirm that LessSpecificReturnType reports false positives. For a code like this:

class Back implements ConfigurationInterface
{
    public function getAliasName(): string
    {
        return 'back_button';
    }
}

and ./vendor/bin/psalter --dry-run --php-version=7.4 --issues=LessSpecificReturnType, I get this error:

image


So even though the method has typehinted string for return, psalter wants to add phpdoc for it anyways.