Skip to content

Enhance QueryBuilder with generics support for better type inference #1002

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

Merged
merged 3 commits into from
Apr 16, 2025

Conversation

alexkart
Copy link
Contributor

@alexkart alexkart commented Apr 15, 2025

This pull request includes several changes to the QueryBuilder class in src/QueryBuilder.php to improve type safety and code clarity by adding template annotations and type hints. The most important changes include adding a template annotation to the class, updating the return type annotations for methods, and ensuring proper type casting.

When trying to specify the correct type in the following code:

/**
     * @return LengthAwarePaginator<int, Client>
     */
    public function queryClients(Request $request): LengthAwarePaginator
    {
        /** @var QueryBuilder<Client> $queryBuilder */
        $queryBuilder = QueryBuilder::for(Client::class, $request);
...

Static analysis (PHPStan) generates the following error:

Method Modules\Client\Repositories\ClientRepository::queryClients() should return Illuminate\Pagination\LengthAwarePaginator<int, Modules\Client\Models\Client> but returns  
         Illuminate\Pagination\LengthAwarePaginator<int, Illuminate\Database\Eloquent\Model>.                                                                                         
         🪪  return.type

It happens because Illuminate\Database\Eloquent\Builder is a generic class:

/**
 * @template TModel of \Illuminate\Database\Eloquent\Model
 */
class Builder implements BuilderContract

but \Spatie\QueryBuilder\QueryBuilder that specifies this class as a @mixin is not generic, so there is no way to specify a concrete type for the TModel.
This PR adds the ability to do this.

@alexkart alexkart marked this pull request as draft April 15, 2025 18:21
@alexkart alexkart marked this pull request as ready for review April 15, 2025 18:28
@freekmurze freekmurze merged commit 3675f7b into spatie:main Apr 16, 2025
1 check passed
@freekmurze
Copy link
Member

Thanks!

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 this pull request may close these issues.

2 participants