Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-generic type when type hinting builder on Eloquent where* methods #926

Closed
stayallive opened this issue Sep 6, 2021 · 6 comments · Fixed by #1144
Closed

Non-generic type when type hinting builder on Eloquent where* methods #926

stayallive opened this issue Sep 6, 2021 · 6 comments · Fixed by #1144
Labels
bug Something isn't working

Comments

@stayallive
Copy link

stayallive commented Sep 6, 2021

  • Larastan Version: 0.7.12
  • --level used: 2

Description

For PhpStorm we like to type hint the builder on where* methods when building an Eloquent query for that aawesome autocompletion PhpStorm an offer.

However I'm seeing something that might be totally expected but I found unexpected.

When type hinting the (Eloquent) Builder on the closure Larastan uses that over the (what I can only assume being injected from stub) generic variant of the builder. I would expect Larastan to also inject the generic version of the builder when I type hint the builder myself since that is just a more specific type of the builder just like I would use a native/naked type hint and use the docblocks to expand on that type hint with generics.

Laravel code where the issue was found

<?php

use App\Models\User;
use App\Models\User\EmailAddress;
use Illuminate\Database\Eloquent\Builder;

// These do not work, non-generic builder -> errors
User::query()->where(function (Builder $query) {
    $query->withTrashed();
});
User::query()->whereHas('emailAddresses', function (Builder $query) {
    $query->validated()->where('email', '=', 'foo@bar.com');
});

// These do work, generic builder -> no error
User::query()->where(function ($query) {
    $query->withTrashed();
});
User::query()->whereHas('emailAddresses', function ($query) {
    $query->validated()->where('email', '=', 'foo@bar.com');
});
 ------ ---------------------------------------------------------------------------------- 
  Line   test.php                                                                          
 ------ ---------------------------------------------------------------------------------- 
  9      Call to an undefined method Illuminate\Database\Eloquent\Builder::withTrashed().  
  12     Call to an undefined method Illuminate\Database\Eloquent\Builder::validated().    
 ------ ---------------------------------------------------------------------------------- 

(withTrashed and validated are both scopes on the models being queried)

@szepeviktor
Copy link
Collaborator

szepeviktor commented Sep 6, 2021

Closures cannot be typehinted for PHPStan, so cannot say
@param Builder<User> $query :(

Wait a little bit and it will be possible.

@stayallive
Copy link
Author

stayallive commented Sep 6, 2021

function ($query) does work (second block in my previous code example) but function (Builder $query does not, so if it's able to figure out the first one something about the second is going wrong with replacing the type hint with a generic builder.

This works without errors, so it works fine type hinting the correct thing:

<?php

use App\Models\User;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Builder;

/**
 * @template TModel of Model
 */
class MockBuilder
{
    /**
     * @param \Closure(Builder<TModel>): void $closure
     */
    public function where(\Closure $closure)
    {
        $closure($this);
    }
}

/** @var MockBuilder<User> $builder */
$builder = new MockBuilder;
$builder->where(function (Builder $query) {
    $query->withTrashed();
});

PhpStan cannot find errors in the example above.

@szepeviktor
Copy link
Collaborator

szepeviktor commented Sep 6, 2021

The closure/anonymous function itself has the problem
phpstan/phpstan#3770
not Closure type.

@stayallive
Copy link
Author

stayallive commented Sep 6, 2021

For the where it can be fixed by doing this change in the EloquentBuilder.stub it looks like:

    /**
     * Add a basic where clause to the query.
     *
-    * @param  \Closure|array<model-property<TModelClass>|int, mixed>|\Illuminate\Database\Query\Expression  $column
+    * @param  \Closure(static<TModelClass>): void|\Illuminate\Database\Query\Expression  $column
     * @param  mixed  $operator
     * @param  mixed  $value
     * @param  string  $boolean
     * @return static
     */
    public function where($column, $operator = null, $value = null, $boolean = 'and');

For some reason array<model-property<TModelClass>|int, mixed> needs to go too to make this work, probably has something to do with how model-property works but as a proof of concept with the above change in the stub file this has no errors anymore:

<?php

use App\Models\User;
use Illuminate\Database\Eloquent\Builder;

User::query()->where(function (Builder $query) {
    $query->withTrashed();
});

So at some level it could be fixed and is working...

@canvural
Copy link
Collaborator

Does this only happen with where?

@canvural canvural added the bug Something isn't working label Sep 22, 2021
@canvural
Copy link
Collaborator

@param \Closure(static<TModelClass>): void|model-property<TModelClass>|array<model-property<TModelClass>|int, mixed>|\Illuminate\Database\Query\Expression $column would work. But there is also a bug in PHPStan itself. We need to wait for that: phpstan/phpstan#5675

canvural added a commit that referenced this issue Feb 28, 2022
canvural added a commit that referenced this issue Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants