[Bug]: Incorrect type hint in "FromQuery" interface

[halloei]

Is the bug applicable and reproducable to the latest version of the package and hasn't it been reported before?

• Yes, it's still reproducable

What version of Laravel Excel are you using?

3.1.35

What version of Laravel are you using?

8.77.0

What version of PHP are you using?

8.1.1

Describe your issue

In your example the result of \App\Invoice::query() is used as return value for the \App\Exports\InvoicesExport::query() method. I'm assuming \App\Invoice is a Laravel model, so the return value will be an instance of \Illuminate\Database\Eloquent\Builder.

However, in the FromQuery interface the return value of the query() method is set to \Illuminate\Database\Query\Builder. These are two different classes, none of them is a parent of the other and they're not interchangeable.

Note: There will be a common interface for both classes, but not until Laravel 9: laravel/framework#37956

How can the issue be reproduced?

Take the example from the offical docs. Your IDE will warn you and PHPStan will throw an error.

What should be the expected behaviour?

\Illuminate\Database\Eloquent\Builder should be the return type of FromQuery::query().

[patrickbrouwers]

It's also allowed to return a query, not only eloquent queries. So changing the return type would limit half of the use cases

[halloei]

It is not allowed to return \Illuminate\Database\Eloquent\Builder, as it's no subclass of \Illuminate\Database\Query\Builder. Please see here:
https://github.com/laravel/framework/blob/v8.77.0/src/Illuminate/Database/Eloquent/Builder.php
https://github.com/laravel/framework/blob/v8.77.0/src/Illuminate/Database/Query/Builder.php

[patrickbrouwers]

It has worked for years (as prove the automated tests). If you have suggestions or improvements please provide them via a PR.

[halloei]

It works because the return type is only declarated in PHPDoc and thus not actually considered by the interpreter. If I change

interface FromQuery
{
    /**
     * @return Builder
     */
    public function query();
}

to

interface FromQuery
{
    public function query(): Builder;
}

the tests will fail:

dev@ba2c49866dfd:/usr/src/app$ vendor/bin/phpunit --stop-on-error
PHPUnit 9.5.11 by Sebastian Bergmann and contributors.

Warning: Your XML configuration validates against a deprecated schema.
Suggestion: Migrate your XML configuration using "--migrate-configuration"!

..........................E

Time: 00:00.401, Memory: 40.50 MB

There was 1 error:

1. Maatwebsite\Excel\Tests\Concerns\FromQueryTest::can_export_from_query
   TypeError: Return value of Maatwebsite\Excel\Tests\Data\Stubs\FromUsersQueryExport::query() must be an instance of Illuminate\Database\Query\Builder, instance of Illuminate\Database\Eloquent\Builder returned

/usr/src/app/tests/Data/Stubs/FromUsersQueryExport.php:17
/usr/src/app/src/Sheet.php:456
/usr/src/app/src/Sheet.php:214
/usr/src/app/src/Writer.php:66
/usr/src/app/src/Excel.php:195
/usr/src/app/src/Excel.php:99
/usr/src/app/src/Concerns/Exportable.php:55
/usr/src/app/tests/Concerns/FromQueryTest.php:53
phpvfscomposer:///usr/src/app/vendor/phpunit/phpunit/phpunit:97

ERRORS!
Tests: 27, Assertions: 50, Errors: 1.

I will open a PR.

[patrickbrouwers]

I thought you wanted to add that return type, adding the other two on the docblock is fine. Larastan never complains for me (highest level), because I declare the typehint myself in my exports.

Merged