diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_constant_criteria.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_constant_criteria.php.inc new file mode 100644 index 00000000..c41792fd --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_constant_criteria.php.inc @@ -0,0 +1,17 @@ +orderBy(['asc' => Criteria::ASC, 'desc' => Criteria::DESC]); + +?> +----- +orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]); + +?> diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_string_lower.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_string_lower.php.inc new file mode 100644 index 00000000..097dc58e --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_string_lower.php.inc @@ -0,0 +1,17 @@ +orderBy(['asc' => 'asc', 'desc' => 'desc']); + +?> +----- +orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]); + +?> diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_string_upper.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_string_upper.php.inc new file mode 100644 index 00000000..7645787e --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_from_string_upper.php.inc @@ -0,0 +1,17 @@ +orderBy(['asc' => 'ASC', 'desc' => 'DESC']); + +?> +----- +orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]); + +?> diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_lower_with_class_constant.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_lower_with_class_constant.php.inc index e30c15fa..a83f61a7 100644 --- a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_lower_with_class_constant.php.inc +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_lower_with_class_constant.php.inc @@ -10,16 +10,3 @@ class ReplaceOrderByAscWithClassConstant protected \DateTimeInterface $messages; } ?> ------ - \Doctrine\Common\Collections\Criteria::ASC])] - protected \DateTimeInterface $messages; -} -?> diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_with_class_constant.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_with_class_constant.php.inc index 04858ad3..577420e2 100644 --- a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_with_class_constant.php.inc +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_asc_with_class_constant.php.inc @@ -10,16 +10,3 @@ class ReplaceOrderByAscWithClassConstant protected \DateTimeInterface $messages; } ?> ------ - \Doctrine\Common\Collections\Criteria::ASC])] - protected \DateTimeInterface $messages; -} -?> diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_desc_with_class_constant.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_desc_with_class_constant.php.inc index 7bd5bfde..4a16231f 100644 --- a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_desc_with_class_constant.php.inc +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/replace_order_by_desc_with_class_constant.php.inc @@ -10,16 +10,3 @@ class ReplaceOrderByAscWithClassConstant protected \DateTimeInterface $messages; } ?> ------ - \Doctrine\Common\Collections\Criteria::DESC])] - protected \DateTimeInterface $messages; -} -?> diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/skip_already_using_const.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/skip_already_using_const.php.inc index 018827ec..8c70fe73 100644 --- a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/skip_already_using_const.php.inc +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/skip_already_using_const.php.inc @@ -1,13 +1,8 @@ Criteria::ASC])] - protected \DateTimeInterface $messages; -} +$criteria = new Criteria(); +$criteria->orderBy(['asc' => \Doctrine\Common\Collections\Order::Ascending, 'desc' => \Doctrine\Common\Collections\Order::Descending]); + ?> diff --git a/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php b/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php index a1636953..d6c573ec 100644 --- a/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php +++ b/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php @@ -5,10 +5,8 @@ namespace Rector\Doctrine\CodeQuality\Rector\Property; use PhpParser\Node; -use PhpParser\Node\Attribute; use PhpParser\Node\Expr\Array_; use PhpParser\Node\Scalar\String_; -use PhpParser\Node\Stmt\Property; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -21,31 +19,30 @@ final class OrderByKeyToClassConstRector extends AbstractRector 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; + '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; + \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 ), ] ); @@ -56,56 +53,87 @@ 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; - } - } - } - - // If Attribute is not OrderBy, return null - if (! $nodeAttribute instanceof Attribute) { + // TODO this rule should not apply on PHP versions earlier that 8.1.0 and doctrine collection greater that 2.2.0 + if (! $node->name instanceof Node\Identifier) { return null; } - if (! isset($nodeAttribute->args[0])) { + if ((string) $node->name !== 'orderBy') { return null; } - if (! $nodeAttribute->args[0]->value instanceof Array_) { + if (count($node->args) < 1) { return null; } - if (! isset($nodeAttribute->args[0]->value->items[0])) { + // TODO find a way to ensure that the 'orderBy' method does apply on a \Doctrine\Common\Collections\Criteria instance + + $arg = $node->args[0]; + if (! $arg instanceof Node\Arg) { return null; } - if (! $nodeAttribute->args[0]->value->items[0]->value instanceof String_) { + if (! $arg->value instanceof Array_) { return null; } - // 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; + $nodeHasChange = false; + // we parse the array here + foreach ($arg->value->items as $key => $item) { + if ($item === null) { + continue; + } + + if ($item->value instanceof String_) { + $value = $item->value->value; + if ($value === 'ASC' || $value === 'asc') { + $node->args[0]->value->items[$key]->value = $this->nodeFactory->createClassConstFetch( + 'Doctrine\Common\Collections\Order', + 'Ascending' + ); + $nodeHasChange = true; + } elseif ($value === 'DESC' || $value === 'desc') { + $node->args[0]->value->items[$key]->value = $this->nodeFactory->createClassConstFetch( + 'Doctrine\Common\Collections\Order', + 'Descending' + ); + $nodeHasChange = true; + } + } elseif ($item->value instanceof Node\Expr\ClassConstFetch) { + if (! in_array('Criteria', $item->value->class->getParts())) { + // TODO find a way to identify + continue; + } + + if ($item->value->name->toString() === 'ASC') { + $node->args[0]->value->items[$key]->value = $this->nodeFactory->createClassConstFetch( + 'Doctrine\Common\Collections\Order', + 'Ascending' + ); + $nodeHasChange = true; + } elseif ($item->value->name->toString() === 'DESC') { + $node->args[0]->value->items[$key]->value = $this->nodeFactory->createClassConstFetch( + 'Doctrine\Common\Collections\Order', + 'Descending' + ); + $nodeHasChange = true; + } + } + } - $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 - ); + if ($nodeHasChange) { + return $node; + } - return $node; + return null; } } diff --git a/tests/Set/DoctrineCOLLECTION22Set/Fixture/asc_desc_in_attributes_not_replaced.php.inc b/tests/Set/DoctrineCOLLECTION22Set/Fixture/asc_desc_in_attributes_not_replaced.php.inc new file mode 100644 index 00000000..4b112a66 --- /dev/null +++ b/tests/Set/DoctrineCOLLECTION22Set/Fixture/asc_desc_in_attributes_not_replaced.php.inc @@ -0,0 +1,12 @@ + \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; +}