diff --git a/src/Expr/ClosureExpressionVisitor.php b/src/Expr/ClosureExpressionVisitor.php index a397b4b..fdf76eb 100644 --- a/src/Expr/ClosureExpressionVisitor.php +++ b/src/Expr/ClosureExpressionVisitor.php @@ -170,11 +170,13 @@ class ClosureExpressionVisitor extends ExpressionVisitor $aValue = ClosureExpressionVisitor::getObjectFieldValue($a, $name, $accessRawFieldValues); $bValue = ClosureExpressionVisitor::getObjectFieldValue($b, $name, $accessRawFieldValues); - if ($aValue === $bValue) { + $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..764b304 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,51 @@ class ClosureExpressionVisitorTest extends TestCase self::assertEquals('c', $objects[2]->getBar()); } + public function testSortDelegateWithEqualButNotIdenticalObjectPrimaryField(): void + { + // Value-equal but not identical primary values must still fall through to + // the secondary field. + $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 + { + // Swapping the input order of two value-equal elements must not change + // which one 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));