driftingly / rector-laravel

Rector upgrades rules for Laravel
http://getrector.org
MIT License
500 stars 49 forks source link

AddGenericReturnTypeToRelationsRector incorrectly handles a morphTo relation #157

Closed Luukdewaaier closed 8 months ago

Luukdewaaier commented 8 months ago

Given the following code (with correct generics):

class Translations extends Model
{
    /**
     * @return MorphTo<Model,  \App\Models\Translations\Translations>
     */
    public function translatable(): MorphTo
    {
        return $this->morphTo('translatable', 'model', 'value');
    }
}

The rule changes it to the following:

class Translations extends Model
{
    /**
     * @return MorphTo<\translatable, \App\Models\Translations\Translations>
     */
    public function translatable(): MorphTo
    {
        return $this->morphTo('translatable', 'model', 'value');
    }
}

Which in return gives problems with Larastan. This is because it is returned by the function getRelatedModelClassFromMethodCall. I can fix it rewriting it to the following (but I guess this breaks other stuff?):

    private function getRelatedModelClassFromMethodCall(MethodCall $methodCall): ?string
    {
        $argType = $this->getType($methodCall->getArgs()[0]->value);

        if (! $argType instanceof GenericClassStringType) {
            return null;
        }

        if ($argType instanceof ConstantStringType) {
            return $argType->getValue();
        }

        $modelType = $argType->getGenericType();

        if (! $modelType instanceof ObjectType) {
            return null;
        }

        return $modelType->getClassName();
    }

What should be the correct fix for this problem? Maybe add a check if the given relation is a morphTo?

canvural commented 8 months ago

I think the rule should skip this method, because it already has a docblock with generic return type.

Luukdewaaier commented 8 months ago

Hi, I have just re-tested this. After running the Rector command it changes te docblock.