Larastan: False positive undefined methods on relations

[bastien-phi]

• Larastan Version: 0.7.2
• --level used: 8

Description

It looks like version 0.7.2 introduced some false-positive errors :

Call to an undefined method Illuminate\Database\Eloquent\Relations\BelongsToMany::orderBy().
Call to an undefined method Illuminate\Database\Eloquent\Relations\MorphMany::where().    
Call to an undefined method Illuminate\Database\Eloquent\Relations\MorphMany::whereIn().
...

Laravel code where the issue was found

Some examples :

$user->load(['garages' => fn (BelongsToMany $builder) => $builder->orderBy('name')]);

$comments = $folder->comments()
            ->orderByDesc('created_at')
            ->with('media', fn (MorphMany $builder) => $builder->where('collection_name', CommentMedia::file()))
            ->get();

[szepeviktor]

You need to add generics to the return types of those methods.
Just did it in https://github.com/szepeviktor/bazar/commit/7826a551373fca3d6add3092ae5b6fe8a1c1aaf9

[szepeviktor]

fn (BelongsToMany $builder) => $builder->orderBy('name')

Bizarre code! :) I may be too old. 👴 I still type _unctio_ in the middle of fn 😄

[canvural]

Hi,

@szepeviktor That's not the reason for the error. The error is because all relations are made generic in Larastan. And when you typehint BelongsToMany $builder you don't specify the generic type, so the error occurs.

Unfortunately, PHPStan does not read docblocks above closures yet. That could solve your problem.

For now you could remove the typehint from the fn (BelongsToMany $builder) (so just fn ($builder)) or you can ignore the error.

I'm gonna keep this issue open either until https://github.com/phpstan/phpstan/issues/3770 is resolved, or we find another solution.

[szepeviktor]

I see using methods instead of Closures the sustainable way.

[osteel]

In case it might help: 0.7.2 introduced the same kind of false positives for me as well.

Example of new error:

Call to an undefined method Illuminate\Database\Eloquent\Relations\BelongsToMany::where()

Code where it happens:

/**
 * @param  Company $company
 * @return bool
 */
public function isOwner(Company $company): bool
{
    return $company->projects()->where(['project_id' => $this->id, 'is_owner' => true])->exists();
}

Definition of Company@projects:

/**
 * @return BelongsToMany
 */
public function projects(): BelongsToMany
{
    ...
}

If I revert to 0.7.1 the error disappears. Bummer because I want to try 0.7.3's Octane check 😉

[canvural]

@osteel BelongsToMany is generic in Larastan. And also Larastan can infer these generic parameters on its own. So if you remove

/**
 * @return BelongsToMany
 */

I beleve your issue will be solved. Having the retrn type in the method signature is enough.

[osteel]

@canvural I removed the projects method's docblock but Larastan still flags the same issue I'm afraid

[canvural]

@osteel There must be something else wrong going on. Can you add \PHPStan\dumpType($company->projects()) somewhere in code and share the result? Result should be something like Illuminate\Database\Eloquent\Relations\BelongsToMany<App\Project>

[osteel]

Larastan 0.7.2:

 ------ --------------------------------------------------------------------------------------------
  Line   Models/Project.php
 ------ --------------------------------------------------------------------------------------------
  184    Dumped type: Illuminate\Database\Eloquent\Relations\BelongsToMany
  185    Call to an undefined method Illuminate\Database\Eloquent\Relations\BelongsToMany::where().
 ------ --------------------------------------------------------------------------------------------

Larastan 0.7.1:

 ------ -------------------------------------------------------------------
  Line   Models/Project.php
 ------ -------------------------------------------------------------------
  184    Dumped type: Illuminate\Database\Eloquent\Relations\BelongsToMany
 ------ -------------------------------------------------------------------

[canvural]

Is the relation defined in trait or something? Anyway in that case you can go back to using docblocks but you need to use @return BelongsToMany<Project>

[osteel]

@canvural that worked 👍

Does that mean Larastan also reads the body of the method? If that's the case, I guess I should haven mentioned that Company is an abstract class, and that this is the actual method definition:

/**
 * The company's projects.
 *
 * @return BelongsToMany
 */
abstract public function projects(): BelongsToMany;

I've now changed it for BelongsToMany<Project> and that seems to have done the trick.

[canvural]

Yes, Larastan reads the method body of relation methods. And tries to find return $this->belongsToMany(Project::class) to infer the generic type for the class. So yeah if the method is abstract you need the docblock with the generic.

[osteel]

@canvural makes sense, thanks for the explanation

[stayallive]

I think I have an issue that is related, since upgrading we are getting:

// Called 'modelKeys' on Laravel collection, but could have been retrieved as a query. 
$keys = $user->workgroups()->wherePivot('has_accepted', '=', true)->get()->modelKeys();

$user->workgroups() is a BelongsToMany relation, I have tried with BelongsToMany type hints, docblocks and adding generics but the errors isn't going anywhere.

Changing the code to (removing the wherePivot) fixes it, but that is of course not feasable:

$keys = $user->workgroups()->get()->modelKeys();

[szepeviktor]

BelongsToMany needs one related model.
https://github.com/nunomaduro/larastan/blob/053b9a4303a4cf51f986556a7e8076084e634d2b/stubs/BelongsToMany.stub#L6
It works! :)
https://github.com/szepeviktor/bazar/commit/7826a551373fca3d6add3092ae5b6fe8a1c1aaf9

[canvural]

Hi,

This should now be fixed as a side effect of #816 Although the error would still happen if you try to get the model from the relation. The issue referenced at https://github.com/nunomaduro/larastan/issues/804#issuecomment-815791796 needs to be solved in PHPStan, to completly solve this I guess.

[bastien-phi]

Thanks @canvural, it works perfectly !