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

Method Builder::whereX() should return static(Builder<Model>) but returns Builder<Model> #1676

Open
sanfair opened this issue Jun 27, 2023 · 3 comments

Comments

@sanfair
Copy link
Contributor

sanfair commented Jun 27, 2023

Description

When defining a new whereX method in custom builder with return type annotation phpstan shows errors.

In PR you can find both possible definitions that correctly execute at runtime, but trigger errors on static analysis.

Real application example: https://github.com/koel/koel/blob/master/app/Builders/AlbumBuilder.php#L10-L13
Note: in this case, an error can be resolved by specifying the return type as self, but it is not always correct.

Possible related issues from phpstan: phpstan/phpstan#5132 phpstan/phpstan#4648
Note: solution with self used from these issues

Code where the issue was found

use Illuminate\Database\Eloquent\Builder as BaseBuilder;

/**
 * @template TModelClass of \Illuminate\Database\Eloquent\Model
 *
 * @extends BaseBuilder<TModelClass>
 */
abstract class Builder extends BaseBuilder
{
    public function whereSomething($column, $values, $boolean = 'and'): static
    {
        return $this->whereNotIn(...func_get_args());
    }

    public function whereSomethingElse($column, $values, $boolean = 'and'): self
    {
        return $this->whereNotIn(...func_get_args());
    }
}

I managed to find the exact place where the type is defined. I tried to replace \PHPStan\Type\Generic\GenericObjectType with \PHPStan\Type\StaticType but then it fails in other places.

https://github.com/nunomaduro/larastan/blob/706784ec6b1bad487692730dc20aeee0ff1b19fa/src/Methods/EloquentBuilderForwardsCallsExtension.php#L150-L154

@szepeviktor
Copy link
Collaborator

szepeviktor commented Jun 27, 2023

Hello!
Have you tried adding @return TModelClass?

@sanfair
Copy link
Contributor Author

sanfair commented Jun 27, 2023

Hello, I may have made some confusion by stripping some information from the error just to fit it in the title.

Here are the actual error messages:

 ------ ------------------------------------------------------------------- 
  Line   app/Models/Builder/Builder.php                                     
 ------ ------------------------------------------------------------------- 
  16     Method App\Models\Builder\Builder::whereSomething() should return  
         static(App\Models\Builder\Builder<TModelClass of Illuminate\Database\Eloquent\Model>) but returns                   
                App\Models\Builder\Builder<Illuminate\Database\Eloquent\Model>.    
 ------ ------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------- 
  Line   app/Models/Builder/UserBuilder.php                                  
 ------ -------------------------------------------------------------------- 
  14     Method App\Models\Builder\UserBuilder::whereSomethingElse() should  
         return App\Models\Builder\UserBuilder but returns                   
                App\Models\Builder\Builder.                                         
 ------ -------------------------------------------------------------------- 

You can check https://github.com/sanfair/larastan-playground/pull/1/checks for more context.

The problem is not in a model class. When phpstan analyzes method Builder::whereSomething() and sees the static keyword it expects that the return type will be static(Builder) but $this->whereNotIn() returns the Builder type without static.
Somehow larastan loses the proper type that should be inferred based on the definition from Laravel (see below)

    /**
     * Add a "where not in" clause to the query.
     *
     * @param  \Illuminate\Contracts\Database\Query\Expression|string  $column
     * @param  mixed  $values
     * @param  string  $boolean
     * @return $this
     */
    public function whereNotIn($column, $values, $boolean = 'and')
    {
        return $this->whereIn($column, $values, $boolean, true);
    }

I recreated the exact type structure but in raw PHP https://phpstan.org/r/bf3dd532-58f3-49c4-818e-fdf99883de72
But in Laravel + larastan inside the Builder class error will be on line 41, with the message as the error on line 49

Please check the types in the test method.

@szepeviktor
Copy link
Collaborator

I recreated the exact type structure but in raw PHP

👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants