Skip to content

Commit

Permalink
Refactor the OrderByKeyToClassConstRector to use the new enum only in…
Browse files Browse the repository at this point in the history
… Criteria::orderBy method call.

Relates to:

- doctrine/collections#389;
- doctrine/orm#11313 (comment)
  • Loading branch information
julienfastre committed Aug 9, 2024
1 parent 3ba7adb commit e165ced
Show file tree
Hide file tree
Showing 12 changed files with 199 additions and 91 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

use Doctrine\Common\Collections\Criteria as SomeAliasedCriteria;

$crit = new SomeAliasedCriteria();
$crit->orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => SomeAliasedCriteria::DESC, 'param3' => 'asc']);

?>
-----
<?php

use Doctrine\Common\Collections\Criteria as SomeAliasedCriteria;

$crit = new SomeAliasedCriteria();
$crit->orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => \Doctrine\Common\Collections\Order::Descending, 'param3' => \Doctrine\Common\Collections\Order::Ascending]);

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['asc' => Criteria::ASC, 'desc' => Criteria::DESC]);

?>
-----
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]);

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['asc' => 'asc', 'desc' => 'desc']);

?>
-----
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]);

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['asc' => 'ASC', 'desc' => 'DESC']);

?>
-----
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]);

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => Criteria::DESC, 'param3' => 'asc']);

?>
-----
<?php

use Doctrine\Common\Collections\Criteria;

$criteria = new Criteria();
$criteria->orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => \Doctrine\Common\Collections\Order::Descending, 'param3' => \Doctrine\Common\Collections\Order::Ascending]);

?>
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,3 @@ class ReplaceOrderByAscWithClassConstant
protected \DateTimeInterface $messages;
}
?>
-----
<?php

namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\OrderByKeyToClassConstRector\Fixture;

use Doctrine\ORM\Mapping as ORM;

class ReplaceOrderByAscWithClassConstant
{
#[ORM\OrderBy(['createdAt' => \Doctrine\Common\Collections\Criteria::ASC])]
protected \DateTimeInterface $messages;
}
?>
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,3 @@ class ReplaceOrderByAscWithClassConstant
protected \DateTimeInterface $messages;
}
?>
-----
<?php

namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\OrderByKeyToClassConstRector\Fixture;

use Doctrine\ORM\Mapping as ORM;

class ReplaceOrderByAscWithClassConstant
{
#[ORM\OrderBy(['createdAt' => \Doctrine\Common\Collections\Criteria::ASC])]
protected \DateTimeInterface $messages;
}
?>
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,3 @@ class ReplaceOrderByAscWithClassConstant
protected \DateTimeInterface $messages;
}
?>
-----
<?php

namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\OrderByKeyToClassConstRector\Fixture;

use Doctrine\ORM\Mapping as ORM;

class ReplaceOrderByAscWithClassConstant
{
#[ORM\OrderBy(['createdAt' => \Doctrine\Common\Collections\Criteria::DESC])]
protected \DateTimeInterface $messages;
}
?>
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
<?php

namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\OrderByKeyToClassConstRector\Fixture;

use Doctrine\Common\Collections\Criteria;
use Doctrine\ORM\Mapping as ORM;

class SkipAlreadyUsingConst
{
#[ORM\OrderBy(['createdAt' => Criteria::ASC])]
protected \DateTimeInterface $messages;
}
$criteria = new Criteria();
$criteria->orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]);

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

$object = new SomeClass();
$object->orderBy(['someType' => 'ASC']);

?>
137 changes: 93 additions & 44 deletions rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
namespace Rector\Doctrine\CodeQuality\Rector\Property;

use PhpParser\Node;
use PhpParser\Node\Attribute;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Scalar\String_;
use PhpParser\Node\Stmt\Property;
use PHPStan\Type\ObjectType;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -18,34 +19,40 @@
*/
final class OrderByKeyToClassConstRector extends AbstractRector
{
private ObjectType $criteriaObjectType;

public function __construct()
{
$this->criteriaObjectType = new ObjectType('Doctrine\Common\Collections\Criteria');
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Replace OrderBy Attribute ASC/DESC with class constant from Criteria',
'Replace OrderBy Attribute ASC/DESC with enum Ordering from Criteria',
[
new CodeSample(
<<<'CODE_SAMPLE'
use Doctrine\ORM\Mapping as ORM;
<?php
class ReplaceOrderByAscWithClassConstant
{
#[ORM\OrderBy(['createdAt' => 'ASC'])]
protected \DateTimeInterface $messages;
}
?>
CODE_SAMPLE
use Doctrine\Common\Collections\Criteria;
$criteria = new Criteria();
$criteria->orderBy(['someProperty' => 'ASC', 'anotherProperty' => 'DESC']);
?>
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
use Doctrine\ORM\Mapping as ORM;
<?php
class ReplaceOrderByAscWithClassConstant
{
#[ORM\OrderBy(['createdAt' => \Doctrine\Common\Collections\Criteria::ASC])]
protected \DateTimeInterface $messages;
}
?>
CODE_SAMPLE
use Doctrine\Common\Collections\Criteria;
$criteria = new Criteria();
$criteria->orderBy(['someProperty' => \Doctrine\Common\Collections\Order::Ascending, 'anotherProperty' => \Doctrine\Common\Collections\Order::Descending]);
?>
CODE_SAMPLE
),
]
);
Expand All @@ -56,56 +63,98 @@ class ReplaceOrderByAscWithClassConstant
*/
public function getNodeTypes(): array
{
return [Property::class];
return [Node\Expr\MethodCall::class];
}

/**
* @param Property $node
* @param Node\Expr\MethodCall $node
*/
public function refactor(Node $node): ?Node
{
$nodeAttribute = null;
foreach ($node->attrGroups as $attrGroup) {
foreach ($attrGroup->attrs as $attr) {
if ($attr->name->toString() === 'Doctrine\ORM\Mapping\OrderBy') {
$nodeAttribute = $attr;
break 2;
}
}
// TODO this rule should not apply on PHP versions earlier that 8.1.0 and doctrine collection greater that 2.2.0
if (! $this->isName($node->name, 'orderBy')) {
return null;
}

// If Attribute is not OrderBy, return null
if (! $nodeAttribute instanceof Attribute) {
$isSuperTypeOfCriteria = $this->criteriaObjectType->isSuperTypeOf($t = $this->nodeTypeResolver->getType($node->var));

if ($isSuperTypeOfCriteria->no() || $isSuperTypeOfCriteria->maybe()) {
return null;
}

if (! isset($nodeAttribute->args[0])) {
$args = $node->getArgs();

if (count($args) < 1) {
return null;
}

if (! $nodeAttribute->args[0]->value instanceof Array_) {
$arg = $args[0];
if (! $arg instanceof Node\Arg) {
return null;
}

if (! isset($nodeAttribute->args[0]->value->items[0])) {
if (! $arg->value instanceof Array_) {
return null;
}

if (! $nodeAttribute->args[0]->value->items[0]->value instanceof String_) {
return null;
$nodeHasChange = false;
$newItems = [];

// we parse the array of the first argument
foreach ($arg->value->items as $item) {

if ($item === null) {
$newItems[] = $item;
continue;
}

if ($item->value instanceof String_ && in_array(
$v = strtoupper($item->value->value),
['ASC', 'DESC'],
true
)) {
$newItems[] = $this->buildArrayItem($v, $item->key);
$nodeHasChange = true;
} elseif (
$item->value instanceof Node\Expr\ClassConstFetch
&& $item->value->class instanceof Node\Name
&& (string) $item->value->class === 'Doctrine\Common\Collections\Criteria'
&& $item->value->name instanceof Node\Identifier
&& in_array($v = strtoupper((string) $item->value->name), ['ASC', 'DESC'], true)
) {
$newItems[] = $this->buildArrayItem($v, $item->key);
$nodeHasChange = true;
} else {
$newItems[] = $item;
}
}

// If Attribute value from key is not `ASC` or `DESC`, return null
if (! in_array($nodeAttribute->args[0]->value->items[0]->value->value, ['ASC', 'asc', 'DESC', 'desc'], true)) {
return null;
if ($nodeHasChange) {
return $this->nodeFactory->createMethodCall(
$node->var,
'orderBy',
$this->nodeFactory->createArgs([$this->nodeFactory->createArg(new Array_($newItems))])
);
}

$upper = strtoupper($nodeAttribute->args[0]->value->items[0]->value->value);
$nodeAttribute->args[0]->value->items[0]->value = $this->nodeFactory->createClassConstFetch(
'Doctrine\Common\Collections\Criteria',
$upper
return null;
}

/**
* @param 'ASC'|'DESC' $direction
* @param Expr $key
* @return ArrayItem
*/
private function buildArrayItem(string $direction, Node\Expr|null $key): Node\Expr\ArrayItem
{
$value = $this->nodeFactory->createClassConstFetch(
'Doctrine\Common\Collections\Order',
match ($direction) {
'ASC' => 'Ascending',
'DESC' => 'Descending',
}
);

return $node;
return new Node\Expr\ArrayItem($value, $key);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

class DummyEntity {

#[ORM\OneToMany(targetEntity: AnotherEntity::class, mappedBy: 'dummy')]
#[ORM\OrderBy(["id" => \Doctrine\Common\Collections\Criteria::ASC])]
public \Doctrine\Common\Collections\Collection $properties;

#[ORM\OneToMany(targetEntity: YetAnotherEntity::class, mappedBy: 'dummy')]
#[ORM\OrderBy(["id" => \Doctrine\Common\Collections\Criteria::DESC])]
public \Doctrine\Common\Collections\Collection $otherProperties;
}

0 comments on commit e165ced

Please sign in to comment.