vimeo / psalm

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

Psalm is inferring incorrect return types when using template #4863

Open ignas2526 opened 3 years ago

ignas2526 commented 3 years ago

I am trying to figure out how to set correct type hinting for Laravel classes. I believe that I've done it correctly, but Psalm keeps inferring wrong type. I'm not sure if its Psalm issue or I'm doing something wrong.

The code is simplified Laravel Eloquent relation. The return types of test1 - test4 methods in the class Something should match the ones specified in the docblock.

Note: I've added "@psalm-suppress InvalidReturnType" in couple places so that it won't complain about missing method implementation.

https://psalm.dev/r/7e5bb88835

Thanks for help.

psalm-github-bot[bot] commented 3 years ago

I found these snippets:

https://psalm.dev/r/7e5bb88835 ```php * @mixin \Illuminate\Database\Eloquent\Builder */ class HasOne extends HasOneOrMany { } /** * @template TRelatedModel of Model * @mixin \Illuminate\Database\Eloquent\Builder */ abstract class HasOneOrMany { } } namespace Illuminate\Database\Eloquent { /** * @template TModel of \Illuminate\Database\Eloquent\Model */ class Builder { /** * @return static * @psalm-suppress InvalidReturnType */ public function where() { } /** * @return TModel|null * @psalm-suppress InvalidReturnType */ public function first() { } } } namespace Illuminate\Database\Eloquent { /** * @mixin \Illuminate\Database\Eloquent\Builder */ abstract class Model { use Concerns\HasRelationships; } } namespace Illuminate\Database\Eloquent\Concerns { use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\HasOne; trait HasRelationships { /** * @template TRelatedModel of Model * @psalm-param class-string $related * * @param string $related * @return \Illuminate\Database\Eloquent\Relations\HasOne * * @psalm-return \Illuminate\Database\Eloquent\Relations\HasOne * @psalm-suppress InvalidReturnType */ public function hasOne($related) { } } } namespace App { use Illuminate\Database\Eloquent\Model; class Something extends Model { /** * @return \Illuminate\Database\Eloquent\Relations\HasOne<\App\Something> */ public function test1() { return $this->hasOne(Something::class); } /** * @return \Illuminate\Database\Eloquent\Relations\HasOne<\App\Something> */ public function test2() { return $this->hasOne(Something::class)->where(); } /** * @return \App\Something|null */ public function test3() { return $this->hasOne(Something::class)->first(); } /** * @return \App\Something|null */ public function test4() { return $this->hasOne(Something::class)->where()->first(); } } } ``` ``` Psalm output (using commit 2a4e5a2): INFO: LessSpecificReturnStatement - 108:17 - The type '(TRelatedModel:Illuminate\Database\Eloquent\Relations\HasOne as Illuminate\Database\Eloquent\Model)|null' is more general than the declared return type 'App\Something|null' for App\Something::test3 INFO: MoreSpecificReturnType - 104:20 - The declared return type 'App\Something|null' for App\Something::test3 is more specific than the inferred return type '(TRelatedModel:Illuminate\Database\Eloquent\Relations\HasOne as Illuminate\Database\Eloquent\Model)|null' INFO: LessSpecificReturnStatement - 116:17 - The type '(TRelatedModel:Illuminate\Database\Eloquent\Relations\HasOne as Illuminate\Database\Eloquent\Model)|null' is more general than the declared return type 'App\Something|null' for App\Something::test4 INFO: MoreSpecificReturnType - 112:20 - The declared return type 'App\Something|null' for App\Something::test4 is more specific than the inferred return type '(TRelatedModel:Illuminate\Database\Eloquent\Relations\HasOne as Illuminate\Database\Eloquent\Model)|null' ```
weirdan commented 3 years ago

Same mixin on several inheritance levels seems to confuse Psalm. Dropping @mixin Builder<TRelatedModel> from HasOne makes it all check fine: https://psalm.dev/r/10511f9007

psalm-github-bot[bot] commented 3 years ago

I found these snippets:

https://psalm.dev/r/10511f9007 ```php * @dont-use-no-mixin-here \Illuminate\Database\Eloquent\Builder */ class HasOne extends HasOneOrMany { } /** * @template TRelatedModel of Model * @mixin \Illuminate\Database\Eloquent\Builder */ abstract class HasOneOrMany { } } namespace Illuminate\Database\Eloquent { /** * @template TModel of \Illuminate\Database\Eloquent\Model */ class Builder { /** * @return static * @psalm-suppress InvalidReturnType */ public function where() { } /** * @return TModel|null * @psalm-suppress InvalidReturnType */ public function first() { } } } namespace Illuminate\Database\Eloquent { /** * @mixin \Illuminate\Database\Eloquent\Builder */ abstract class Model { use Concerns\HasRelationships; } } namespace Illuminate\Database\Eloquent\Concerns { use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\HasOne; trait HasRelationships { /** * @template TRelatedModel of Model * @psalm-param class-string $related * * @param string $related * @return \Illuminate\Database\Eloquent\Relations\HasOne * * @psalm-return \Illuminate\Database\Eloquent\Relations\HasOne * @psalm-suppress InvalidReturnType */ public function hasOne($related) { } } } namespace App { use Illuminate\Database\Eloquent\Model; class Something extends Model { /** * @return \Illuminate\Database\Eloquent\Relations\HasOne<\App\Something> */ public function test1() { return $this->hasOne(Something::class); } /** * @return \Illuminate\Database\Eloquent\Relations\HasOne<\App\Something> */ public function test2() { return $this->hasOne(Something::class)->where(); } /** * @return \App\Something|null */ public function test3() { return $this->hasOne(Something::class)->first(); } /** * @return \App\Something|null */ public function test4() { return $this->hasOne(Something::class)->where()->first(); } } } ``` ``` Psalm output (using commit 2a4e5a2): No issues! ```
alies-dev commented 1 year ago

I have a similar issue with @mixin and static template:

https://psalm.dev/r/14346c8ed0

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/14346c8ed0 ```php */ abstract class Model {} /** * @template TModel of Model */ class Query { public function where(): static { return $this; } } class User extends Model {} $q = User::where(); /** @psalm-check-type-exact $q = Query */ ``` ``` Psalm output (using commit 3af9ccd): ERROR: CheckType - 20:48 - Checked variable $q = Query does not match $q = Query INFO: UnusedVariable - 19:1 - $q is never referenced or the value is not used ```