Custom builder doc blocks are not parsed as builder generic

[BertvanHoekelen]

• Larastan Version: 0.5.2
• --level used: max

The behavior changed with PR #457

Description

Calling collection methods on a result collection gives errors like Cannot call method pluck() on array<Illuminate\Database\Eloquent\Builder<mixed>>|Illuminate\Database\Eloquent\Collection.

This also happens to count(), contains(), avg(), first().

Laravel code where the issue was found

<?php

declare(strict_types=1);

namespace Tests\Unit;

use App\Models\Transaction;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;

class Topic extends Model
{
    public function categories(): HasMany
    {
        return $this->hasMany(Category::class);
    }
}

class Category extends Model
{
    public function topics(): HasMany
    {
        return $this->hasMany(Topic::class);
    }
}

$categoryIds = Transaction::query()
    ->groupBy('category_id')
    ->get(['category_id'])
    ->pluck('category_id');

[canvural]

Hi,

I cannot reproduce this. Added this

/** @return \Illuminate\Support\Collection<User> */
public function testBug(): \Illuminate\Support\Collection
{
    return User::query()
        ->groupBy('category_id')
        ->get(['category_id'])
        ->pluck('category_id');
}

to the ReturnTypes/BuilderExtension.php tests

Can you give more info or make a PR with failing test?

[BertvanHoekelen]

Ah sorry was testing it on my own model. Which has the following docblocks.

 * @method static \App\Models\Builders\TransactionBuilder newModelQuery()
 * @method static \App\Models\Builders\TransactionBuilder newQuery()
 * @method static \App\Models\Builders\TransactionBuilder query()

If I change those to the following larastan works but my auto complete in my IDE is gone :(

 * @method static \App\Models\Builders\TransactionBuilder<Transaction> newModelQuery()
 * @method static \App\Models\Builders\TransactionBuilder<Transaction> newQuery()
 * @method static \App\Models\Builders\TransactionBuilder<Transaction> query()

[szepeviktor]

Your IDE does not support array<types>?

[canvural]

There is nothing we can do about that. Unless PHPStan also has some language server like Psalm.

[BertvanHoekelen]

Your IDE does not support array<types>?

Nope PHPStorm does not support the following.

/*
 * @method static \App\Models\Builders\TransactionBuilder<Transaction> newModelQuery()
 * @method static \App\Models\Builders\TransactionBuilder<Transaction> newQuery()
 * @method static \App\Models\Builders\TransactionBuilder<Transaction> query()
 */
class Transaction extends Model
{
}

Also the ide helper generates.

/*
 * @method static \App\Models\Builders\TransactionBuilder newModelQuery()
 * @method static \App\Models\Builders\TransactionBuilder newQuery()
 * @method static \App\Models\Builders\TransactionBuilder query()
 */
class Transaction extends Model
{
}

To bad, without proper ide support I'm unable to continue using PHPStan :(

[canvural]

Because it's not an array Its generic class.

Why do you need to put the docblocks anyway? Larastan should be able to undersand that.

[canvural]

Also, you can use @phpstan-method together with your normal docblocks.

[BertvanHoekelen]

Because it's not an array Its generic class.

Why do you need to put the docblocks anyway? Larastan should be able to undersand that.

Because PHPStorm otherwise doesn't understand :(

Also, you can use @phpstan-method together with your normal docblocks.

That also doesn't work. No matter the order of methods.

 * @method static \App\Models\Builders\TransactionBuilder newModelQuery()
 * @method static \App\Models\Builders\TransactionBuilder newQuery()
 * @phpstan-method static \App\Models\Builders\TransactionBuilder<Transaction> query()
 * @method static \App\Models\Builders\TransactionBuilder query()

[canvural]

@phpstan-method doesn't seem to work correctly. Maybe you can open an issue there, with a small example at phpstan.org

[BertvanHoekelen]

@canvural Can we reopen this so that we can implement something like #479 for this please?

[canvural]

@BertvanHoekelen I'm not sure if it is possible to do something like that here. But we can discuss, sure.

So what would be the rule to convert the docblocks?

[BertvanHoekelen]

Well the ide-helper generates(and is correctly read by the ide):
* @method static \App\Models\Builders\CustomBuilder|\App\Models\ModelWitthCustomBuilder query() which should be interpreted by larastan as @method static \App\Models\Builders\CustomBuilder<ModelWithCustomBuilder>.

Or if there is no custom builder:
* @method static \Illuminate\Database\Eloquent\Builder|\App\Models\Foo query() as @method static Builder<\App\Models\Foo>. This is basically the same as \Illuminate\Database\Eloquent\Collection|\App\Account[] $accounts to \Illuminate\Database\Eloquent\Collection<\App\Account> $accounts

[canvural]

But your original issue was with /** @method static \App\Models\Builders\TransactionBuilder newModelQuery() */ Was this generated by ide-helper or by you?

[BertvanHoekelen]

That one was generated by myself before installing the ide-helper to do it for me, my apologies. If I now create a new model called Foo and run the ide-helper it would look like:

<?php

namespace App;

use Illuminate\Database\Eloquent\Model;

/**
 * App\Foo
 *
 * @method static \Illuminate\Database\Eloquent\Builder|\App\Foo newModelQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|\App\Foo newQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|\App\Foo query()
 * @mixin \Eloquent
 */
class Foo extends Model
{
    //
}

[canvural]

@mr-feek Do you also want to take this one?

[mr-feek]

Sure, I can take a look

[BertvanHoekelen]

That would be awesome @mr-feek.Thanks! If not I can give it a shot on Monday.

[mr-feek]

Happy to help, but I can't reproduce. Feel free to push up a failing test case for me.

I added the following to App\User.php

/**
 * @property-read \Illuminate\Database\Eloquent\Collection|\App\Account[] $accounts
 * @method static \Illuminate\Database\Eloquent\Builder|\App\User newModelQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|\App\User newQuery()
 * @method static \Illuminate\Database\Eloquent\Builder|\App\User query()
 */
class User extends Authenticatable

then i made a test

<?php

namespace Tests\Features\Models;

use App\User;

class Queries
{
    /** @test */
    public function it_reproduces_461(): void
    {
        User::query()->pluck('foo');
    }
}

And phpstan passed on max

[canvural]

The issue is not about calling the method on it, but to get the model after calling get So I believe User::query()->where('foo', 'bar')->get() would give you the error with those docblocks.

And the extension we need is almost the same as the #479 We want to convert \Illuminate\Database\Eloquent\Builder|\App\User into \Illuminate\Database\Eloquent\Builder<\App\User>

[BertvanHoekelen]

Hey @mr-feek,

This test fails with:

Method Tests\\Features\\ReturnTypes\\CustomEloquentBuilderTest1::testGetModelFromModelWithCustomBuilderQuery() should return Illuminate\\Database\\Eloquent\\Collection<Tests\\Features\\ReturnTypes\\ModelWithCustomBuilderAndDocBlocks> but returns Illuminate\\Database\\Eloquent\\Collection<Illuminate\\Database\\Eloquent\\static>|Tests\\Features\\ReturnTypes\\CustomBuilder2<Tests\\Features\\ReturnTypes\\ModelWithCustomBuilderAndDocBlocks>.

<?php

declare(strict_types=1);

namespace Tests\Features\ReturnTypes;

use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;

class CustomEloquentBuilderTest1
{
    /**
     * @return \Illuminate\Database\Eloquent\Collection<\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks>
     */
    public function testGetModelFromModelWithCustomBuilderQuery()
    {
        return ModelWithCustomBuilderAndDocBlocks::query()->get();
    }
}

/**
 * @method static \Tests\Features\ReturnTypes\CustomBuilder2|\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks newModelQuery()
 * @method static \Tests\Features\ReturnTypes\CustomBuilder2|\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks newQuery()
 * @method static \Tests\Features\ReturnTypes\CustomBuilder2|\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks query()
 */
class ModelWithCustomBuilderAndDocBlocks extends Model
{
    /**
     * @param \Illuminate\Database\Query\Builder $query
     *
     * @return CustomBuilder2<\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks>
     */
    public function newEloquentBuilder($query): CustomBuilder2
    {
        return new CustomBuilder2($query);
    }
}

/**
 * @template TModelClass
 * @extends Builder<TModelClass>
 */
class CustomBuilder2 extends Builder
{

}

If the dockblocks of ModelWithCustomBuilderAndDocBlocks are changed to the following the test succeeds:

/**
 * @method static \Tests\Features\ReturnTypes\CustomBuilder2<\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks> newModelQuery()
 * @method static \Tests\Features\ReturnTypes\CustomBuilder2<\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks> newQuery()
 * @method static \Tests\Features\ReturnTypes\CustomBuilder2<\Tests\Features\ReturnTypes\ModelWithCustomBuilderAndDocBlocks> query()
 */