Skip to content

Commit

Permalink
Add new rule - ExplicitRelationCollectionRector
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed Mar 21, 2024
1 parent a1d50bb commit cee4ddd
Show file tree
Hide file tree
Showing 9 changed files with 283 additions and 28 deletions.
3 changes: 2 additions & 1 deletion config/sets/doctrine-code-quality.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 30 additions & 1 deletion docs/rector_rules_overview.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# 18 Rules Overview
# 19 Rules Overview

## ChangeCompositeExpressionAddMultipleWithWithRector

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}

?>
Original file line number Diff line number Diff line change
@@ -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();
}
}

?>
Original file line number Diff line number Diff line change
@@ -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);
};
124 changes: 124 additions & 0 deletions rules/CodeQuality/Rector/Class_/ExplicitRelationCollectionRector.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
final class InitializeDefaultEntityCollectionRector extends AbstractRector
{
/**
* @var class-string[]
* @var string[]
*/
private const TO_MANY_ANNOTATION_CLASSES = [
'Doctrine\ORM\Mapping\OneToMany',
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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;
}
}
15 changes: 4 additions & 11 deletions src/NodeAnalyzer/AttrinationFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ public function __construct(
) {
}

/**
* @api
* @param class-string $name
*/
public function getByOne(
Property|Class_|ClassMethod|Param $node,
string $name
Expand All @@ -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);
Expand All @@ -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;
}
}

0 comments on commit cee4ddd

Please sign in to comment.