From 12401b54e31202725c19200535fb3129bcedc653 Mon Sep 17 00:00:00 2001
From: DualFroz <me@dualfroz.com>
Date: Fri, 4 Sep 2026 06:21:06 +0200
Subject: [PATCH] fix: use value equality for sortByField tie detection

ClosureExpressionVisitor::sortByField() decided ties with a strict
"===" identity check before falling through to the next sort field.
For object-typed fields (e.g. two DateTimeImmutable instances that
represent the same point in time but are not the same instance),
this check never passes, so secondary/tertiary Criteria::orderBy()
fields were silently ignored and the resulting comparator was not
antisymmetric (swapping the input order of two such elements changed
the sort result).

Replace the "===" tie check with the same "<=>" comparison already
used to determine ordering, so a tie is now detected whenever the
values are equal under that comparison, matching value semantics for
both scalars and comparable objects.

Fixes #437
---
 src/Expr/ClosureExpressionVisitor.php  | 12 +++++--
 tests/ClosureExpressionVisitorTest.php | 48 ++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/src/Expr/ClosureExpressionVisitor.php b/src/Expr/ClosureExpressionVisitor.php
index a397b4b..01afa49 100644
--- a/src/Expr/ClosureExpressionVisitor.php
+++ b/src/Expr/ClosureExpressionVisitor.php
@@ -170,11 +170,19 @@ class ClosureExpressionVisitor extends ExpressionVisitor
             $aValue = ClosureExpressionVisitor::getObjectFieldValue($a, $name, $accessRawFieldValues);
             $bValue = ClosureExpressionVisitor::getObjectFieldValue($b, $name, $accessRawFieldValues);
 
-            if ($aValue === $bValue) {
+            // Use the same comparison that determines the ordering below (<=>, which is
+            // built from the same <, ==, > semantics as the "$aValue > $bValue" check) to
+            // detect ties. A strict "===" identity check misses value-equal-but-not-identical
+            // objects (e.g. two DateTimeImmutable instances for the same point in time),
+            // which silently skips the next sort field and can make the comparator
+            // non-antisymmetric.
+            $comparison = $aValue <=> $bValue;
+
+            if ($comparison === 0) {
                 return $next($a, $b);
             }
 
-            return ($aValue > $bValue ? 1 : -1) * $orientation;
+            return $comparison * $orientation;
         };
     }
 
diff --git a/tests/ClosureExpressionVisitorTest.php b/tests/ClosureExpressionVisitorTest.php
index 182f783..d579c00 100644
--- a/tests/ClosureExpressionVisitorTest.php
+++ b/tests/ClosureExpressionVisitorTest.php
@@ -6,6 +6,7 @@ namespace Doctrine\Tests\Common\Collections;
 
 use ArrayAccess;
 use ArrayIterator;
+use DateTimeImmutable;
 use Doctrine\Common\Collections\Expr\ClosureExpressionVisitor;
 use Doctrine\Common\Collections\Expr\Comparison;
 use Doctrine\Common\Collections\Expr\CompositeExpression;
@@ -487,6 +488,53 @@ class ClosureExpressionVisitorTest extends TestCase
         self::assertEquals('c', $objects[2]->getBar());
     }
 
+    public function testSortDelegateWithEqualButNotIdenticalObjectPrimaryField(): void
+    {
+        // Two distinct DateTimeImmutable instances representing the same point in time are
+        // value-equal ("==") but not identical ("==="). The primary field tie must still fall
+        // through to the secondary field instead of being reported as a strict mismatch.
+        $sameInstant = static fn (): DateTimeImmutable => new DateTimeImmutable('2024-12-10 16:04:11');
+
+        $objects = [
+            new TestObject($sameInstant(), 5),
+            new TestObject($sameInstant(), 6),
+        ];
+
+        self::assertNotSame($objects[0]->getFoo(), $objects[1]->getFoo());
+        self::assertEquals($objects[0]->getFoo(), $objects[1]->getFoo());
+
+        $sort = ClosureExpressionVisitor::sortByField('bar', -1, null, true);
+        $sort = ClosureExpressionVisitor::sortByField('foo', -1, $sort, true);
+
+        usort($objects, $sort);
+
+        self::assertEquals(6, $objects[0]->getBar());
+        self::assertEquals(5, $objects[1]->getBar());
+    }
+
+    public function testSortByFieldComparatorIsAntisymmetricForEqualButNotIdenticalObjects(): void
+    {
+        // A comparator that only falls through to the next field on a strict "===" tie is not
+        // antisymmetric for value-equal objects: swapping the input order of two equal-but-not-
+        // identical elements must not change which element sorts first.
+        $sameInstant = static fn (): DateTimeImmutable => new DateTimeImmutable('2024-12-10 16:04:11');
+
+        $element5 = new TestObject($sameInstant(), 5);
+        $element6 = new TestObject($sameInstant(), 6);
+
+        $sort = ClosureExpressionVisitor::sortByField('bar', -1, null, true);
+        $sort = ClosureExpressionVisitor::sortByField('foo', -1, $sort, true);
+
+        $orderedFromAscendingInput = [$element5, $element6];
+        usort($orderedFromAscendingInput, $sort);
+
+        $orderedFromDescendingInput = [$element6, $element5];
+        usort($orderedFromDescendingInput, $sort);
+
+        self::assertEquals(6, $orderedFromAscendingInput[0]->getBar());
+        self::assertEquals(6, $orderedFromDescendingInput[0]->getBar());
+    }
+
     public function testArrayComparison(): void
     {
         $closure = $this->visitor->walkComparison($this->builder->eq('foo', 42));