From cee4ddda0d9b131a101f9bcef83c9692a25eedd5 Mon Sep 17 00:00:00 2001
From: Tomas Votruba <tomas.vot@gmail.com>
Date: Thu, 21 Mar 2024 21:59:54 +0100
Subject: [PATCH] Add new rule - ExplicitRelationCollectionRector

---
 config/sets/doctrine-code-quality.php         |   3 +-
 docs/rector_rules_overview.md                 |  31 ++++-
 .../ExplicitRelationCollectionRectorTest.php  |  27 ++++
 .../Fixture/already_assigned.php.inc          |  41 ++++++
 .../Fixture/some_class.php.inc                |  35 +++++
 .../config/configured_rule.php                |  10 ++
 .../ExplicitRelationCollectionRector.php      | 124 ++++++++++++++++++
 ...nitializeDefaultEntityCollectionRector.php |  25 ++--
 src/NodeAnalyzer/AttrinationFinder.php        |  15 +--
 9 files changed, 283 insertions(+), 28 deletions(-)
 create mode 100644 rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/ExplicitRelationCollectionRectorTest.php
 create mode 100644 rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/already_assigned.php.inc
 create mode 100644 rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/some_class.php.inc
 create mode 100644 rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/config/configured_rule.php
 create mode 100644 rules/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector.php

diff --git a/config/sets/doctrine-code-quality.php b/config/sets/doctrine-code-quality.php
index 53b75b4d..bd2b59d4 100644
--- a/config/sets/doctrine-code-quality.php
+++ b/config/sets/doctrine-code-quality.php
@@ -19,11 +19,12 @@
 
 return static function (RectorConfig $rectorConfig): void {
     $rectorConfig->rules([
-        InitializeDefaultEntityCollectionRector::class,
         MakeEntityDateTimePropertyDateTimeInterfaceRector::class,
         MoveCurrentDateTimeDefaultInEntityToConstructorRector::class,
         CorrectDefaultTypesOnEntityPropertyRector::class,
         ImproveDoctrineCollectionDocTypeInEntityRector::class,
+        InitializeDefaultEntityCollectionRector::class,
+        \Rector\Doctrine\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector::class,
         RemoveEmptyTableAttributeRector::class,
 
         // typed properties in entities from annotations/attributes
diff --git a/docs/rector_rules_overview.md b/docs/rector_rules_overview.md
index 092a1c1c..f788356b 100644
--- a/docs/rector_rules_overview.md
+++ b/docs/rector_rules_overview.md
@@ -1,4 +1,4 @@
-# 18 Rules Overview
+# 19 Rules Overview
 
 ## ChangeCompositeExpressionAddMultipleWithWithRector
 
@@ -87,6 +87,35 @@ Replace EventSubscriberInterface with AsDoctrineListener attribute(s)
 
 <br>
 
+## ExplicitRelationCollectionRector
+
+Use explicit collection in one-to-many relations of Doctrine entity
+
+- class: [`Rector\Doctrine\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector`](../rules/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector.php)
+
+```diff
++use Doctrine\ORM\Mapping\Entity;
+ use Doctrine\ORM\Mapping\OneToMany;
+-use Doctrine\ORM\Mapping\Entity;
++use Doctrine\Common\Collections\ArrayCollection;
++use Doctrine\Common\Collections\Collection;
+
+ #[Entity]
+ class SomeClass
+ {
+     #[OneToMany(targetEntity: 'SomeClass')]
+-    private $items = [];
++    private Collection $items;
++
++    public function __construct()
++    {
++        $this->items = new ArrayCollection();
++    }
+ }
+```
+
+<br>
+
 ## ExtractArrayArgOnQueryBuilderSelectRector
 
 Extract array arg on QueryBuilder select, addSelect, groupBy, addGroupBy
diff --git a/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/ExplicitRelationCollectionRectorTest.php b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/ExplicitRelationCollectionRectorTest.php
new file mode 100644
index 00000000..cd9c37e9
--- /dev/null
+++ b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/ExplicitRelationCollectionRectorTest.php
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector;
+
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class ExplicitRelationCollectionRectorTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    public static function provideData(): \Iterator
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}
diff --git a/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/already_assigned.php.inc b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/already_assigned.php.inc
new file mode 100644
index 00000000..7f35e62a
--- /dev/null
+++ b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/already_assigned.php.inc
@@ -0,0 +1,41 @@
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector\Fixture;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\OneToMany;
+
+#[Entity]
+class AlreadyAssigned
+{
+    #[OneToMany(targetEntity: 'SomeClass')]
+    private $items = [];
+    public function __construct()
+    {
+        $this->items = new ArrayCollection();
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector\Fixture;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\OneToMany;
+
+#[Entity]
+class AlreadyAssigned
+{
+    #[OneToMany(targetEntity: 'SomeClass')]
+    private \Doctrine\Common\Collections\Collection $items;
+    public function __construct()
+    {
+        $this->items = new ArrayCollection();
+    }
+}
+
+?>
diff --git a/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/some_class.php.inc b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/some_class.php.inc
new file mode 100644
index 00000000..1554ab09
--- /dev/null
+++ b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/Fixture/some_class.php.inc
@@ -0,0 +1,35 @@
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector\Fixture;
+
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\OneToMany;
+
+#[Entity]
+class SomeClass
+{
+    #[OneToMany(targetEntity: 'SomeClass')]
+    private $items = [];
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Doctrine\Tests\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector\Fixture;
+
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\OneToMany;
+
+#[Entity]
+class SomeClass
+{
+    #[OneToMany(targetEntity: 'SomeClass')]
+    private \Doctrine\Common\Collections\Collection $items;
+    public function __construct()
+    {
+        $this->items = new \Doctrine\Common\Collections\ArrayCollection();
+    }
+}
+
+?>
diff --git a/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/config/configured_rule.php
new file mode 100644
index 00000000..5d8990f6
--- /dev/null
+++ b/rules-tests/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector/config/configured_rule.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Doctrine\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(ExplicitRelationCollectionRector::class);
+};
diff --git a/rules/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector.php b/rules/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector.php
new file mode 100644
index 00000000..ab3e6758
--- /dev/null
+++ b/rules/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector.php
@@ -0,0 +1,124 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Doctrine\CodeQuality\Rector\Class_;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Name\FullyQualified;
+use PhpParser\Node\Stmt\Class_;
+use Rector\Doctrine\NodeAnalyzer\AttrinationFinder;
+use Rector\Doctrine\NodeFactory\ArrayCollectionAssignFactory;
+use Rector\NodeManipulator\ClassDependencyManipulator;
+use Rector\Rector\AbstractRector;
+use Rector\TypeDeclaration\AlreadyAssignDetector\ConstructorAssignDetector;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\Doctrine\Tests\CodeQuality\Rector\Class_\ExplicitRelationCollectionRector\ExplicitRelationCollectionRectorTest
+ */
+final class ExplicitRelationCollectionRector extends AbstractRector
+{
+    public function __construct(
+        private readonly AttrinationFinder $attrinationFinder,
+        private readonly ConstructorAssignDetector $constructorAssignDetector,
+        private readonly ArrayCollectionAssignFactory $arrayCollectionAssignFactory,
+        private readonly ClassDependencyManipulator $classDependencyManipulator,
+    ) {
+    }
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('Use explicit collection in one-to-many relations of Doctrine entity', [
+            new CodeSample(
+                <<<'CODE_SAMPLE'
+use Doctrine\ORM\Mapping\OneToMany;
+use Doctrine\ORM\Mapping\Entity;
+
+#[Entity]
+class SomeClass
+{
+    #[OneToMany(targetEntity: 'SomeClass')]
+    private $items = [];
+}
+CODE_SAMPLE
+
+                ,
+                <<<'CODE_SAMPLE'
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\OneToMany;
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+
+#[Entity]
+class SomeClass
+{
+    #[OneToMany(targetEntity: 'SomeClass')]
+    private Collection $items;
+
+    public function __construct()
+    {
+        $this->items = new ArrayCollection();
+    }
+}
+CODE_SAMPLE
+            ),
+        ]);
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [Class_::class];
+    }
+
+    /**
+     * @param Class_ $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        if (! $this->attrinationFinder->hasByOne($node, 'Doctrine\ORM\Mapping\Entity')) {
+            return null;
+        }
+
+        $arrayCollectionAssigns = [];
+
+        foreach ($node->getProperties() as $property) {
+            if (! $this->attrinationFinder->hasByOne($property, 'Doctrine\ORM\Mapping\OneToMany')) {
+                continue;
+            }
+
+            // make sure has collection
+            if (! $property->type instanceof Node) {
+                $property->type = new FullyQualified('Doctrine\Common\Collections\Collection');
+            }
+
+            // make sure is null
+            if ($property->props[0]->default instanceof Expr) {
+                $property->props[0]->default = null;
+            }
+
+            /** @var string $propertyName */
+            $propertyName = $this->getName($property);
+            if ($this->constructorAssignDetector->isPropertyAssigned($node, $propertyName)) {
+                continue;
+            }
+
+            $arrayCollectionAssigns[] = $this->arrayCollectionAssignFactory->createFromPropertyName($propertyName);
+
+            // make sure it is initialized in constructor
+        }
+
+        if ($arrayCollectionAssigns === []) {
+            return null;
+        }
+
+        $this->classDependencyManipulator->addStmtsToConstructorIfNotThereYet($node, $arrayCollectionAssigns);
+
+        return $node;
+    }
+}
diff --git a/rules/CodeQuality/Rector/Class_/InitializeDefaultEntityCollectionRector.php b/rules/CodeQuality/Rector/Class_/InitializeDefaultEntityCollectionRector.php
index 98a10218..609369a9 100644
--- a/rules/CodeQuality/Rector/Class_/InitializeDefaultEntityCollectionRector.php
+++ b/rules/CodeQuality/Rector/Class_/InitializeDefaultEntityCollectionRector.php
@@ -23,7 +23,7 @@
 final class InitializeDefaultEntityCollectionRector extends AbstractRector
 {
     /**
-     * @var class-string[]
+     * @var string[]
      */
     private const TO_MANY_ANNOTATION_CLASSES = [
         'Doctrine\ORM\Mapping\OneToMany',
@@ -100,7 +100,15 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        return $this->refactorClass($node);
+        $toManyPropertyNames = $this->resolveToManyPropertyNames($node);
+        if ($toManyPropertyNames === []) {
+            return null;
+        }
+
+        $assigns = $this->createAssignsOfArrayCollectionsForPropertyNames($toManyPropertyNames);
+        $this->classDependencyManipulator->addStmtsToConstructorIfNotThereYet($node, $assigns);
+
+        return $node;
     }
 
     /**
@@ -144,17 +152,4 @@ private function createAssignsOfArrayCollectionsForPropertyNames(array $property
 
         return $assigns;
     }
-
-    private function refactorClass(Class_ $class): Class_|null
-    {
-        $toManyPropertyNames = $this->resolveToManyPropertyNames($class);
-        if ($toManyPropertyNames === []) {
-            return null;
-        }
-
-        $assigns = $this->createAssignsOfArrayCollectionsForPropertyNames($toManyPropertyNames);
-        $this->classDependencyManipulator->addStmtsToConstructorIfNotThereYet($class, $assigns);
-
-        return $class;
-    }
 }
diff --git a/src/NodeAnalyzer/AttrinationFinder.php b/src/NodeAnalyzer/AttrinationFinder.php
index 9169efd6..0fbe2472 100644
--- a/src/NodeAnalyzer/AttrinationFinder.php
+++ b/src/NodeAnalyzer/AttrinationFinder.php
@@ -24,10 +24,6 @@ public function __construct(
     ) {
     }
 
-    /**
-     * @api
-     * @param class-string $name
-     */
     public function getByOne(
         Property|Class_|ClassMethod|Param $node,
         string $name
@@ -40,9 +36,6 @@ public function getByOne(
         return $this->attributeFinder->findAttributeByClass($node, $name);
     }
 
-    /**
-     * @param class-string $name
-     */
     public function hasByOne(Property|Class_|ClassMethod|Param $node, string $name): bool
     {
         $phpDocInfo = $this->phpDocInfoFactory->createFromNode($node);
@@ -55,16 +48,16 @@ public function hasByOne(Property|Class_|ClassMethod|Param $node, string $name):
     }
 
     /**
-     * @param class-string[] $names
+     * @param string[] $classNames
      */
-    public function hasByMany(Property $property, array $names): bool
+    public function hasByMany(Property $property, array $classNames): bool
     {
         $phpDocInfo = $this->phpDocInfoFactory->createFromNode($property);
-        if ($phpDocInfo instanceof PhpDocInfo && $phpDocInfo->hasByAnnotationClasses($names)) {
+        if ($phpDocInfo instanceof PhpDocInfo && $phpDocInfo->hasByAnnotationClasses($classNames)) {
             return true;
         }
 
-        $attribute = $this->attributeFinder->findAttributeByClasses($property, $names);
+        $attribute = $this->attributeFinder->findAttributeByClasses($property, $classNames);
         return $attribute instanceof Attribute;
     }
 }