Conversation
SortDirection in EntityRepository::findBy() and findOneBy()
GromNaN
force-pushed
the
sort-direction-repository
branch
4 times, most recently
from
September 13, 2026 21:11
583cc15 to
dc62639
Compare
greg0ire
reviewed
Sep 13, 2026
greg0ire
reviewed
Sep 13, 2026
Member
|
Since this contains a breaking change, maybe we should mute the issue on 3.7.x, and this PR should target 3.8.x? On the other hand, it's just phpdoc/static analysis. I'm on the fence… |
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.
Address review feedback on doctrine#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
GromNaN
force-pushed
the
sort-direction-repository
branch
from
September 15, 2026 14:12
dc62639 to
6b77782
Compare
greg0ire
approved these changes
Sep 15, 2026
stof
approved these changes
Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EntityRepository::findBy()andfindOneBy()now accept and document\SortDirectionvalues for$orderBy, while'asc'/'desc'strings keep working without a deprecation.The
ObjectRepositoryinterface ofdoctrine/persistence3.x and 4.x documents strings as the only valid order values, so the string form stays accepted on the repository API. As an implementer of the interface,EntityRepositorycan widen its documented parameter type, so passing\SortDirection::Ascendingor\SortDirection::Descendingis now allowed and documented there, matching whatQueryBuilder,Expr\OrderByand the mapping API already accept. Native signatures are unchanged.Breaking change
This is a static-analysis BC break for custom repositories that override
findBy()orfindOneBy()and forward$orderBytoparent::*(): the documented$orderBytype now also accepts\SortDirection, so an override whose docblock reproduces the previous type (theObjectRepositorystring union forfindBy(), orarray<string, string>forfindOneBy()) is reported as incompatible by static analysis. Widen the override docblock accordingly.ObjectRepositoryitself is unchanged.Static analysis only allows
SortDirectionwhen the repository is typed asEntityRepository(or a subclass), not asDoctrine\Persistence\ObjectRepository, untildoctrine/persistence5 is required. The deprecation scope is clarified inUPGRADE.mdaccordingly.Changes:
$orderByphpdoc ofEntityRepository::findBy()andfindOneBy(), and ofEntityPersister::load()UPGRADE.mdand the reference docs