Fix attribute support in ImproveDoctrineCollectionDocTypeInEntityRect…

…or (#339) * add fixutre for var and attribute * Fix attribute support in ImproveDoctrineCollectionDocTypeInEntityRector * move attribute to own directory * extract SetterCollectionResolver * misc * add full generic collection type * improve sample to note setter as well * extract DoctrineClass enum
rectorphp · Aug 15, 2024 · 657d715 · 657d715
1 parent 11ac7b9
commit 657d715
Show file tree

Hide file tree

Showing 14 changed files with 265 additions and 67 deletions.
diff --git a/...ixture/OneToMany/no_var_attribute.php.inc → ...ToMany/Attribute/no_var_attribute.php.inc b/...ixture/OneToMany/no_var_attribute.php.inc → ...ToMany/Attribute/no_var_attribute.php.inc
@@ -1,8 +1,7 @@
 <?php
 
-namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany;
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany\Attribute;
 
-use Doctrine\Common\Collections\Collection;
 use Doctrine\ORM\Mapping as ORM;
 use Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training;
 
@@ -17,9 +16,8 @@ class NoVar
 -----
 <?php
 
-namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany;
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany\Attribute;
 
-use Doctrine\Common\Collections\Collection;
 use Doctrine\ORM\Mapping as ORM;
 use Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training;
 

diff --git a/...any/skip_collection_generic_class.php.inc → ...ute/skip_collection_generic_class.php.inc b/...any/skip_collection_generic_class.php.inc → ...ute/skip_collection_generic_class.php.inc
@@ -1,6 +1,6 @@
 <?php
 
-namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany;
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany\Attribute;
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Collection;

diff --git a/...ny/skip_collection_generic_object.php.inc → ...te/skip_collection_generic_object.php.inc b/...ny/skip_collection_generic_object.php.inc → ...te/skip_collection_generic_object.php.inc
@@ -1,6 +1,6 @@
 <?php
 
-namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany;
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany\Attribute;
 
 use DateTimeImmutable;
 use Doctrine\Common\Collections\ArrayCollection;

diff --git a/...rineCollectionDocTypeInEntityRector/Fixture/OneToMany/Attribute/var_and_attribute.php.inc b/...rineCollectionDocTypeInEntityRector/Fixture/OneToMany/Attribute/var_and_attribute.php.inc
@@ -0,0 +1,52 @@
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany\Attribute;
+
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training;
+
+#[ORM\Entity]
+class VarAndAttribute
+{
+    /**
+     * @var Collection|Training[]
+     */
+    #[ORM\OneToMany(targetEntity:Training::class, mappedBy:"trainer")]
+    private $trainings = [];
+
+    public function setTrainings($trainings)
+    {
+        $this->trainings = $trainings;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany\Attribute;
+
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training;
+
+#[ORM\Entity]
+class VarAndAttribute
+{
+    /**
+     * @var \Doctrine\Common\Collections\Collection<int, \Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training>
+     */
+    #[ORM\OneToMany(targetEntity:Training::class, mappedBy:"trainer")]
+    private $trainings = [];
+
+    /**
+     * @param \Doctrine\Common\Collections\Collection<int, \Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training> $trainings
+     */
+    public function setTrainings($trainings)
+    {
+        $this->trainings = $trainings;
+    }
+}
+
+?>
diff --git a/...Rector/Property/ImproveDoctrineCollectionDocTypeInEntityRector/config/configured_rule.php b/...Rector/Property/ImproveDoctrineCollectionDocTypeInEntityRector/config/configured_rule.php
@@ -5,6 +5,5 @@
 use Rector\Config\RectorConfig;
 use Rector\Doctrine\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector;
 
-return static function (RectorConfig $rectorConfig): void {
-    $rectorConfig->rule(ImproveDoctrineCollectionDocTypeInEntityRector::class);
-};
+return RectorConfig::configure()
+    ->withRules([ImproveDoctrineCollectionDocTypeInEntityRector::class]);
diff --git a/...ality/AttributeTransformer/PropertyAttributeTransformer/OneToManyAttributeTransformer.php b/...ality/AttributeTransformer/PropertyAttributeTransformer/OneToManyAttributeTransformer.php
@@ -32,7 +32,7 @@ public function transform(EntityMapping $entityMapping, Property|Param $property
         unset($oneToManyMapping[EntityMappingKey::ORDER_BY]);
 
         $args = $this->nodeFactory->createArgs($oneToManyMapping);
-        NodeValueNormalizer::ensureKeyIsClassConstFetch($args, 'targetEntity');
+        NodeValueNormalizer::ensureKeyIsClassConstFetch($args, EntityMappingKey::TARGET_ENTITY);
 
         $property->attrGroups[] = AttributeFactory::createGroup($this->getClassName(), $args);
     }

diff --git a/rules/CodeQuality/Enum/DoctrineClass.php b/rules/CodeQuality/Enum/DoctrineClass.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Doctrine\CodeQuality\Enum;
+
+final readonly class DoctrineClass
+{
+    /**
+     * @var string
+     */
+    public const COLLECTION = 'Doctrine\Common\Collections\Collection';
+}
diff --git a/rules/CodeQuality/Rector/Property/ImproveDoctrineCollectionDocTypeInEntityRector.php b/rules/CodeQuality/Rector/Property/ImproveDoctrineCollectionDocTypeInEntityRector.php
@@ -5,27 +5,25 @@
 namespace Rector\Doctrine\CodeQuality\Rector\Property;
 
 use PhpParser\Node;
+use PhpParser\Node\Attribute;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Stmt\Class_;
-use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Property;
 use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
-use PHPStan\Reflection\Php\PhpPropertyReflection;
-use PHPStan\Type\Type;
+use PHPStan\Type\Generic\GenericObjectType;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
 use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
 use Rector\Doctrine\CodeQuality\Enum\CollectionMapping;
+use Rector\Doctrine\CodeQuality\Enum\EntityMappingKey;
+use Rector\Doctrine\CodeQuality\SetterCollectionResolver;
 use Rector\Doctrine\NodeAnalyzer\AttributeFinder;
 use Rector\Doctrine\NodeAnalyzer\TargetEntityResolver;
 use Rector\Doctrine\PhpDocParser\DoctrineDocBlockResolver;
 use Rector\Doctrine\TypeAnalyzer\CollectionTypeFactory;
 use Rector\Doctrine\TypeAnalyzer\CollectionTypeResolver;
 use Rector\Doctrine\TypeAnalyzer\CollectionVarTagValueNodeResolver;
-use Rector\NodeManipulator\AssignManipulator;
 use Rector\Rector\AbstractRector;
-use Rector\Reflection\ReflectionResolver;
-use Rector\StaticTypeMapper\StaticTypeMapper;
 use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -37,16 +35,14 @@ final class ImproveDoctrineCollectionDocTypeInEntityRector extends AbstractRecto
 {
     public function __construct(
         private readonly CollectionTypeFactory $collectionTypeFactory,
-        private readonly AssignManipulator $assignManipulator,
         private readonly CollectionTypeResolver $collectionTypeResolver,
         private readonly CollectionVarTagValueNodeResolver $collectionVarTagValueNodeResolver,
         private readonly PhpDocTypeChanger $phpDocTypeChanger,
         private readonly DoctrineDocBlockResolver $doctrineDocBlockResolver,
-        private readonly ReflectionResolver $reflectionResolver,
         private readonly AttributeFinder $attributeFinder,
         private readonly TargetEntityResolver $targetEntityResolver,
         private readonly PhpDocInfoFactory $phpDocInfoFactory,
-        private readonly StaticTypeMapper $staticTypeMapper
+        private readonly SetterCollectionResolver $setterCollectionResolver,
     ) {
     }
 
@@ -70,6 +66,11 @@ class SomeClass
      * @var Collection|Trainer[]
      */
     private $trainings = [];
+
+    public function setTrainings($trainings)
+    {
+        $this->trainings = $trainings;
+    }
 }
 CODE_SAMPLE
                     ,
@@ -87,6 +88,14 @@ class SomeClass
      * @var Collection<int, Trainer>
      */
     private $trainings = [];
+
+    /**
+     * @param Collection<int, Trainer> $trainings
+     */
+    public function setTrainings($trainings)
+    {
+        $this->trainings = $trainings;
+    }
 }
 CODE_SAMPLE
                 ),
@@ -124,8 +133,9 @@ private function refactorProperty(Property $property): ?Property
         $targetEntityExpr = $this->attributeFinder->findAttributeByClassesArgByName(
             $property,
             CollectionMapping::TO_MANY_CLASSES,
-            'targetEntity'
+            EntityMappingKey::TARGET_ENTITY
         );
+
         if (! $targetEntityExpr instanceof Expr) {
             return null;
         }
@@ -145,8 +155,11 @@ private function refactorClassMethod(Class_ $class): ?Class_
                 continue;
             }
 
-            $collectionObjectType = $this->resolveCollectionSetterAssignType($class, $classMethod);
-            if (! $collectionObjectType instanceof Type) {
+            $collectionObjectType = $this->setterCollectionResolver->resolveAssignedGenericCollectionType(
+                $class,
+                $classMethod
+            );
+            if (! $collectionObjectType instanceof GenericObjectType) {
                 continue;
             }
 
@@ -163,6 +176,7 @@ private function refactorClassMethod(Class_ $class): ?Class_
 
             /** @var string $parameterName */
             $parameterName = $this->getName($param);
+
             $this->phpDocTypeChanger->changeParamType(
                 $classMethod,
                 $phpDocInfo,
@@ -181,35 +195,6 @@ private function refactorClassMethod(Class_ $class): ?Class_
         return null;
     }
 
-    private function resolveCollectionSetterAssignType(Class_ $class, ClassMethod $classMethod): ?Type
-    {
-        $propertyFetches = $this->assignManipulator->resolveAssignsToLocalPropertyFetches($classMethod);
-        if (count($propertyFetches) !== 1) {
-            return null;
-        }
-
-        $phpPropertyReflection = $this->reflectionResolver->resolvePropertyReflectionFromPropertyFetch(
-            $propertyFetches[0]
-        );
-        if (! $phpPropertyReflection instanceof PhpPropertyReflection) {
-            return null;
-        }
-
-        $propertyName = (string) $this->nodeNameResolver->getName($propertyFetches[0]);
-        $property = $class->getProperty($propertyName);
-
-        if (! $property instanceof Property) {
-            return null;
-        }
-
-        $varTagValueNode = $this->collectionVarTagValueNodeResolver->resolve($property);
-        if (! $varTagValueNode instanceof VarTagValueNode) {
-            return null;
-        }
-
-        return $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($varTagValueNode->type, $property);
-    }
-
     private function refactorPropertyPhpDocInfo(Property $property, PhpDocInfo $phpDocInfo): ?Property
     {
         $varTagValueNode = $this->collectionVarTagValueNodeResolver->resolve($property);
@@ -240,14 +225,24 @@ private function refactorPropertyPhpDocInfo(Property $property, PhpDocInfo $phpD
 
     private function refactorAttribute(Expr $expr, PhpDocInfo $phpDocInfo, Property $property): ?Property
     {
-        $phpDocVarTagValueNode = $phpDocInfo->getVarTagValueNode();
-        $phpDocCollectionVarTagValueNode = $this->collectionVarTagValueNodeResolver->resolve($property);
 
-        if ($phpDocVarTagValueNode instanceof VarTagValueNode && ! $phpDocCollectionVarTagValueNode instanceof VarTagValueNode) {
-            return null;
+        $toManyAttribute = $this->attributeFinder->findAttributeByClasses(
+            $property,
+            CollectionMapping::TO_MANY_CLASSES
+        );
+        if ($toManyAttribute instanceof Attribute) {
+            $targetEntityClassName = $this->targetEntityResolver->resolveFromAttribute($toManyAttribute);
+        } else {
+            $phpDocVarTagValueNode = $phpDocInfo->getVarTagValueNode();
+            $phpDocCollectionVarTagValueNode = $this->collectionVarTagValueNodeResolver->resolve($property);
+
+            if ($phpDocVarTagValueNode instanceof VarTagValueNode && ! $phpDocCollectionVarTagValueNode instanceof VarTagValueNode) {
+                return null;
+            }
+
+            $targetEntityClassName = $this->targetEntityResolver->resolveFromExpr($expr);
         }
 
-        $targetEntityClassName = $this->targetEntityResolver->resolveFromExpr($expr);
         if ($targetEntityClassName === null) {
             return null;
         }