Allow SortDirection in EntityRepository::findBy() and findOneBy()

UPGRADE.md

@@ -145,6 +145,18 @@ This applies to the following methods:
 - `Doctrine\ORM\Query\Expr\OrderBy::__construct()`
 - `Doctrine\ORM\Query\Expr\OrderBy::add()`
 
+This deprecation does NOT apply to `EntityRepository::findBy()` and
+`findOneBy()`: since `doctrine/persistence` 3.x and 4.x document strings as
+the only order values in `ObjectRepository`, these methods keep accepting
+strings (`'ASC'` / `'DESC'`, case-insensitive) without deprecation. Both
+strings and `\SortDirection` are accepted; static analysis only allows
+`\SortDirection` when the repository is typed as `EntityRepository`, not as
+`Doctrine\Persistence\ObjectRepository`. `doctrine/persistence` 5.0 will
+also accept `\SortDirection` in `ObjectRepository`, at which point the enum
+form will be statically valid regardless of how the repository is typed.
+Sort directions given to `matching()` / `Criteria` are also not deprecated
+(handled by `doctrine/collections`).
+
 ```diff
 -$qb->orderBy('u.name', 'ASC')
 -   ->addOrderBy('u.createdAt', 'DESC');
@@ -246,6 +258,13 @@ for extending classes. If you extend the querybuilder and override any of the
 above methods, you will need to update the method signature to add support for
 `\SortDirection` as well. Same goes for `Expr\OrderBy::add()`.
 
+`EntityRepository::findBy()` and `findOneBy()` keep their native signature
+(`array|null $orderBy`), but their documented `$orderBy` type is widened to
+also accept `\SortDirection`. If you extend `EntityRepository` and override
+`findBy()` or `findOneBy()`, you will need to update the method docblock to
+add support for `\SortDirection` as well. The `ObjectRepository` interface
+is unchanged.
+
 ## Conditional breaking changes
 
 3.7 adds support for `doctrine/collections` 3. If you upgrade to that version

docs/en/reference/query-builder.rst

@@ -214,8 +214,10 @@ Here is a complete list of helper methods available in ``QueryBuilder``:
         // Example - $qb->orderBy('u.surname', \SortDirection::Descending)
         public function orderBy($sort, $order = null);
 
-        // Example - $qb->addOrderBy('u.firstName')
-        public function addOrderBy($sort, $order = null); // Default $order = 'ASC'
+        // Example - $qb->addOrderBy('u.firstName', \SortDirection::Ascending)
+        // NOTE: the default is \SortDirection::Ascending when $order is omitted;
+        // passing strings or null as $order is deprecated
+        public function addOrderBy($sort, $order = null);
     }
 
 Binding parameters to your query

docs/en/reference/working-with-objects.rst

@@ -666,6 +666,21 @@ The ``EntityRepository#findBy()`` method additionally accepts orderings, limit a
     <?php
     $tenUsers = $em->getRepository('MyProject\Domain\User')->findBy(array('age' => 20), array('name' => 'ASC'), 10, 0);
 
+The ``findBy()`` and ``findOneBy()`` methods also accept ``\SortDirection``
+values as sort directions, which can be mixed with ``'ASC'`` / ``'DESC'``
+strings:
+
+.. code-block:: php
+
+    <?php
+    $tenUsers = $em->getRepository('MyProject\Domain\User')
+        ->findBy(array('age' => 20), array('name' => \SortDirection::Descending), 10, 0);
+
+Unlike ``QueryBuilder#orderBy()`` and the mapped ``OrderBy`` attribute,
+repository methods do not deprecate string sort directions: they must keep
+accepting strings while ``doctrine/persistence`` 3.x and 4.x document them as
+the only statically valid values. Both forms are therefore supported.
+
 If you pass an array of values Doctrine will convert the query into a WHERE field IN (..) query automatically:
 
 .. code-block:: php

src/EntityRepository.php

@@ -16,6 +16,7 @@
 use Doctrine\ORM\Query\ResultSetMappingBuilder;
 use Doctrine\ORM\Repository\Exception\InvalidMagicMethodCall;
 use Doctrine\Persistence\ObjectRepository;
+use SortDirection;
 
 use function array_slice;
 use function lcfirst;
@@ -102,6 +103,8 @@ public function findAll(): array
      *
      * {@inheritDoc}
      *
+     * @phpstan-param array<string, SortDirection|'asc'|'desc'|'ASC'|'DESC'>|null $orderBy
+     *
      * @phpstan-return list<T>
      */
     public function findBy(array $criteria, array|null $orderBy = null, int|null $limit = null, int|null $offset = null): array
@@ -115,7 +118,7 @@ public function findBy(array $criteria, array|null $orderBy = null, int|null $li
      * Finds a single entity by a set of criteria.
      *
      * @phpstan-param array<string, mixed> $criteria
-     * @phpstan-param array<string, string>|null $orderBy
+     * @phpstan-param array<string, SortDirection|'asc'|'desc'|'ASC'|'DESC'>|null $orderBy
      *
      * @phpstan-return T|null
      */

src/Persisters/Entity/EntityPersister.php

@@ -143,21 +143,21 @@ public function getOwningTable(string $fieldName): string;
     /**
      * Loads an entity by a list of field criteria.
      *
-     * @param mixed[]                 $criteria The criteria by which to load the entity.
-     * @param object|null             $entity   The entity to load the data into. If not specified,
-     *                                          a new entity is created.
-     * @param AssociationMapping|null $assoc    The association that connects the entity
-     *                                          to load to another entity, if any.
-     * @param mixed[]                 $hints    Hints for entity creation.
-     * @param LockMode|int|null       $lockMode One of the \Doctrine\DBAL\LockMode::* constants
-     *                                          or NULL if no specific lock mode should be used
-     *                                          for loading the entity.
-     * @param int|null                $limit    Limit number of results.
-     * @param string[]|null           $orderBy  Criteria to order by.
-     * @phpstan-param array<string, mixed>       $criteria
-     * @phpstan-param array<string, mixed>       $hints
-     * @phpstan-param LockMode::*|null           $lockMode
-     * @phpstan-param array<string, string>|null $orderBy
+     * @param mixed[]                                  $criteria The criteria by which to load the entity.
+     * @param object|null                              $entity   The entity to load the data into. If not specified,
+     *                                                           a new entity is created.
+     * @param AssociationMapping|null                  $assoc    The association that connects the entity
+     *                                                           to load to another entity, if any.
+     * @param mixed[]                                  $hints    Hints for entity creation.
+     * @param LockMode|int|null                        $lockMode One of the \Doctrine\DBAL\LockMode::* constants
+     *                                                           or NULL if no specific lock mode should be used
+     *                                                           for loading the entity.
+     * @param int|null                                 $limit    Limit number of results.
+     * @param array<string, SortDirection|string>|null $orderBy  Criteria to order by.
+     * @phpstan-param array<string, mixed>              $criteria
+     * @phpstan-param array<string, mixed>              $hints
+     * @phpstan-param LockMode::*|null                  $lockMode
+     * @phpstan-param array<string, SortDirection|string>|null $orderBy
      *
      * @return object|null The loaded and managed entity instance or NULL if the entity can not be found.
      *

tests/StaticAnalysis/Repository/find-by-sort-direction.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\StaticAnalysis\Repository;
+
+use Doctrine\ORM\EntityRepository;
+use SortDirection;
+
+/** @template T of object */
+final class FindBySortDirection
+{
+    /**
+     * @param EntityRepository<T> $repository
+     *
+     * @return list<T>
+     */
+    public function findBy(EntityRepository $repository): array
+    {
+        return $repository->findBy([], [
+            'name' => SortDirection::Ascending,
+            'id'   => SortDirection::Descending,
+        ]);
+    }
+
+    /**
+     * @param EntityRepository<T> $repository
+     *
+     * @return T|null
+     */
+    public function findOneBy(EntityRepository $repository): object|null
+    {
+        return $repository->findOneBy([], [
+            'name' => SortDirection::Descending,
+        ]);
+    }
+}

tests/Tests/ORM/Functional/EntityRepositoryTest.php

@@ -8,6 +8,7 @@
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\DBAL\LockMode;
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Doctrine\ORM\EntityRepository;
 use Doctrine\ORM\Exception\ORMException;
 use Doctrine\ORM\Exception\UnrecognizedIdentifierFields;
@@ -27,13 +28,16 @@
 use Doctrine\Tests\Models\DDC753\DDC753EntityWithDefaultCustomRepository;
 use Doctrine\Tests\OrmFunctionalTestCase;
 use PHPUnit\Framework\Attributes\Group;
+use SortDirection;
 
 use function array_values;
 use function defined;
 use function reset;
 
 class EntityRepositoryTest extends OrmFunctionalTestCase
 {
+    use VerifyDeprecations;
+
     protected function setUp(): void
     {
         $this->useModelSet('cms');
@@ -405,6 +409,7 @@ public function testFindOneByAssociationKey(): void
     #[Group('DDC-1241')]
     public function testFindOneByOrderBy(): void
     {
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/11313');
         $this->loadFixture();
 
         $repos    = $this->_em->getRepository(CmsUser::class);
@@ -414,6 +419,17 @@ public function testFindOneByOrderBy(): void
         self::assertNotSame($userAsc, $userDesc);
     }
 
+    public function testFindOneByOrderBySortDirection(): void
+    {
+        $this->loadFixture();
+
+        $repos    = $this->_em->getRepository(CmsUser::class);
+        $userAsc  = $repos->findOneBy([], ['username' => SortDirection::Ascending]);
+        $userDesc = $repos->findOneBy([], ['username' => SortDirection::Descending]);
+
+        self::assertNotSame($userAsc, $userDesc);
+    }
+
     #[Group('DDC-817')]
     public function testFindByAssociationKey(): void
     {
@@ -489,6 +505,7 @@ public function testFindByLimitOffset(): void
     #[Group('DDC-1094')]
     public function testFindByOrderBy(): void
     {
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/11313');
         $this->loadFixture();
 
         $repos     = $this->_em->getRepository(CmsUser::class);
@@ -501,6 +518,20 @@ public function testFindByOrderBy(): void
         self::assertSame($usersAsc[3], $usersDesc[0]);
     }
 
+    public function testFindByOrderBySortDirection(): void
+    {
+        $this->loadFixture();
+
+        $repos     = $this->_em->getRepository(CmsUser::class);
+        $usersAsc  = $repos->findBy([], ['username' => SortDirection::Ascending]);
+        $usersDesc = $repos->findBy([], ['username' => SortDirection::Descending]);
+
+        self::assertCount(4, $usersAsc, 'Pre-condition: only four users in fixture');
+        self::assertCount(4, $usersDesc, 'Pre-condition: only four users in fixture');
+        self::assertSame($usersAsc[0], $usersDesc[3]);
+        self::assertSame($usersAsc[3], $usersDesc[0]);
+    }
+
     #[Group('DDC-1376')]
     public function testFindByOrderByAssociation(): void
     {