From 81ac5a2c3ea2355d0c419f700ceda7ed5309779d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Brajkovi=C4=87?= Date: Tue, 2 Jun 2026 22:04:40 +0200 Subject: [PATCH] fix usage of nullsafe operator with collection accessor --- .../PurgatoryPropertyAccessor.php | 33 ++++- .../PropertyAccess/Fixtures/Foo.php | 2 + .../PurgatoryPropertyAccessorTest.php | 116 +++++++++++++++++- 3 files changed, 142 insertions(+), 9 deletions(-) diff --git a/src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php b/src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php index a1065349..591061d9 100644 --- a/src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php +++ b/src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php @@ -8,6 +8,7 @@ use Symfony\Component\PropertyAccess\Exception\AccessException; use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; +use Symfony\Component\PropertyAccess\PropertyPath; use Symfony\Component\PropertyAccess\PropertyPathInterface; /** @@ -34,10 +35,19 @@ public function getValue(object|array $objectOrArray, string|PropertyPathInterfa /** @var array{0: string, 1: string} $propertyPathParts */ $propertyPathParts = explode(separator: self::DELIMITER, string: (string) $propertyPath, limit: 2); - $collection = $this->propertyAccessor->getValue($objectOrArray, $propertyPathParts[0]); + $basePropertyPath = new PropertyPath($propertyPathParts[0]); + $collection = $this->propertyAccessor->getValue($objectOrArray, $basePropertyPath); if (!is_iterable($collection)) { - throw new ValueNotIterableException($collection, $propertyPathParts[0]); + // Honor the null-safe operator: when the collection segment resolves to null because of + // a null-safe short-circuit (e.g. "property?.collection[*].id" with a null "property"), + // yield no values instead of throwing. A null collection not guarded by "?." + // (e.g. "property.collection[*].id" where "collection" itself is null) is still an error. + if (null === $collection && $this->isNullSafeCollection($objectOrArray, $basePropertyPath)) { + return null; + } + + throw new ValueNotIterableException($collection, (string) $basePropertyPath); } $values = []; @@ -87,8 +97,25 @@ public function isReadable(object|array $objectOrArray, string|PropertyPathInter $this->getValue($objectOrArray, $propertyPath); return true; - } catch (AccessException|UnexpectedTypeException) { + } catch (AccessException|UnexpectedTypeException|ValueNotIterableException) { return false; } } + + /** + * Determines whether a null collection segment is the legitimate result of a null-safe ("?.") + * short-circuit, rather than the collection itself resolving to null. + * + * @param object|array $objectOrArray + */ + private function isNullSafeCollection(object|array $objectOrArray, PropertyPath $path): bool + { + if ($path->isNullSafe($path->getLength() - 1)) { + return true; + } + + $parent = $path->getParent(); + + return null !== $parent && null === $this->propertyAccessor->getValue($objectOrArray, $parent); + } } diff --git a/tests/RouteProvider/PropertyAccess/Fixtures/Foo.php b/tests/RouteProvider/PropertyAccess/Fixtures/Foo.php index ad52af1d..5b1d40f4 100644 --- a/tests/RouteProvider/PropertyAccess/Fixtures/Foo.php +++ b/tests/RouteProvider/PropertyAccess/Fixtures/Foo.php @@ -11,6 +11,8 @@ class Foo public function __construct( public readonly int $id, public readonly Collection $children, + public readonly ?self $linked = null, + public readonly ?Collection $nullableChildren = null, ) { } diff --git a/tests/RouteProvider/PropertyAccess/PurgatoryPropertyAccessorTest.php b/tests/RouteProvider/PropertyAccess/PurgatoryPropertyAccessorTest.php index 690dc189..4037173e 100644 --- a/tests/RouteProvider/PropertyAccess/PurgatoryPropertyAccessorTest.php +++ b/tests/RouteProvider/PropertyAccess/PurgatoryPropertyAccessorTest.php @@ -130,17 +130,121 @@ public static function traversableProvider(): iterable ]; } - public function testNotTraversable(): void + #[DataProvider('nullSafeProvider')] + public function testNullSafeCollectionPath(object $object, string $propertyPath, mixed $expectedResult): void { + self::assertTrue($this->purgatoryPropertyAccessor->isReadable($object, $propertyPath)); + self::assertSame( + expected: $expectedResult, + actual: $this->purgatoryPropertyAccessor->getValue($object, $propertyPath), + ); + } + + public static function nullSafeProvider(): iterable + { + yield 'null-safe parent short-circuits to null' => [ + 'object' => new Foo( + id: 100, + children: new ArrayCollection([]), + linked: null, + ), + 'propertyPath' => 'linked?.children[*].id', + 'expectedResult' => null, + ]; + + yield 'null-safe parent present with populated collection yields values' => [ + 'object' => new Foo( + id: 100, + children: new ArrayCollection([]), + linked: new Foo( + id: 1, + children: new ArrayCollection([ + new Foo(id: 10, children: new ArrayCollection([])), + new Foo(id: 11, children: new ArrayCollection([])), + ]), + ), + ), + 'propertyPath' => 'linked?.children[*].id', + 'expectedResult' => [10, 11], + ]; + + yield 'null-safe collection element resolving to null short-circuits to null' => [ + 'object' => new Foo( + id: 100, + children: new ArrayCollection([]), + nullableChildren: null, + ), + 'propertyPath' => 'nullableChildren?[*].id', + 'expectedResult' => null, + ]; + + yield 'nested null-safe short-circuit contributes a null entry, consistent with a scalar leaf' => [ + 'object' => new Foo( + id: 0, + children: new ArrayCollection([ + new Foo(id: 1, children: new ArrayCollection([]), linked: null), + new Foo( + id: 2, + children: new ArrayCollection([]), + linked: new Foo( + id: 20, + children: new ArrayCollection([ + new Foo(id: 100, children: new ArrayCollection([])), + new Foo(id: 101, children: new ArrayCollection([])), + ]), + ), + ), + ]), + ), + 'propertyPath' => 'children[*].linked?.children[*].id', + 'expectedResult' => [null, 100, 101], + ]; + } + + #[DataProvider('notTraversableProvider')] + public function testNotTraversableThrows(object $object, string $propertyPath, string $expectedMessage): void + { + self::assertFalse($this->purgatoryPropertyAccessor->isReadable($object, $propertyPath)); + $this->expectException(ValueNotIterableException::class); - $this->expectExceptionMessage('Expected an iterable, "int" given at property path "id[*]".'); + $this->expectExceptionMessage($expectedMessage); - $this->purgatoryPropertyAccessor->getValue( - objectOrArray: new Foo( + $this->purgatoryPropertyAccessor->getValue($object, $propertyPath); + } + + public static function notTraversableProvider(): iterable + { + yield 'scalar value is not iterable' => [ + 'object' => new Foo( id: 1, children: new ArrayCollection([]), ), - propertyPath: 'id[*].id', - ); + 'propertyPath' => 'id[*].id', + 'expectedMessage' => 'Expected an iterable, "int" given at property path "id[*]".', + ]; + + yield 'non-null-safe collection resolving to null' => [ + 'object' => new Foo( + id: 100, + children: new ArrayCollection([]), + nullableChildren: null, + ), + 'propertyPath' => 'nullableChildren[*].id', + 'expectedMessage' => 'Expected an iterable, "null" given at property path "nullableChildren[*]".', + ]; + + yield 'null-safe parent present but non-null-safe collection resolving to null' => [ + 'object' => new Foo( + id: 100, + children: new ArrayCollection([]), + linked: new Foo( + id: 1, + children: new ArrayCollection([]), + nullableChildren: null, + ), + ), + 'propertyPath' => 'linked?.nullableChildren[*].id', + 'expectedMessage' => 'Expected an iterable, "null" given at property path "linked?.nullableChildren[*]".', + ]; } }