psalm / psalm-plugin-laravel

A Psalm plugin for Laravel
MIT License
302 stars 72 forks source link

Static return types are being ignored #129

Closed eithed closed 3 years ago

eithed commented 3 years ago

Describe the bug When using find or findOrFail methods within Laravel as such:

$account = Account::findOrFail($request->account_id);

It will be inferred that Illuminate\Database\Eloquent\Collection<Illuminate\Database\Eloquent\Model>|Illuminate\Database\Eloquent\Model is being returned, even though what is actually returned is object of Account type, per declaration within find or findOrFail being * @return \Illuminate\Database\Eloquent\Model|\Illuminate\Database\Eloquent\Collection|static|static[]

This will cause additional errors to be returned like:

ERROR: UndefinedMagicPropertyFetch - app/Http/Controllers/Policy/StoreController.php:36:31 - Magic instance property Illuminate\Database\Eloquent\Collection::$current_turnover is not defined (see https://psalm.dev/218)

or

ERROR: InvalidArgument - app/Services/CoverTransactionService.php:351:72 - Argument 1 of App\Snapshots\AccountSnapshot::__construct expects App\Models\Account, Illuminate\Database\Eloquent\Collection<Illuminate\Database\Eloquent\Model>|Illuminate\Database\Eloquent\Model provided (see https://psalm.dev/004)

It's as if static was not being taken into account when considering returned types.

As a workaround one needs to preamble with @var as such:

/**
 * @var Account $account
 */
 $account = Account::findOrFail($request->account_id);

but I believe the expected behaviour should be inferring that Account model is being returned per static

Impacted Versions

barryvdh/laravel-ide-helper                      v2.9.0             Laravel IDE Helper, generates correct PHPDocs for all Facade classes, to improve auto-completion.
cknow/laravel-money                              v6.1.0             Laravel Money
fruitcake/laravel-cors                           v2.0.3             Adds CORS (Cross-Origin Resource Sharing) headers support in your Laravel application
jessarcher/laravel-castable-data-transfer-object v1.0.0             Automatically cast JSON columns to rich PHP objects in Laravel using Spatie's data-transfer-object class
kreait/laravel-firebase                          2.4.0              A Laravel package for the Firebase PHP Admin SDK
laravel/framework                                v8.30.1            The Laravel Framework.
laravel/horizon                                  v5.7.0             Dashboard and code-driven configuration for Laravel queues.
laravel/tinker                                   v2.6.1             Powerful REPL for the Laravel framework.
psalm/plugin-laravel                             v1.4.2             A Laravel plugin for Psalm
spatie/laravel-activitylog                       3.17.0             A very simple activity logger to monitor the users of your website or application
spatie/laravel-enum                              2.4.0              Laravel Enum support
spatie/laravel-event-sourcing                    4.8.0              The easiest way to get started with event sourcing in Laravel
spatie/laravel-json-api-paginate                 1.10.0             A paginator that plays nice with the JSON API spec
spatie/laravel-query-builder                     3.3.4              Easily build Eloquent queries from API requests
spatie/laravel-schemaless-attributes             1.8.3              Add schemaless attributes to Eloquent models
vimeo/psalm                                      4.6.2              A static analysis tool for finding errors in PHP applications
mr-feek commented 3 years ago

I wasn't able to reproduce this in the test suite: https://github.com/psalm/psalm-plugin-laravel/commit/997c3e61447c3a05c219e4fc62a50e707fc0a55b

I believe this may be resolved now. Please reopen if you think I'm incorrect!