Larastan: PHPDoc for relation is ignored

Created on 2 Apr 2020  路  11Comments  路  Source: nunomaduro/larastan

  • Larastan Version: master [https://github.com/nunomaduro/larastan/commit/842e18cdc12d56d95ef90a13e72bfa33d0b3ea1a]
  • --level used: max

Description

Sometimes traits are used to define generic relations on a model. Larastan will in this case infer the type of the relation as Relation<Model>. Larastan should read the PHPDoc on the model to determine the type of the relation but the PHPDoc is ignored.

Laravel code where the issue was found

<?php

declare(strict_types=1);

namespace Tests\Features\ReturnTypes;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;

class RelationTest
{
    public function testRelation(): ?Person
    {
        $person = new Person();

        return $person->parent;
    }
}
/**
 * @property Person|null $parent <== this should be used by Larastan but it is ignored.
 */
class Person extends Model
{
    // This method is usually defined in a trait.
    public function parent(): BelongsTo
    {
        // Seen as BelongsTo<Model>
        return $this->belongsTo(get_class($this));
    }
}

Error thrown:

Method Tests\\Features\\ReturnTypes\\RelationTest::testRelation() should return Tests\\Features\\ReturnTypes\\Person|null but returns Illuminate\\Database\\Eloquent\\Model|null.
bug

Most helpful comment

Fixed in https://github.com/nunomaduro/larastan/commit/ec22906dba63325b21c1ac2640879dfd55a1394f

If the property found in annotations, extension will not be run. So PHPStan will get the type from the annotation.

All 11 comments

@Daanra are you only encountering this issue when using traits?

@mr-feek It is not related to traits. It is because of get_class($this) part. We are trying to see what is the first argument to the relation model to use that for the generic class. I thought it'd be only string. But seems we need to evaluate the get_class($this) to get the type. Which I don't know if it's possible.

@canvural The first argument might also be a function call that retrieves the appropriate class name from the config.

For example, the laravel-permission library defines the following relation:

/**
 * A model may have multiple direct permissions.
 */
public function permissions(): MorphToMany
{
        return $this->morphToMany(
            config('permission.models.permission'),
            'model',
            config('permission.table_names.model_has_permissions'),
            config('permission.column_names.model_morph_key'),
            'permission_id'
        );
}

See https://github.com/spatie/laravel-permission/blob/551aa2f950e71505d05445bb63d84dd494d86165/src/Traits/HasPermissions.php#L40

The same issue arises there.

But that cannot be statically determined. The result depends on a function call. Which is dynamic. So, I'd consider that an edge case. And that situation might be solved with stub files in the user code, not in Larastan.

Yeah, that's why it would be nice if Larastan keeps inferring the type of the relation as Model if it can not be statically determined. But if there exists a @property PHPDoc annotation for that relation it should use that instead.

I believe Larastan did this in the previous version but right now the @property tag is ignored.

I'm having the same issues. Just giving my two cents here: @Daanra , I agree with you, would be nice if the PHPDoc was the priority, Psalm does that. :pray:

@Daanra you can use static::class instead of get_class($this). I think PHPStan can infer the type this way.

Nevertheless, the issue still exists. For example:

<?php

/**
 * @property-read Author $author
 */
class Book extends Model
{
    public function author(): BelongsTo
    {
        return $this->belongsTo(Author::class);
    }
}

$book->author->name;

When calling $book->author->name PHPstan reports Cannot access property $name on App\Author|null.

This is related to how PHPStan works. It has an order to execute the extensions. Any 3rd party extension is executed first in order, and the extension that reads the annotations is executed later.

So for the original code example in the issue, Larastan founds the parent property, so extension that reads the annotations is not executed.

We can do couple of things here:

  • We can say we found the property only if we can read the actual model. So, in that case, the annotation extension would run and find the parent property.
  • We can run the AnnotationExtension manually ourselves first. And trust the user completely to write the correct annotation. This can lead to some possible false negatives as in @sebdesign's case. $book->author would actually return null if there is no relationship that exists in the database. So if we were to trust the PHPDoc there, we would always expect an Author as a result.

@canvural I believe is your second point makes sense.

If the user has added a @property annotation on the model, larastan should trust that. If no annotation is found, the extension should infer the property type from the relationship. The type would be nullable of course, even if that leads to false negatives.

Fixed in https://github.com/nunomaduro/larastan/commit/ec22906dba63325b21c1ac2640879dfd55a1394f

If the property found in annotations, extension will not be run. So PHPStan will get the type from the annotation.

@canvural You rock! Thank you so much!

Was this page helpful?
0 / 5 - 0 ratings