Use normal Collection<ClassName> when property has indexBy (#359)

* Use normal array<Collection> when property has indexBy * fix phpstan * fix phpstan
rectorphp · Dec 20, 2024 · 22bc264 · 22bc264
1 parent a0ab15c
commit 22bc264
Show file tree

Hide file tree

Showing 9 changed files with 174 additions and 10 deletions.
diff --git a/...oCollectionPropertyGetterByToManyAnnotationRector/Fixture/OneToMany/with_index_by.php.inc b/...oCollectionPropertyGetterByToManyAnnotationRector/Fixture/OneToMany/with_index_by.php.inc
@@ -0,0 +1,62 @@
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Fixture\OneToMany;
+
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Source\Training;
+
+#[ORM\Entity()]
+class Trainer
+{
+    /**
+     * @var Collection|Training[]
+     */
+    #[ORM\OneToMany(targetEntity: Training::class, indexBy: "id", mappedBy: "trainer")]
+    private $trainings = [];
+
+    public function getTrainings()
+    {
+        return $this->trainings;
+    }
+
+    public function setTrainings($trainings)
+    {
+        $this->trainings = $trainings;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Fixture\OneToMany;
+
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Source\Training;
+
+#[ORM\Entity()]
+class Trainer
+{
+    /**
+     * @var Collection|Training[]
+     */
+    #[ORM\OneToMany(targetEntity: Training::class, indexBy: "id", mappedBy: "trainer")]
+    private $trainings = [];
+
+    /**
+     * @return \Doctrine\Common\Collections\Collection<\Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Source\Training>
+     */
+    public function getTrainings()
+    {
+        return $this->trainings;
+    }
+
+    public function setTrainings($trainings)
+    {
+        $this->trainings = $trainings;
+    }
+}
+
+?>
diff --git a/..._/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector/Source/Training.php b/..._/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector/Source/Training.php
@@ -4,7 +4,11 @@
 
 namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Source;
 
+use Doctrine\ORM\Mapping as ORM;
+
 final class Training
 {
-
+    #[ORM\Column(name: 'id', type: 'string')]
+    #[ORM\Id]
+    private string $id;
 }
diff --git a/...ty/ImproveDoctrineCollectionDocTypeInEntityRector/Fixture/OneToMany/with_index_by.php.inc b/...ty/ImproveDoctrineCollectionDocTypeInEntityRector/Fixture/OneToMany/with_index_by.php.inc
@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany;
+
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training;
+
+/**
+ * @ORM\Entity
+ */
+class WithIndexBy
+{
+    /**
+     * @ORM\OneToMany(targetEntity=Training::class, mappedBy="trainer", "indexBy"="id")
+     * @var Collection<int, string>|Training[]
+     */
+    private $trainings = [];
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany;
+
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training;
+
+/**
+ * @ORM\Entity
+ */
+class WithIndexBy
+{
+    /**
+     * @ORM\OneToMany(targetEntity=Training::class, mappedBy="trainer", "indexBy"="id")
+     * @var \Doctrine\Common\Collections\Collection<\Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training>
+     */
+    private $trainings = [];
+}
+
+?>
diff --git a/...ity/Rector/Class_/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector.php b/...ity/Rector/Class_/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector.php
@@ -126,7 +126,10 @@ public function refactor(Node $node): ?Node
 
             // update docblock with known collection type
             $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);
-            $newVarType = $this->collectionTypeFactory->createType($collectionObjectType);
+            $newVarType = $this->collectionTypeFactory->createType(
+                $collectionObjectType,
+                $this->collectionTypeResolver->hasIndexBy($property)
+            );
             $this->phpDocTypeChanger->changeReturnType($classMethod, $phpDocInfo, $newVarType);
             $hasChanged = true;
         }

diff --git a/rules/CodeQuality/Rector/Property/ImproveDoctrineCollectionDocTypeInEntityRector.php b/rules/CodeQuality/Rector/Property/ImproveDoctrineCollectionDocTypeInEntityRector.php
@@ -208,15 +208,21 @@ private function refactorPropertyPhpDocInfo(Property $property, PhpDocInfo $phpD
                 return null;
             }
 
-            $newVarType = $this->collectionTypeFactory->createType($collectionObjectType);
+            $newVarType = $this->collectionTypeFactory->createType(
+                $collectionObjectType,
+                $this->collectionTypeResolver->hasIndexBy($property)
+            );
             $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $newVarType);
         } else {
             $collectionObjectType = $this->collectionTypeResolver->resolveFromToManyProperty($property);
             if (! $collectionObjectType instanceof FullyQualifiedObjectType) {
                 return null;
             }
 
-            $newVarType = $this->collectionTypeFactory->createType($collectionObjectType);
+            $newVarType = $this->collectionTypeFactory->createType(
+                $collectionObjectType,
+                $this->collectionTypeResolver->hasIndexBy($property)
+            );
             $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $newVarType);
         }
 
@@ -249,7 +255,10 @@ private function refactorAttribute(Expr $expr, PhpDocInfo $phpDocInfo, Property
 
         $fullyQualifiedObjectType = new FullyQualifiedObjectType($targetEntityClassName);
 
-        $genericObjectType = $this->collectionTypeFactory->createType($fullyQualifiedObjectType);
+        $genericObjectType = $this->collectionTypeFactory->createType(
+            $fullyQualifiedObjectType,
+            $this->collectionTypeResolver->hasIndexBy($property)
+        );
         $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $genericObjectType);
 
         return $property;

diff --git a/rules/CodeQuality/SetterCollectionResolver.php b/rules/CodeQuality/SetterCollectionResolver.php
@@ -16,6 +16,7 @@
 use PHPStan\Type\UnionType;
 use Rector\Doctrine\CodeQuality\Enum\DoctrineClass;
 use Rector\Doctrine\TypeAnalyzer\CollectionTypeFactory;
+use Rector\Doctrine\TypeAnalyzer\CollectionTypeResolver;
 use Rector\Doctrine\TypeAnalyzer\CollectionVarTagValueNodeResolver;
 use Rector\NodeManipulator\AssignManipulator;
 use Rector\NodeNameResolver\NodeNameResolver;
@@ -31,7 +32,8 @@ public function __construct(
         private NodeNameResolver $nodeNameResolver,
         private CollectionVarTagValueNodeResolver $collectionVarTagValueNodeResolver,
         private StaticTypeMapper $staticTypeMapper,
-        private CollectionTypeFactory $collectionTypeFactory
+        private CollectionTypeFactory $collectionTypeFactory,
+        private CollectionTypeResolver $collectionTypeResolver
     ) {
     }
 
@@ -80,7 +82,10 @@ public function resolveAssignedGenericCollectionType(Class_ $class, ClassMethod
             if (count($nonCollectionTypes) === 1) {
                 $soleType = $nonCollectionTypes[0];
                 if ($soleType instanceof ArrayType && $soleType->getItemType() instanceof ObjectType) {
-                    return $this->collectionTypeFactory->createType($soleType->getItemType());
+                    return $this->collectionTypeFactory->createType(
+                        $soleType->getItemType(),
+                        $this->collectionTypeResolver->hasIndexBy($property)
+                    );
                 }
             }
         }

diff --git a/src/NodeManipulator/ToManyRelationPropertyTypeResolver.php b/src/NodeManipulator/ToManyRelationPropertyTypeResolver.php
@@ -19,6 +19,7 @@
 use Rector\Doctrine\NodeAnalyzer\AttributeFinder;
 use Rector\Doctrine\PhpDoc\ShortClassExpander;
 use Rector\Doctrine\TypeAnalyzer\CollectionTypeFactory;
+use Rector\Doctrine\TypeAnalyzer\CollectionTypeResolver;
 use Rector\PhpParser\Node\Value\ValueResolver;
 use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType;
 
@@ -30,6 +31,7 @@ public function __construct(
         private AttributeFinder $attributeFinder,
         private ValueResolver $valueResolver,
         private CollectionTypeFactory $collectionTypeFactory,
+        private CollectionTypeResolver $collectionTypeResolver
     ) {
     }
 
@@ -92,6 +94,9 @@ private function resolveTypeFromTargetEntity(Expr|string $targetEntity, Property
         $entityFullyQualifiedClass = $this->shortClassExpander->resolveFqnTargetEntity($targetEntity, $property);
         $fullyQualifiedObjectType = new FullyQualifiedObjectType($entityFullyQualifiedClass);
 
-        return $this->collectionTypeFactory->createType($fullyQualifiedObjectType);
+        return $this->collectionTypeFactory->createType(
+            $fullyQualifiedObjectType,
+            $this->collectionTypeResolver->hasIndexBy($property)
+        );
     }
 }
diff --git a/src/TypeAnalyzer/CollectionTypeFactory.php b/src/TypeAnalyzer/CollectionTypeFactory.php
@@ -10,9 +10,10 @@
 
 final class CollectionTypeFactory
 {
-    public function createType(ObjectType $objectType): GenericObjectType
+    public function createType(ObjectType $objectType, bool $withIndexBy): GenericObjectType
     {
-        $genericTypes = [new IntegerType(), $objectType];
+        $genericTypes = $withIndexBy ? [$objectType] : [new IntegerType(), $objectType];
+
         return new GenericObjectType('Doctrine\Common\Collections\Collection', $genericTypes);
     }
 }
diff --git a/src/TypeAnalyzer/CollectionTypeResolver.php b/src/TypeAnalyzer/CollectionTypeResolver.php
@@ -10,20 +10,25 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\ClassConstFetch;
 use PhpParser\Node\Identifier;
+use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Property;
+use PHPStan\PhpDocParser\Ast\NodeTraverser;
 use PHPStan\PhpDocParser\Ast\Type\ArrayTypeNode;
 use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
 use PHPStan\PhpDocParser\Ast\Type\TypeNode;
 use PHPStan\PhpDocParser\Ast\Type\UnionTypeNode;
 use Rector\BetterPhpDocParser\PhpDoc\ArrayItemNode;
 use Rector\BetterPhpDocParser\PhpDoc\DoctrineAnnotationTagValueNode;
 use Rector\BetterPhpDocParser\PhpDoc\StringNode;
+use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
+use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
 use Rector\Doctrine\CodeQuality\Enum\CollectionMapping;
 use Rector\Doctrine\CodeQuality\Enum\EntityMappingKey;
 use Rector\Doctrine\CodeQuality\Enum\OdmMappingKey;
 use Rector\Doctrine\NodeAnalyzer\AttrinationFinder;
 use Rector\Doctrine\NodeAnalyzer\TargetEntityResolver;
 use Rector\Doctrine\PhpDoc\ShortClassExpander;
+use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 use Rector\StaticTypeMapper\Naming\NameScopeFactory;
 use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType;
 
@@ -39,6 +44,8 @@ public function __construct(
         private ShortClassExpander $shortClassExpander,
         private AttrinationFinder $attrinationFinder,
         private TargetEntityResolver $targetEntityResolver,
+        private PhpDocInfoFactory $phpDocInfoFactory,
+        private SimpleCallableNodeTraverser $simpleCallableNodeTraverser
     ) {
     }
 
@@ -62,6 +69,31 @@ public function resolveFromTypeNode(TypeNode $typeNode, Node $node): ?FullyQuali
         return null;
     }
 
+    public function hasIndexBy(Property|Param $property): bool
+    {
+        $phpDocInfo = $this->phpDocInfoFactory->createFromNode($property);
+        if ($phpDocInfo instanceof PhpDocInfo && str_contains((string) $phpDocInfo->getPhpDocNode(), 'indexBy')) {
+            return true;
+        }
+
+        $attrGroups = $property->attrGroups;
+        $hasIndexBy = false;
+
+        $this->simpleCallableNodeTraverser->traverseNodesWithCallable(
+            $attrGroups,
+            function (Node $node) use (&$hasIndexBy): ?int {
+                if ($node instanceof Arg && $node->name instanceof Identifier && $node->name->toString() === 'indexBy') {
+                    $hasIndexBy = true;
+                    return NodeTraverser::STOP_TRAVERSAL;
+                }
+
+                return null;
+            }
+        );
+
+        return $hasIndexBy;
+    }
+
     public function resolveFromToManyProperty(Property $property): ?FullyQualifiedObjectType
     {
         $doctrineAnnotationTagValueNodeOrAttribute = $this->attrinationFinder->getByMany(