Files
doctrine-collections/tests/ClosureExpressionVisitorTest.php
dualfroz 12401b54e3 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
2026-09-04 06:21:06 +02:00

643 lines
22 KiB
PHP

<?php
declare(strict_types=1);
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;
use Doctrine\Common\Collections\ExpressionBuilder;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
use PHPUnit\Framework\Attributes\RequiresPhp;
use PHPUnit\Framework\TestCase;
use RuntimeException;
use stdClass;
use function usort;
#[Group('DDC-1637')]
class ClosureExpressionVisitorTest extends TestCase
{
use VerifyDeprecations;
private ClosureExpressionVisitor $visitor;
private ExpressionBuilder $builder;
protected function setUp(): void
{
$this->visitor = new ClosureExpressionVisitor(true);
$this->builder = new ExpressionBuilder();
}
public function testEmbeddedObjectComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->eq('foo.foo', 1));
$this->assertTrue($closure(new TestObject(new TestObject(1))));
$this->assertFalse($closure(new TestObject(new TestObject(2))));
}
public function testGetEmbeddedObjectFieldValueAccessingRawValue(): void
{
$object = new TestObject(new TestObjectPrivatePropertyOnly(42));
self::assertSame(42, $this->visitor->getObjectFieldValue($object, 'foo.fooBar', true));
}
#[RequiresPhp('>= 8.4.0')]
public function testGetObjectFieldValueAccessingRawValueBypassingPropertyHook(): void
{
$object = new TestObjectPropertyHook();
$object->fooBar = 42;
self::assertSame(42, $this->visitor->getObjectFieldValue($object, 'fooBar', true));
}
public function testGetObjectFieldValueAccessingRawValue(): void
{
$object = new TestObjectPrivatePropertyOnly(42);
self::assertSame(42, $this->visitor->getObjectFieldValue($object, 'fooBar', true));
}
public function testGetObjectFieldValueFindingParentClassAccessingRawValue(): void
{
$object = new TestObjectWithPrivatePropertyInParentClass(42);
self::assertSame(42, $this->visitor->getObjectFieldValue($object, 'fooBar', true));
}
public function testGetObjectFieldValueNonexistentFieldAccessingRawValue(): void
{
$object = new stdClass();
$this->expectException(RuntimeException::class);
$this->visitor->getObjectFieldValue($object, 'fooBar', true);
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueIsAccessor(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObject(1, 2, true);
self::assertTrue($this->visitor->getObjectFieldValue($object, 'baz'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueIsAccessorWithIsPrefix(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObject(1, 2, true);
self::assertTrue($this->visitor->getObjectFieldValue($object, 'isBaz'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueIsAccessorCamelCase(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObjectNotCamelCase(1);
self::assertEquals(1, $this->visitor->getObjectFieldValue($object, 'foo_bar'));
self::assertEquals(1, $this->visitor->getObjectFieldValue($object, 'foobar'));
self::assertEquals(1, $this->visitor->getObjectFieldValue($object, 'fooBar'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueIsAccessorBoth(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObjectBothCamelCaseAndUnderscore(1, 2);
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'foo_bar'));
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'foobar'));
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'fooBar'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueIsAccessorOnePublic(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObjectPublicCamelCaseAndPrivateUnderscore(1, 2);
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'foo_bar'));
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'foobar'));
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'fooBar'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueIsAccessorBothPublic(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObjectPublicCamelCaseAndPrivateUnderscore(1, 2);
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'foo_bar'));
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'foobar'));
self::assertEquals(2, $this->visitor->getObjectFieldValue($object, 'fooBar'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueBlankAccessor(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObjectBlankGetter(1);
self::assertEquals(1, $this->visitor->getObjectFieldValue($object, 'foobar'));
self::assertEquals(1, $this->visitor->getObjectFieldValue($object, 'fooBar'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueMagicCallMethod(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new TestObject(1, 2, true, 3);
self::assertEquals(3, $this->visitor->getObjectFieldValue($object, 'qux'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValueArrayAccess(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = self::createMock(ArrayAccess::class);
$object->expects(self::once())
->method('offsetGet')
->with('foo')
->willReturn(33);
self::assertSame(33, $this->visitor->getObjectFieldValue($object, 'foo'));
}
#[IgnoreDeprecations]
public function testGetObjectFieldValuePublicPropertyIsNull(): void
{
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/472');
$object = new stdClass();
$object->foo = null;
self::assertSame(null, $this->visitor->getObjectFieldValue($object, 'foo'));
}
public function testWalkEqualsComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->eq('foo', 1));
self::assertTrue($closure(new TestObject(1)));
self::assertFalse($closure(new TestObject(2)));
}
public function testWalkNotEqualsComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->neq('foo', 1));
self::assertFalse($closure(new TestObject(1)));
self::assertTrue($closure(new TestObject(2)));
}
public function testWalkLessThanComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->lt('foo', 1));
self::assertFalse($closure(new TestObject(1)));
self::assertTrue($closure(new TestObject(0)));
}
public function testWalkLessThanEqualsComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->lte('foo', 1));
self::assertFalse($closure(new TestObject(2)));
self::assertTrue($closure(new TestObject(1)));
self::assertTrue($closure(new TestObject(0)));
}
public function testWalkGreaterThanEqualsComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->gte('foo', 1));
self::assertTrue($closure(new TestObject(2)));
self::assertTrue($closure(new TestObject(1)));
self::assertFalse($closure(new TestObject(0)));
}
public function testWalkGreaterThanComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->gt('foo', 1));
self::assertTrue($closure(new TestObject(2)));
self::assertFalse($closure(new TestObject(1)));
self::assertFalse($closure(new TestObject(0)));
}
public function testWalkInComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->in('foo', [1, 2, 3, '04']));
self::assertTrue($closure(new TestObject(2)));
self::assertTrue($closure(new TestObject(1)));
self::assertFalse($closure(new TestObject(0)));
self::assertFalse($closure(new TestObject(4)));
self::assertTrue($closure(new TestObject('04')));
}
public function testWalkInComparisonObjects(): void
{
$closure = $this->visitor->walkComparison($this->builder->in('foo', [new TestObject(1), new TestObject(2), new TestObject(4)]));
self::assertTrue($closure(new TestObject(new TestObject(2))));
self::assertTrue($closure(new TestObject(new TestObject(1))));
self::assertFalse($closure(new TestObject(new TestObject(0))));
self::assertTrue($closure(new TestObject(new TestObject(4))));
self::assertFalse($closure(new TestObject(new TestObject('baz'))));
}
public function testWalkNotInComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->notIn('foo', [1, 2, 3, '04']));
self::assertFalse($closure(new TestObject(1)));
self::assertFalse($closure(new TestObject(2)));
self::assertTrue($closure(new TestObject(0)));
self::assertTrue($closure(new TestObject(4)));
self::assertFalse($closure(new TestObject('04')));
}
public function testWalkNotInComparisonObjects(): void
{
$closure = $this->visitor->walkComparison($this->builder->notIn('foo', [new TestObject(1), new TestObject(2), new TestObject(4)]));
self::assertFalse($closure(new TestObject(new TestObject(1))));
self::assertFalse($closure(new TestObject(new TestObject(2))));
self::assertTrue($closure(new TestObject(new TestObject(0))));
self::assertFalse($closure(new TestObject(new TestObject(4))));
self::assertTrue($closure(new TestObject(new TestObject('baz'))));
}
public function testWalkContainsComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->contains('foo', 'hello'));
self::assertTrue($closure(new TestObject('hello world')));
self::assertFalse($closure(new TestObject('world')));
}
public function testWalkMemberOfComparisonWithObject(): void
{
$closure = $this->visitor->walkComparison($this->builder->memberof('foo', 2));
self::assertTrue($closure(new TestObject([1, 2, 3])));
self::assertTrue($closure(new TestObject([2])));
self::assertTrue($closure(new TestObject(new ArrayIterator([2]))));
self::assertFalse($closure(new TestObject([1, 3, 5])));
self::assertFalse($closure(new TestObject([1, '02'])));
self::assertFalse($closure(new TestObject(new ArrayIterator([4]))));
}
public function testWalkStartsWithComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->startsWith('foo', 'hello'));
self::assertTrue($closure(new TestObject('hello world')));
self::assertFalse($closure(new TestObject('world')));
}
public function testWalkEndsWithComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->endsWith('foo', 'world'));
self::assertTrue($closure(new TestObject('hello world')));
self::assertFalse($closure(new TestObject('hello')));
}
public function testWalkUnknownOperatorComparisonThrowException(): void
{
self::expectException(RuntimeException::class);
self::expectExceptionMessage('Unknown comparison operator: unknown');
$closure = $this->visitor->walkComparison(new Comparison('foo', 'unknown', 2));
$closure(new TestObject(2));
}
public function testWalkAndCompositeExpression(): void
{
$closure = $this->visitor->walkCompositeExpression(
$this->builder->andX(
$this->builder->eq('foo', 1),
$this->builder->eq('bar', 1),
),
);
self::assertTrue($closure(new TestObject(1, 1)));
self::assertFalse($closure(new TestObject(1, 0)));
self::assertFalse($closure(new TestObject(0, 1)));
self::assertFalse($closure(new TestObject(0, 0)));
}
public function testWalkOrCompositeExpression(): void
{
$closure = $this->visitor->walkCompositeExpression(
$this->builder->orX(
$this->builder->eq('foo', 1),
$this->builder->eq('bar', 1),
),
);
self::assertTrue($closure(new TestObject(1, 1)));
self::assertTrue($closure(new TestObject(1, 0)));
self::assertTrue($closure(new TestObject(0, 1)));
self::assertFalse($closure(new TestObject(0, 0)));
}
public function testWalkOrAndCompositeExpression(): void
{
$closure = $this->visitor->walkCompositeExpression(
$this->builder->orX(
$this->builder->andX(
$this->builder->eq('foo', 1),
$this->builder->eq('bar', 1),
),
$this->builder->andX(
$this->builder->eq('foo', 2),
$this->builder->eq('bar', 2),
),
),
);
self::assertTrue($closure(new TestObject(1, 1)));
self::assertTrue($closure(new TestObject(2, 2)));
self::assertFalse($closure(new TestObject(1, 2)));
self::assertFalse($closure(new TestObject(2, 1)));
self::assertFalse($closure(new TestObject(0, 0)));
}
public function testWalkAndOrCompositeExpression(): void
{
$closure = $this->visitor->walkCompositeExpression(
$this->builder->andX(
$this->builder->orX(
$this->builder->eq('foo', 1),
$this->builder->eq('foo', 2),
),
$this->builder->orX(
$this->builder->eq('bar', 3),
$this->builder->eq('bar', 4),
),
),
);
self::assertTrue($closure(new TestObject(1, 3)));
self::assertTrue($closure(new TestObject(1, 4)));
self::assertTrue($closure(new TestObject(2, 3)));
self::assertTrue($closure(new TestObject(2, 4)));
self::assertFalse($closure(new TestObject(1, 0)));
self::assertFalse($closure(new TestObject(2, 0)));
self::assertFalse($closure(new TestObject(0, 3)));
self::assertFalse($closure(new TestObject(0, 4)));
}
public function testWalkNotCompositeExpression(): void
{
$closure = $this->visitor->walkCompositeExpression(
$this->builder->not(
$this->builder->eq('foo', 1),
),
);
self::assertFalse($closure(new TestObject(1)));
self::assertTrue($closure(new TestObject(0)));
}
public function testWalkUnknownCompositeExpressionThrowException(): void
{
self::expectException(RuntimeException::class);
self::expectExceptionMessage('Unknown composite Unknown');
$closure = $this->visitor->walkCompositeExpression(
new CompositeExpression('Unknown', []),
);
$closure(new TestObject());
}
public function testSortByFieldAscending(): void
{
$objects = [new TestObject('b'), new TestObject('a'), new TestObject('c')];
$sort = ClosureExpressionVisitor::sortByField('foo', 1, null, true);
usort($objects, $sort);
self::assertEquals('a', $objects[0]->getFoo());
self::assertEquals('b', $objects[1]->getFoo());
self::assertEquals('c', $objects[2]->getFoo());
}
public function testSortByFieldDescending(): void
{
$objects = [new TestObject('b'), new TestObject('a'), new TestObject('c')];
$sort = ClosureExpressionVisitor::sortByField('foo', -1, null, true);
usort($objects, $sort);
self::assertEquals('c', $objects[0]->getFoo());
self::assertEquals('b', $objects[1]->getFoo());
self::assertEquals('a', $objects[2]->getFoo());
}
public function testSortByFieldKeepOrderWhenSameValue(): void
{
$firstElement = new TestObject('a');
$secondElement = new TestObject('a');
$objects = [$firstElement, $secondElement];
$sort = ClosureExpressionVisitor::sortByField('foo', 0, null, true);
usort($objects, $sort);
self::assertSame([$firstElement, $secondElement], $objects);
}
public function testSortDelegate(): void
{
$objects = [new TestObject('a', 'c'), new TestObject('a', 'b'), new TestObject('a', 'a')];
$sort = ClosureExpressionVisitor::sortByField('bar', 1, null, true);
$sort = ClosureExpressionVisitor::sortByField('foo', 1, $sort, true);
usort($objects, $sort);
self::assertEquals('a', $objects[0]->getBar());
self::assertEquals('b', $objects[1]->getBar());
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));
self::assertTrue($closure(['foo' => 42]));
}
}
class TestObject
{
public function __construct(
private readonly mixed $foo = null,
private readonly mixed $bar = null,
private readonly mixed $baz = null,
private readonly mixed $qux = null,
) {
}
/** @param array<int, mixed> $arguments */
public function __call(string $name, array $arguments): mixed
{
if ($name === 'getqux') {
return $this->qux;
}
}
public function getFoo(): mixed
{
return $this->foo;
}
public function getBar(): mixed
{
return $this->bar;
}
public function isBaz(): mixed
{
return $this->baz;
}
}
class TestObjectNotCamelCase
{
public function __construct(private readonly int|null $foo_bar)
{
}
public function getFooBar(): int|null
{
return $this->foo_bar;
}
}
class TestObjectBothCamelCaseAndUnderscore
{
public function __construct(private readonly int|null $foo_bar = null, private readonly int|null $fooBar = null)
{
}
public function getFooBar(): int|null
{
return $this->fooBar;
}
}
class TestObjectPublicCamelCaseAndPrivateUnderscore
{
public function __construct(private readonly int|null $foo_bar = null, public int|null $fooBar = null)
{
}
public function getFooBar(): int|null
{
return $this->fooBar;
}
}
class TestObjectBothPublic
{
public function __construct(public mixed $foo_bar = null, public mixed $fooBar = null)
{
}
public function getFooBar(): mixed
{
return $this->foo_bar;
}
}
class TestObjectBlankGetter
{
public function __construct(public int|null $fooBar = null)
{
}
public function fooBar(): int|null
{
return $this->fooBar;
}
}
class TestObjectWithPrivatePropertyInParentClass extends TestObjectPrivatePropertyOnly
{
}