Add TChildModel for relations with childs in AddGenericReturnTypeToRelationsRector (#82)

* Move getting and verifying of the relation method call to own method

* Check if method name equals with doesMethodHasName

* Add TChildModel for relations which needs it

* Don't update the docblock if return type already contains the correct generic

This avoids overwriting non-FQCN with our fully qualified ones.

* Add another test

* Apply Rector

* Fix code style

* [ci-review] Rector Rectify

Author: rene-bos

src/NodeAnalyzer/LumenRouteRegisteringMethodAnalyzer.php

@@ -19,9 +19,9 @@ public function __construct(
     ) {
     }
 
-    public function isLumenRoutingClass(MethodCall $node): bool
+    public function isLumenRoutingClass(MethodCall $methodCall): bool
     {
-        return $this->nodeTypeResolver->isObjectType($node->var, new ObjectType('Laravel\Lumen\Routing\Router'));
+        return $this->nodeTypeResolver->isObjectType($methodCall->var, new ObjectType('Laravel\Lumen\Routing\Router'));
     }
 
     public function isRoutesRegisterGroup(Identifier|Expr $name): bool

src/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector.php

@@ -11,20 +11,33 @@
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Return_;
+use PHPStan\Analyser\Scope;
 use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode;
 use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
 use PHPStan\Type\Constant\ConstantStringType;
 use PHPStan\Type\Generic\GenericClassStringType;
+use PHPStan\Type\Generic\GenericObjectType;
 use PHPStan\Type\ObjectType;
 use Rector\BetterPhpDocParser\ValueObject\Type\FullyQualifiedIdentifierTypeNode;
-use Rector\Core\Rector\AbstractRector;
+use Rector\Core\Rector\AbstractScopeAwareRector;
 use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 /** @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorTest */
-class AddGenericReturnTypeToRelationsRector extends AbstractRector
+class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector
 {
+    // Relation methods which are supported by this Rector.
+    private const RELATION_METHODS = [
+        'hasOne', 'hasOneThrough', 'morphOne',
+        'belongsTo', 'morphTo',
+        'hasMany', 'hasManyThrough', 'morphMany',
+        'belongsToMany', 'morphToMany', 'morphedByMany',
+    ];
+
+    // Relation methods which need the class as TChildModel.
+    private const RELATION_WITH_CHILD_METHODS = ['belongsTo', 'morphTo'];
+
     public function __construct(
         private readonly TypeComparator $typeComparator
     ) {
@@ -78,11 +91,12 @@ public function getNodeTypes(): array
         return [ClassMethod::class];
     }
 
-    /**
-     * @param ClassMethod $node
-     */
-    public function refactor(Node $node): ?Node
+    public function refactorWithScope(Node $node, Scope $scope): ?Node
     {
+        if (! $node instanceof ClassMethod) {
+            return null;
+        }
+
         if ($this->shouldSkipNode($node)) {
             return null;
         }
@@ -111,41 +125,46 @@ public function refactor(Node $node): ?Node
         // Don't update an existing return type if it differs from the native return type (thus the one without generics).
         // E.g. we only add generics to an existing return type, but don't change the type itself.
         if (
-            $phpDocInfo->getReturnTagValue() !== null &&
-            ! $this->typeComparator->arePhpParserAndPhpStanPhpDocTypesEqual(
+            $phpDocInfo->getReturnTagValue() !== null
+            && ! $this->areNativeTypeAndPhpDocReturnTypeEqual(
+                $node,
                 $methodReturnType,
                 $phpDocInfo->getReturnTagValue()
-                    ->type,
-                $node
             )
         ) {
             return null;
         }
 
-        $returnStatement = $this->betterNodeFinder->findFirstInFunctionLikeScoped(
-            $node,
-            fn (Node $subNode): bool => $subNode instanceof Return_
-        );
-
-        if (! $returnStatement instanceof Return_) {
+        $relationMethodCall = $this->getRelationMethodCall($node);
+        if (! $relationMethodCall instanceof MethodCall) {
             return null;
         }
 
-        $relationMethodCall = $this->betterNodeFinder->findFirstInstanceOf($returnStatement, MethodCall::class);
+        $relatedClass = $this->getRelatedModelClassFromMethodCall($relationMethodCall);
 
-        if (! $relationMethodCall instanceof MethodCall) {
+        if ($relatedClass === null) {
             return null;
         }
 
-        $relatedClass = $this->getRelatedModelClassFromMethodCall($relationMethodCall);
+        $classForChildGeneric = $this->getClassForChildGeneric($scope, $relationMethodCall);
 
-        if ($relatedClass === null) {
+        // Don't update the docblock if return type already contains the correct generics. This avoids overwriting
+        // non-FQCN with our fully qualified ones.
+        if (
+            $phpDocInfo->getReturnTagValue() !== null
+            && $this->areGenericTypesEqual(
+                $node,
+                $phpDocInfo->getReturnTagValue(),
+                $relatedClass,
+                $classForChildGeneric
+            )
+        ) {
             return null;
         }
 
         $genericTypeNode = new GenericTypeNode(
             new FullyQualifiedIdentifierTypeNode($methodReturnTypeName),
-            [new FullyQualifiedIdentifierTypeNode($relatedClass)],
+            $this->getGenericTypes($relatedClass, $classForChildGeneric),
         );
 
         // Update or add return tag
@@ -161,43 +180,125 @@ public function refactor(Node $node): ?Node
 
     private function getRelatedModelClassFromMethodCall(MethodCall $methodCall): ?string
     {
-        $methodName = $methodCall->name;
+        $argType = $this->getType($methodCall->getArgs()[0]->value);
 
-        if (! $methodName instanceof Identifier) {
+        if ($argType instanceof ConstantStringType) {
+            return $argType->getValue();
+        }
+
+        if (! $argType instanceof GenericClassStringType) {
             return null;
         }
 
-        // Called method should be one of the Laravel's relation methods
-        if (! in_array($methodName->name, [
-            'hasOne', 'hasOneThrough', 'morphOne',
-            'belongsTo', 'morphTo',
-            'hasMany', 'hasManyThrough', 'morphMany',
-            'belongsToMany', 'morphToMany', 'morphedByMany',
-        ], true)) {
+        $modelType = $argType->getGenericType();
+
+        if (! $modelType instanceof ObjectType) {
             return null;
         }
 
-        if (count($methodCall->getArgs()) < 1) {
+        return $modelType->getClassName();
+    }
+
+    private function getRelationMethodCall(ClassMethod $classMethod): ?MethodCall
+    {
+        $node = $this->betterNodeFinder->findFirstInFunctionLikeScoped(
+            $classMethod,
+            fn (Node $subNode): bool => $subNode instanceof Return_
+        );
+
+        if (! $node instanceof Return_) {
             return null;
         }
 
-        $argType = $this->getType($methodCall->getArgs()[0]->value);
+        $methodCall = $this->betterNodeFinder->findFirstInstanceOf($node, MethodCall::class);
 
-        if ($argType instanceof ConstantStringType) {
-            return $argType->getValue();
+        if (! $methodCall instanceof MethodCall) {
+            return null;
         }
 
-        if (! $argType instanceof GenericClassStringType) {
+        // Called method should be one of the Laravel's relation methods
+        if (! $this->doesMethodHasName($methodCall, self::RELATION_METHODS)) {
             return null;
         }
 
-        $modelType = $argType->getGenericType();
+        if (count($methodCall->getArgs()) < 1) {
+            return null;
+        }
 
-        if (! $modelType instanceof ObjectType) {
+        return $methodCall;
+    }
+
+    /**
+     * We need the current class for generics which need a TChildModel. This is the case by for example the BelongsTo
+     * relation.
+     */
+    private function getClassForChildGeneric(Scope $scope, MethodCall $methodCall): ?string
+    {
+        if (! $this->doesMethodHasName($methodCall, self::RELATION_WITH_CHILD_METHODS)) {
             return null;
         }
 
-        return $modelType->getClassName();
+        $classReflection = $scope->getClassReflection();
+
+        return $classReflection?->getName();
+    }
+
+    private function areNativeTypeAndPhpDocReturnTypeEqual(
+        ClassMethod $classMethod,
+        Node $node,
+        ReturnTagValueNode $returnTagValueNode
+    ): bool {
+        $phpDocPHPStanType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType(
+            $returnTagValueNode->type,
+            $classMethod
+        );
+
+        $phpDocPHPStanTypeWithoutGenerics = $phpDocPHPStanType;
+        if ($phpDocPHPStanType instanceof GenericObjectType) {
+            $phpDocPHPStanTypeWithoutGenerics = new ObjectType($phpDocPHPStanType->getClassName());
+        }
+
+        $methodReturnTypePHPStanType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($node);
+
+        return $this->typeComparator->areTypesEqual(
+            $methodReturnTypePHPStanType,
+            $phpDocPHPStanTypeWithoutGenerics,
+        );
+    }
+
+    private function areGenericTypesEqual(
+        Node $node,
+        ReturnTagValueNode $returnTagValueNode,
+        string $relatedClass,
+        ?string $classForChildGeneric
+    ): bool {
+        $phpDocPHPStanType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType(
+            $returnTagValueNode->type,
+            $node
+        );
+
+        if (! $phpDocPHPStanType instanceof GenericObjectType) {
+            return false;
+        }
+
+        $phpDocTypes = $phpDocPHPStanType->getTypes();
+        if ($phpDocTypes === []) {
+            return false;
+        }
+
+        if (! $this->typeComparator->areTypesEqual($phpDocTypes[0], new ObjectType($relatedClass))) {
+            return false;
+        }
+
+        $phpDocHasChildGeneric = count($phpDocTypes) === 2;
+        if ($classForChildGeneric === null && ! $phpDocHasChildGeneric) {
+            return true;
+        }
+
+        if ($classForChildGeneric === null || ! $phpDocHasChildGeneric) {
+            return false;
+        }
+        return $this->typeComparator->areTypesEqual($phpDocTypes[1], new ObjectType($classForChildGeneric));
     }
 
     private function shouldSkipNode(ClassMethod $classMethod): bool
@@ -218,4 +319,31 @@ private function shouldSkipNode(ClassMethod $classMethod): bool
 
         return false;
     }
+
+    /**
+     * @param array<string> $methodNames
+     */
+    private function doesMethodHasName(MethodCall $methodCall, array $methodNames): bool
+    {
+        $methodName = $methodCall->name;
+
+        if (! $methodName instanceof Identifier) {
+            return false;
+        }
+        return in_array($methodName->name, $methodNames, true);
+    }
+
+    /**
+     * @return FullyQualifiedIdentifierTypeNode[]
+     */
+    private function getGenericTypes(string $relatedClass, ?string $childClass): array
+    {
+        $generics = [new FullyQualifiedIdentifierTypeNode($relatedClass)];
+
+        if ($childClass !== null) {
+            $generics[] = new FullyQualifiedIdentifierTypeNode($childClass);
+        }
+
+        return $generics;
+    }
 }

stubs/Illuminate/Database/Eloquent/Relations/BelongsTo.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace Illuminate\Database\Eloquent\Relations;
+
+if (class_exists('Illuminate\Database\Eloquent\Relations\BelongsTo')) {
+    return;
+}
+
+class BelongsTo extends Relation
+{
+}

tests/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector/Fixture/relation-with-child-generics.php.inc

@@ -0,0 +1,40 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+
+class Account extends Model {}
+
+class User extends Model
+{
+    public function accounts(): BelongsTo
+    {
+        return $this->belongsTo(Account::class);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+
+class Account extends Model {}
+
+class User extends Model
+{
+    /**
+     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo<\RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture\Account, \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture\User>
+     */
+    public function accounts(): BelongsTo
+    {
+        return $this->belongsTo(Account::class);
+    }
+}
+
+?>

tests/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector/Fixture/relation-with-existing-child-generic.php.inc

@@ -0,0 +1,43 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+
+class Account extends Model {}
+
+class User extends Model
+{
+    /**
+     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo<\RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture\Account, User>
+     */
+    public function accounts(): BelongsTo
+    {
+        return $this->belongsTo(Account::class);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+
+class Account extends Model {}
+
+class User extends Model
+{
+    /**
+     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo<\RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\Fixture\Account, User>
+     */
+    public function accounts(): BelongsTo
+    {
+        return $this->belongsTo(Account::class);
+    }
+}
+
+?>