psalm-plugin-laravel icon indicating copy to clipboard operation
psalm-plugin-laravel copied to clipboard

Psalm not detecting helper methods?

Open DeeRock94 opened this issue 3 years ago • 8 comments

Describe the bug So I've noticed when using helper methods such as

return response()->json([
'data' => 'hello world'
]);

within a method that should have an invalid return type, this flags as being No Errors Found ?

So for example:

    public function test(): \App\Models\User
    {
        return response()->json([
            'hello world'
        ]);
    }

My IDE flags Return value is expected to be '\App\Models\User', '\Illuminate\Http\JsonResponse' returned which I'd expect, but psalm flags:

------------------------------
                              
       No errors found!       
                              
------------------------------

If I were to return just "hello world" without the use of the json helper. I do get an error.

    public function test(): \App\Models\User
    {
        return 'hello world';
    }

Response:

ERROR: InvalidReturnType - app/Http/Controllers/Api/WebsiteBuilderController.php:208:29 - The declared return type 'App\Models\User' for App\Http\Controllers\Api\WebsiteBuilderController::test is incorrect, got '"hello world"' (see https://psalm.dev/011)
    public function test(): \App\Models\User


ERROR: InvalidReturnStatement - app/Http/Controllers/Api/WebsiteBuilderController.php:210:16 - The inferred type '"hello world"' does not match the declared return type 'App\Models\User' for App\Http\Controllers\Api\WebsiteBuilderController::test (see https://psalm.dev/128)
        return 'hello world';


------------------------------
2 errors found
------------------------------

Impacted Versions Please paste the output of composer show | grep -E 'psalm|laravel' so we can see what versions are impacted

barryvdh/laravel-ide-helper           v2.12.3                        Laravel IDE Helper, generates correct PHPDocs for all Facade classes, to improve auto-completion.
beyondcode/laravel-websockets         1.13.1                         An easy to use WebSocket server
fruitcake/laravel-cors                v2.2.0                         Adds CORS (Cross-Origin Resource Sharing) headers support in your Laravel application
laravel/cashier                       v13.8.6                        Laravel Cashier provides an expressive, fluent interface to Stripe's subscription billing services.
laravel/framework                     v9.8.1                         The Laravel Framework.
laravel/sail                          v1.13.9                        Docker files for running a basic Laravel application.
laravel/sanctum                       v2.15.1                        Laravel Sanctum provides a featherweight authentication system for SPAs and simple APIs.
laravel/serializable-closure          v1.1.1                         Laravel Serializable Closure provides an easy and secure way to serialize closures in PHP.
laravel/socialite                     v5.5.2                         Laravel wrapper around OAuth 1 & OAuth 2 libraries.
laravel/tinker                        v2.7.2                         Powerful REPL for the Laravel framework.
laravel/vapor-cli                     v1.36.1                        The Laravel Vapor CLI
laravel/vapor-core                    v2.21.3                        The kernel and invokation handlers for Laravel Vapor
marvinlabs/laravel-discord-logger     v1.2.0                         Logging to a discord channel in Laravel
psalm/plugin-laravel                  v2.0.0                         A Laravel plugin for Psalm
spatie/laravel-ignition               1.2.1                          A beautiful error page for Laravel applications.
spatie/laravel-permission             5.5.2                          Permission handling for Laravel 6.0 and up
spatie/laravel-ray                    1.29.6                         Easily debug Laravel apps
vimeo/psalm                           4.23.0                         A static analysis tool for finding errors in PHP applications

Additional context I'm unsure if this lies within psalm its self or within the psalm-plugin-laravel. I could also be misunderstanding something but I'd of thought psalm would have flagged this.

I'd also like to mention using the JsonResponse facade, actually works. Example below:

    public function test(): \App\Models\User
    {
        return new JsonResponse([
            'data' => 'hello world'
        ]);
    }

Psalm:

ERROR: InvalidReturnType - app/Http/Controllers/Api/WebsiteBuilderController.php:208:29 - The declared return type 'App\Models\User' for App\Http\Controllers\Api\WebsiteBuilderController::test is incorrect, got 'Illuminate\Http\JsonResponse' (see https://psalm.dev/011)
    public function test(): \App\Models\User


ERROR: InvalidReturnStatement - app/Http/Controllers/Api/WebsiteBuilderController.php:210:16 - The inferred type 'Illuminate\Http\JsonResponse' does not match the declared return type 'App\Models\User' for App\Http\Controllers\Api\WebsiteBuilderController::test (see https://psalm.dev/128)
        return new JsonResponse([
            'data' => 'hello world'
        ]);


------------------------------
2 errors found
------------------------------

Anyone who can identify something I've done wrong too would be greatly appreciated as it could be a misunderstanding on my part

DeeRock94 avatar May 16 '22 10:05 DeeRock94

What's the output of

    public function test(): \App\Models\User
    {
		 $r = response();
        /** @psalm-trace $r */;
        $j = $r->json(['hello world']);
        /** @psalm-trace $j */;
        return $j;
    }

weirdan avatar May 16 '22 15:05 weirdan

Hey @weirdan - Thanks for the reply

The response to this when running psalm is below:

------------------------------
                              
       No errors found!       
                              
------------------------------

However, PHPStorm continues to flag: Return value is expected to be '\App\Models\User', '\Illuminate\Http\JsonResponse' returned

DeeRock94 avatar May 16 '22 16:05 DeeRock94

Make sure you use --show-info=true cli argument, or @psalm-trace output may not show up.

weirdan avatar May 16 '22 17:05 weirdan

So I have other errors within there which I'm aware of when using --show-info=true, however, I CMD + F'ed and searched for App\Http\Controllers\Api\WebsiteBuilderController::test to see if anything within this log flagged something to do with the method in question used for testing - Sadly 0 results.

A lot of the other issues I've got in this log are related to missing return types etc within Models, something I'm actively fixing but isn't relevant to a simple method such as this

DeeRock94 avatar May 16 '22 17:05 DeeRock94

Adding /** @psalm-trace $j */; should make Psalm return an error called "Trace". If it's not popping up, make sure the file is actually analyzed by Psalm. Look at your config or check if it appears when running Psalm with --debug-by-line

orklah avatar May 17 '22 18:05 orklah

Ah @orklah spot on, you're right! Thanks for that, so I done this and got this response:

INFO: Trace - app/Http/Controllers/Api/WebsiteBuilderController.php:211:31 - $r: Illuminate\Contracts\Routing\ResponseFactory|Illuminate\Http\Response (see https://psalm.dev/224)
        /** @psalm-trace $r */;


INFO: Trace - app/Http/Controllers/Api/WebsiteBuilderController.php:213:31 - $j: Illuminate\Http\JsonResponse|mixed (see https://psalm.dev/224)
        /** @psalm-trace $j */;

It would appear that $j coulld be JsonResponse|mixed which might be why technically psalm is not flagging this when something that isn't JsonResponse is returned, do you think this is the case? The response being too ambiguous?

DeeRock94 avatar May 17 '22 18:05 DeeRock94

yeah, if the response is mixed, you'd have to be at level 1 for Psalm to show you the error (since mixed is potentially your type).

You'll have to check why mixed is returned there. Either the source is not properly documented or magic is involved somehow. I don't know laravel at all so I can't check.

orklah avatar May 17 '22 19:05 orklah

Many of the helper methods could be typed more precisely using a conditional return type.

weirdan avatar May 26 '22 04:05 weirdan

@DeeRock94 I just tested with plugin v2.5 and it properly reports about an issue:

InvalidReturnType The declared return type 'Tests\\Psalm\\LaravelPlugin\\Models\\User' for test is incorrect, got 'Illuminate\\Http\\JsonResponse'

Closing

alies-dev avatar Jan 27 '23 11:01 alies-dev