feat: adjust AddGenericReturnTypeToRelationsRector rule to be able to…

… generate new generic style code (#263)

* feat: adjust AddGenericReturnTypeToRelationsRector rule to be able to generate new generic style code

* make rule configuration default to false to keep BC

docs/rector_rules_overview.md

@@ -63,6 +63,8 @@ Adds the `@extends` annotation to Factories.
 
 Add generic return type to relations in child of `Illuminate\Database\Eloquent\Model`
 
+:wrench: **configure it!**
+
 - class: [`RectorLaravel\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector`](../src/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector.php)
 
 ```diff
@@ -82,6 +84,23 @@ Add generic return type to relations in child of `Illuminate\Database\Eloquent\M
 
 <br>
 
+```diff
+ use App\Account;
+ use Illuminate\Database\Eloquent\Model;
+ use Illuminate\Database\Eloquent\Relations\HasMany;
+
+ class User extends Model
+ {
++    /** @return HasMany<Account, $this> */
+     public function accounts(): HasMany
+     {
+         return $this->hasMany(Account::class);
+     }
+ }
+```
+
+<br>
+
 ## AddGuardToLoginEventRector
 
 Add new `$guard` argument to Illuminate\Auth\Events\Login

src/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector.php

@@ -12,23 +12,30 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode;
 use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
+use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Type\Constant\ConstantStringType;
 use PHPStan\Type\Generic\GenericClassStringType;
 use PHPStan\Type\Generic\GenericObjectType;
 use PHPStan\Type\ObjectType;
+use PHPStan\Type\ThisType;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
 use Rector\BetterPhpDocParser\ValueObject\Type\FullyQualifiedIdentifierTypeNode;
 use Rector\Comments\NodeDocBlock\DocBlockUpdater;
+use Rector\Contract\Rector\ConfigurableRectorInterface;
 use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
 use Rector\PhpParser\Node\BetterNodeFinder;
 use Rector\Rector\AbstractScopeAwareRector;
 use Rector\StaticTypeMapper\StaticTypeMapper;
-use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+use Webmozart\Assert\Assert;
 
-/** @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorTest */
-class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector
+/**
+ * @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorNewGenericsTest
+ * @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorOldGenericsTest
+ */
+class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector implements ConfigurableRectorInterface
 {
     // Relation methods which are supported by this Rector.
     private const RELATION_METHODS = [
@@ -41,6 +48,11 @@ class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector
     // Relation methods which need the class as TChildModel.
     private const RELATION_WITH_CHILD_METHODS = ['belongsTo', 'morphTo'];
 
+    // Relation methods which need the class as TIntermediateModel.
+    private const RELATION_WITH_INTERMEDIATE_METHODS = ['hasManyThrough', 'hasOneThrough'];
+
+    private bool $shouldUseNewGenerics = false;
+
     public function __construct(
         private readonly TypeComparator $typeComparator,
         private readonly DocBlockUpdater $docBlockUpdater,
@@ -55,7 +67,7 @@ public function getRuleDefinition(): RuleDefinition
         return new RuleDefinition(
             'Add generic return type to relations in child of Illuminate\Database\Eloquent\Model',
             [
-                new CodeSample(
+                new ConfiguredCodeSample(
                     <<<'CODE_SAMPLE'
 use App\Account;
 use Illuminate\Database\Eloquent\Model;
@@ -84,8 +96,39 @@ public function accounts(): HasMany
         return $this->hasMany(Account::class);
     }
 }
+CODE_SAMPLE,
+                    ['shouldUseNewGenerics' => false]),
+                new ConfiguredCodeSample(
+                    <<<'CODE_SAMPLE'
+use App\Account;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasMany;
+
+class User extends Model
+{
+    public function accounts(): HasMany
+    {
+        return $this->hasMany(Account::class);
+    }
+}
 CODE_SAMPLE
-                ),
+
+                    ,
+                    <<<'CODE_SAMPLE'
+use App\Account;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasMany;
+
+class User extends Model
+{
+    /** @return HasMany<Account, $this> */
+    public function accounts(): HasMany
+    {
+        return $this->hasMany(Account::class);
+    }
+}
+CODE_SAMPLE,
+                    ['shouldUseNewGenerics' => true]),
             ]
         );
     }
@@ -154,6 +197,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
         }
 
         $classForChildGeneric = $this->getClassForChildGeneric($scope, $relationMethodCall);
+        $classForIntermediateGeneric = $this->getClassForIntermediateGeneric($relationMethodCall);
 
         // Don't update the docblock if return type already contains the correct generics. This avoids overwriting
         // non-FQCN with our fully qualified ones.
@@ -163,15 +207,16 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
                 $node,
                 $phpDocInfo->getReturnTagValue(),
                 $relatedClass,
-                $classForChildGeneric
+                $classForChildGeneric,
+                $classForIntermediateGeneric
             )
         ) {
             return null;
         }
 
         $genericTypeNode = new GenericTypeNode(
             new FullyQualifiedIdentifierTypeNode($methodReturnTypeName),
-            $this->getGenericTypes($relatedClass, $classForChildGeneric),
+            $this->getGenericTypes($relatedClass, $classForChildGeneric, $classForIntermediateGeneric),
         );
 
         // Update or add return tag
@@ -187,6 +232,24 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
         return $node;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public function configure(array $configuration): void
+    {
+        if ($configuration === []) {
+            $this->shouldUseNewGenerics = false;
+
+            return;
+        }
+
+        Assert::count($configuration, 1);
+        Assert::keyExists($configuration, 'shouldUseNewGenerics');
+        Assert::boolean($configuration['shouldUseNewGenerics']);
+
+        $this->shouldUseNewGenerics = $configuration['shouldUseNewGenerics'];
+    }
+
     private function getRelatedModelClassFromMethodCall(MethodCall $methodCall): ?string
     {
         $argType = $this->getType($methodCall->getArgs()[0]->value);
@@ -243,6 +306,10 @@ private function getRelationMethodCall(ClassMethod $classMethod): ?MethodCall
      */
     private function getClassForChildGeneric(Scope $scope, MethodCall $methodCall): ?string
     {
+        if ($this->shouldUseNewGenerics) {
+            return null;
+        }
+
         if (! $this->doesMethodHasName($methodCall, self::RELATION_WITH_CHILD_METHODS)) {
             return null;
         }
@@ -252,6 +319,45 @@ private function getClassForChildGeneric(Scope $scope, MethodCall $methodCall):
         return $classReflection?->getName();
     }
 
+    /**
+     * We need the intermediate class for generics which need a TIntermediateModel.
+     * This is the case for *through relations
+     */
+    private function getClassForIntermediateGeneric(MethodCall $methodCall): ?string
+    {
+        if (! $this->shouldUseNewGenerics) {
+            return null;
+        }
+
+        if (! $this->doesMethodHasName($methodCall, self::RELATION_WITH_INTERMEDIATE_METHODS)) {
+            return null;
+        }
+
+        $args = $methodCall->getArgs();
+
+        if (count($args) < 2) {
+            return null;
+        }
+
+        $argType = $this->getType($args[1]->value);
+
+        if ($argType instanceof ConstantStringType && $argType->isClassStringType()->yes()) {
+            return $argType->getValue();
+        }
+
+        if (! $argType instanceof GenericClassStringType) {
+            return null;
+        }
+
+        $modelType = $argType->getGenericType();
+
+        if (! $modelType instanceof ObjectType) {
+            return null;
+        }
+
+        return $modelType->getClassName();
+    }
+
     private function areNativeTypeAndPhpDocReturnTypeEqual(
         ClassMethod $classMethod,
         Node $node,
@@ -279,7 +385,8 @@ private function areGenericTypesEqual(
         Node $node,
         ReturnTagValueNode $returnTagValueNode,
         string $relatedClass,
-        ?string $classForChildGeneric
+        ?string $classForChildGeneric,
+        ?string $classForIntermediateGeneric
     ): bool {
         $phpDocPHPStanType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType(
             $returnTagValueNode->type,
@@ -299,16 +406,37 @@ private function areGenericTypesEqual(
             return false;
         }
 
-        $phpDocHasChildGeneric = count($phpDocTypes) === 2;
-        if ($classForChildGeneric === null && ! $phpDocHasChildGeneric) {
-            return true;
+        if (! $this->shouldUseNewGenerics) {
+            $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));
+        }
+
+        $phpDocHasIntermediateGeneric = count($phpDocTypes) === 3;
+
+        if ($classForIntermediateGeneric === null && ! $phpDocHasIntermediateGeneric) {
+            // If there is only one generic, it means method is using the old format. We should update it.
+            if (count($phpDocTypes) === 1) {
+                return false;
+            }
+
+            // We want to convert the existing relationship definition to use `$this` as the second generic
+            return $phpDocTypes[1] instanceof ThisType;
         }
 
-        if ($classForChildGeneric === null || ! $phpDocHasChildGeneric) {
+        if ($classForIntermediateGeneric === null || ! $phpDocHasIntermediateGeneric) {
             return false;
         }
 
-        return $this->typeComparator->areTypesEqual($phpDocTypes[1], new ObjectType($classForChildGeneric));
+        return $this->typeComparator->areTypesEqual($phpDocTypes[1], new ObjectType($classForIntermediateGeneric));
     }
 
     private function shouldSkipNode(ClassMethod $classMethod, Scope $scope): bool
@@ -341,16 +469,24 @@ private function doesMethodHasName(MethodCall $methodCall, array $methodNames):
     }
 
     /**
-     * @return FullyQualifiedIdentifierTypeNode[]
+     * @return IdentifierTypeNode[]
      */
-    private function getGenericTypes(string $relatedClass, ?string $childClass): array
+    private function getGenericTypes(string $relatedClass, ?string $childClass, ?string $intermediateClass): array
     {
         $generics = [new FullyQualifiedIdentifierTypeNode($relatedClass)];
 
-        if ($childClass !== null) {
+        if (! $this->shouldUseNewGenerics && $childClass !== null) {
             $generics[] = new FullyQualifiedIdentifierTypeNode($childClass);
         }
 
+        if ($this->shouldUseNewGenerics) {
+            if ($intermediateClass !== null) {
+                $generics[] = new FullyQualifiedIdentifierTypeNode($intermediateClass);
+            }
+
+            $generics[] = new IdentifierTypeNode('$this');
+        }
+
         return $generics;
     }
 }

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

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

tests/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector/AddGenericReturnTypeToRelationsRectorNewGenericsTest.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class AddGenericReturnTypeToRelationsRectorNewGenericsTest extends AbstractRectorTestCase
+{
+    public static function provideData(): Iterator
+    {
+        //        yield [__DIR__ . '/Fixture/NewGenerics/has-one-through.php.inc'];
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture/NewGenerics');
+    }
+
+    /**
+     * @test
+     */
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/use_new_generics_configured_rule.php';
+    }
+}

tests/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector/AddGenericReturnTypeToRelationsRectorTest.php → tests/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector/AddGenericReturnTypeToRelationsRectorOldGenericsTest.php

@@ -8,11 +8,11 @@
 use PHPUnit\Framework\Attributes\DataProvider;
 use Rector\Testing\PHPUnit\AbstractRectorTestCase;
 
-final class AddGenericReturnTypeToRelationsRectorTest extends AbstractRectorTestCase
+final class AddGenericReturnTypeToRelationsRectorOldGenericsTest extends AbstractRectorTestCase
 {
     public static function provideData(): Iterator
     {
-        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture/OldGenerics');
     }
 
     /**
@@ -26,6 +26,6 @@ public function test(string $filePath): void
 
     public function provideConfigFilePath(): string
     {
-        return __DIR__ . '/config/configured_rule.php';
+        return __DIR__ . '/config/use_old_generics_configured_rule.php';
     }
 }