Fix type hint on OrderBy attribute

[curry684]

PHPStan 2.0 does not accept a string backed enum as a string.

[greg0ire]

Please retarget to 2.20.x, I think it also applies here.

[curry684]

It was completely rewritten for 3.x so needs a different change: https://github.com/doctrine/orm/blob/2.20.x/src/Mapping/OrderBy.php

I doubt it's really worth targeting 2.x as the error only occurs when picking up on the deprecation of Criteria::ASC which tells you to upgrade to Order::Ascending, and then running PHPStan 2 on max level.

[curry684]

Created #11780 targetting 2.20.x

[greg0ire]

Given the static analysis error, I guess targeting 2 is indeed not worth it. Sorry for the confusion.

[derrabus]

Do we really allow the Order enum here? I'm confused.

[curry684]

The deprecation explicitly replaces the const string with a reference to Order::Ascending:

https://github.com/doctrine/collections/blob/2790cb57e6510e4404873bdca6688e447550d720/src/Criteria.php#L22-L26

Which is a backed enum:

https://github.com/doctrine/collections/blob/2790cb57e6510e4404873bdca6688e447550d720/src/Order.php#L7-L11

But, come to think of it, the deprecation may just be triggered horribly wrong here as I vaguely recall Doctrine ORM having major internal inconsistencies here and requiring the literal "ASC" string in anything relating to OrderBy instead of providing constants/enums for it, or relying on those in Collections despite it being a direct dependency, which historically just work because they're the same string anyway.

[derrabus]

None of our test cases use the enum for this attribute which indeed still demands a string. This change is not correct, sorry.

[curry684]

Yep, just checked and using the enum causes some horrible crashes, so closing the PR is correct ofc.

But erm, are we ever going to fix the horrible inconsistencies here? Because I don't see why we're insisting on using constant magic strings (a strict no-go in every software architecture book) instead of relying on a well defined predictable enum of a direct dependency.

[derrabus]

There's a discussion on this somewhere in this tracker, but I'm on a phone and can't look this up right now.

[curry684]

At #11313