From c053e9c3050602ce311f7517572cf5018ade3503 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Sun, 13 Sep 2026 21:55:19 +0200 Subject: [PATCH 1/2] Allow SortDirection in EntityRepository::findBy() and findOneBy() Repository methods must keep accepting 'asc'/'desc' strings without a deprecation because the ObjectRepository interface of doctrine/persistence 3.x and 4.x documents strings as the only order values. As an implementer, EntityRepository can still widen its documented parameter type, so \SortDirection is now accepted and documented there as well. This makes the new SortDirection guidance usable on the most common repository API. The persister interface load() is aligned to the same widened type. Add functional tests (with and without second level cache), static analysis coverage, and clarify the deprecation scope in UPGRADE.md and the reference docs. --- UPGRADE.md | 20 ++++++++++ docs/en/reference/query-builder.rst | 6 ++- docs/en/reference/working-with-objects.rst | 15 ++++++++ src/EntityRepository.php | 5 ++- src/Persisters/Entity/EntityPersister.php | 30 +++++++-------- .../Repository/find-by-sort-direction.php | 37 +++++++++++++++++++ .../ORM/Functional/EntityRepositoryTest.php | 31 ++++++++++++++++ 7 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 tests/StaticAnalysis/Repository/find-by-sort-direction.php diff --git a/UPGRADE.md b/UPGRADE.md index 53b8feb8d39..9b472553b74 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -145,6 +145,16 @@ 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`. 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 +256,16 @@ 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`. This is an assumed static-analysis BC break for +custom repositories that override one of these methods and forward `$orderBy` +to `parent::*()`: an override whose docblock reproduces the previous type (the +`ObjectRepository` string union for `findBy()`, or `array` for +`findOneBy()`) triggers an `argument.type` error at that call. Widen the +override docblock to keep static analysis passing. The `ObjectRepository` +interface is unchanged. + ## Conditional breaking changes 3.7 adds support for `doctrine/collections` 3. If you upgrade to that version diff --git a/docs/en/reference/query-builder.rst b/docs/en/reference/query-builder.rst index f85c19742e8..b64c44c8873 100644 --- a/docs/en/reference/query-builder.rst +++ b/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 diff --git a/docs/en/reference/working-with-objects.rst b/docs/en/reference/working-with-objects.rst index d780b553941..9cfc78ca694 100644 --- a/docs/en/reference/working-with-objects.rst +++ b/docs/en/reference/working-with-objects.rst @@ -666,6 +666,21 @@ The ``EntityRepository#findBy()`` method additionally accepts orderings, limit a 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 + + 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 diff --git a/src/EntityRepository.php b/src/EntityRepository.php index 6d7f86e4073..d5f6582467a 100644 --- a/src/EntityRepository.php +++ b/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|null $orderBy + * * @phpstan-return list */ 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 $criteria - * @phpstan-param array|null $orderBy + * @phpstan-param array|null $orderBy * * @phpstan-return T|null */ diff --git a/src/Persisters/Entity/EntityPersister.php b/src/Persisters/Entity/EntityPersister.php index c74c75a314e..c0cad0104db 100644 --- a/src/Persisters/Entity/EntityPersister.php +++ b/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 $criteria - * @phpstan-param array $hints - * @phpstan-param LockMode::*|null $lockMode - * @phpstan-param array|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|null $orderBy Criteria to order by. + * @phpstan-param array $criteria + * @phpstan-param array $hints + * @phpstan-param LockMode::*|null $lockMode + * @phpstan-param array|null $orderBy * * @return object|null The loaded and managed entity instance or NULL if the entity can not be found. * diff --git a/tests/StaticAnalysis/Repository/find-by-sort-direction.php b/tests/StaticAnalysis/Repository/find-by-sort-direction.php new file mode 100644 index 00000000000..a77ea5c02ba --- /dev/null +++ b/tests/StaticAnalysis/Repository/find-by-sort-direction.php @@ -0,0 +1,37 @@ + $repository + * + * @return list + */ + public function findBy(EntityRepository $repository): array + { + return $repository->findBy([], [ + 'name' => SortDirection::Ascending, + 'id' => SortDirection::Descending, + ]); + } + + /** + * @param EntityRepository $repository + * + * @return T|null + */ + public function findOneBy(EntityRepository $repository): object|null + { + return $repository->findOneBy([], [ + 'name' => SortDirection::Descending, + ]); + } +} diff --git a/tests/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Tests/ORM/Functional/EntityRepositoryTest.php index 300bf526031..5dadda38b67 100644 --- a/tests/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/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,6 +28,7 @@ use Doctrine\Tests\Models\DDC753\DDC753EntityWithDefaultCustomRepository; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\Attributes\Group; +use SortDirection; use function array_values; use function defined; @@ -34,6 +36,8 @@ 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 { From 6b77782d06a1c1ebfda604a8a60614b3828dd1cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 15 Sep 2026 16:11:40 +0200 Subject: [PATCH 2/2] Clarify EntityRepository sort direction BC break in UPGRADE.md Address review feedback on #12613: - drop the ambiguous "assumed" and the PHPStan-specific argument.type identifier; reword the BC break to match the QueryBuilder wording - note that doctrine/persistence 5.0 will accept SortDirection in ObjectRepository --- UPGRADE.md | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 9b472553b74..c71f0edc5c4 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -151,9 +151,11 @@ 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`. Sort directions given to -`matching()` / `Criteria` are also not deprecated (handled by -`doctrine/collections`). +`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') @@ -258,13 +260,10 @@ above methods, you will need to update the method signature to add support for `EntityRepository::findBy()` and `findOneBy()` keep their native signature (`array|null $orderBy`), but their documented `$orderBy` type is widened to -also accept `\SortDirection`. This is an assumed static-analysis BC break for -custom repositories that override one of these methods and forward `$orderBy` -to `parent::*()`: an override whose docblock reproduces the previous type (the -`ObjectRepository` string union for `findBy()`, or `array` for -`findOneBy()`) triggers an `argument.type` error at that call. Widen the -override docblock to keep static analysis passing. The `ObjectRepository` -interface is unchanged. +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