diff --git a/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_aliased_to_order_mixed_content.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_aliased_to_order_mixed_content.php.inc new file mode 100644 index 00000000..6a8703c1 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_aliased_to_order_mixed_content.php.inc @@ -0,0 +1,17 @@ +orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => SomeAliasedCriteria::DESC, 'param3' => 'asc']); + +?> +----- +orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => \Doctrine\Common\Collections\Order::Descending, 'param3' => \Doctrine\Common\Collections\Order::Ascending]); + +?> 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/criteria_to_order_mixed_content.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_mixed_content.php.inc new file mode 100644 index 00000000..8ced41a0 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/criteria_to_order_mixed_content.php.inc @@ -0,0 +1,17 @@ +orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => Criteria::DESC, 'param3' => 'asc']); + +?> +----- +orderBy(['param1' => \Doctrine\Common\Collections\Order::Ascending, 'param2' => \Doctrine\Common\Collections\Order::Descending, 'param3' => \Doctrine\Common\Collections\Order::Ascending]); + +?> 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-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/skip_order_by_not_on_criteria.php.inc b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/skip_order_by_not_on_criteria.php.inc new file mode 100644 index 00000000..ffa4066f --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Property/OrderByKeyToClassConstRector/Fixture/skip_order_by_not_on_criteria.php.inc @@ -0,0 +1,6 @@ +orderBy(['someType' => 'ASC']); + +?> diff --git a/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php b/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php index a1636953..2a0cec2a 100644 --- a/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php +++ b/rules/CodeQuality/Rector/Property/OrderByKeyToClassConstRector.php @@ -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; @@ -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; + '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 +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); } } 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; +}