Skip to content

Commit f4e2368

Browse files
Brajk19HypeMC
Brajk19
authored and
HypeMC
committed
fix usage of nullsafe operator with collection accessor
1 parent 6a11e38 commit f4e2368

5 files changed

Lines changed: 174 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [1.3.1] - 2026-06-08
9+
10+
### Fixed
11+
12+
- Fix nullsafe operator usage with collection property accessors by @Brajk19
13+
in https://github.com/sofascore/purgatory-bundle/pull/140
14+
815
## [1.3.0] - 2025-12-15
916

1017
### Added
@@ -48,6 +55,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4855

4956
- Initial release
5057

58+
[1.3.1]: https://github.com/sofascore/purgatory-bundle/compare/v1.3.0...v1.3.1
5159
[1.3.0]: https://github.com/sofascore/purgatory-bundle/compare/v1.2.1...v1.3.0
5260
[1.2.1]: https://github.com/sofascore/purgatory-bundle/compare/v1.2.0...v1.2.1
5361
[1.2.0]: https://github.com/sofascore/purgatory-bundle/compare/v1.1.1...v1.2.0

‎phpstan-baseline.neon‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,12 @@ parameters:
138138
count: 1
139139
path: src/DependencyInjection/PurgatoryExtension.php
140140

141+
-
142+
message: '#^Comparison operation "\<\=" between 8 and 5 is always false\.$#'
143+
identifier: smallerOrEqual.alwaysFalse
144+
count: 1
145+
path: src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php
146+
141147
-
142148
message: '#^Trait Sofascore\\PurgatoryBundle\\Test\\InteractsWithPurgatory is used zero times and is not analysed\.$#'
143149
identifier: trait.unused

‎src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@
55
namespace Sofascore\PurgatoryBundle\RouteProvider\PropertyAccess;
66

77
use Sofascore\PurgatoryBundle\Exception\ValueNotIterableException;
8+
use Symfony\Component\HttpKernel\Kernel;
89
use Symfony\Component\PropertyAccess\Exception\AccessException;
10+
use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException;
911
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
1012
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
13+
use Symfony\Component\PropertyAccess\PropertyPath;
1114
use Symfony\Component\PropertyAccess\PropertyPathInterface;
1215

1316
/**
@@ -25,6 +28,11 @@ public function __construct(
2528
/**
2629
* @param object|array<array-key, mixed> $objectOrArray
2730
* @param string|PropertyPathInterface $propertyPath
31+
*
32+
* @throws InvalidArgumentException
33+
* @throws AccessException
34+
* @throws UnexpectedTypeException
35+
* @throws ValueNotIterableException
2836
*/
2937
public function getValue($objectOrArray, $propertyPath): mixed
3038
{
@@ -35,10 +43,19 @@ public function getValue($objectOrArray, $propertyPath): mixed
3543
/** @var array{0: string, 1: string} $propertyPathParts */
3644
$propertyPathParts = explode(separator: self::DELIMITER, string: (string) $propertyPath, limit: 2);
3745

38-
$collection = $this->propertyAccessor->getValue($objectOrArray, $propertyPathParts[0]);
46+
$basePropertyPath = new PropertyPath($propertyPathParts[0]);
47+
$collection = $this->propertyAccessor->getValue($objectOrArray, $basePropertyPath);
3948

4049
if (!is_iterable($collection)) {
41-
throw new ValueNotIterableException($collection, $propertyPathParts[0]);
50+
// Honor the null-safe operator: when the collection segment resolves to null because of
51+
// a null-safe short-circuit (e.g. "property?.collection[*].id" with a null "property"),
52+
// yield no values instead of throwing. A null collection not guarded by "?."
53+
// (e.g. "property.collection[*].id" where "collection" itself is null) is still an error.
54+
if (null === $collection && $this->isNullSafeCollection($objectOrArray, $basePropertyPath)) {
55+
return null;
56+
}
57+
58+
throw new ValueNotIterableException($collection, (string) $basePropertyPath);
4259
}
4360

4461
$values = [];
@@ -91,8 +108,29 @@ public function isReadable($objectOrArray, $propertyPath): bool
91108
$this->getValue($objectOrArray, $propertyPath);
92109

93110
return true;
94-
} catch (AccessException|UnexpectedTypeException) {
111+
} catch (AccessException|UnexpectedTypeException|ValueNotIterableException) {
95112
return false;
96113
}
97114
}
115+
116+
/**
117+
* Determines whether a null collection segment is the legitimate result of a null-safe ("?.")
118+
* short-circuit, rather than the collection itself resolving to null.
119+
*
120+
* @param object|array<array-key, mixed> $objectOrArray
121+
*/
122+
private function isNullSafeCollection(object|array $objectOrArray, PropertyPath $path): bool
123+
{
124+
if (Kernel::MAJOR_VERSION <= 5) {
125+
return false;
126+
}
127+
128+
if ($path->isNullSafe($path->getLength() - 1)) {
129+
return true;
130+
}
131+
132+
$parent = $path->getParent();
133+
134+
return null !== $parent && null === $this->propertyAccessor->getValue($objectOrArray, $parent);
135+
}
98136
}

‎tests/RouteProvider/PropertyAccess/Fixtures/Foo.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ class Foo
1111
public function __construct(
1212
public readonly int $id,
1313
public readonly Collection $children,
14+
public readonly ?self $linked = null,
15+
public readonly ?Collection $nullableChildren = null,
1416
) {
1517
}
1618

‎tests/RouteProvider/PropertyAccess/PurgatoryPropertyAccessorTest.php‎

Lines changed: 117 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Sofascore\PurgatoryBundle\Exception\ValueNotIterableException;
1212
use Sofascore\PurgatoryBundle\RouteProvider\PropertyAccess\PurgatoryPropertyAccessor;
1313
use Sofascore\PurgatoryBundle\Tests\RouteProvider\PropertyAccess\Fixtures\Foo;
14+
use Symfony\Component\HttpKernel\Kernel;
1415
use Symfony\Component\PropertyAccess\PropertyAccess;
1516

1617
#[CoversClass(PurgatoryPropertyAccessor::class)]
@@ -130,17 +131,127 @@ public static function traversableProvider(): iterable
130131
];
131132
}
132133

133-
public function testNotTraversable(): void
134+
#[DataProvider('nullSafeProvider')]
135+
public function testNullSafeCollectionPath(object $object, string $propertyPath, mixed $expectedResult): void
134136
{
137+
if (Kernel::MAJOR_VERSION <= 5) {
138+
self::markTestSkipped('Requires Symfony 6 or higher.');
139+
}
140+
141+
self::assertTrue($this->purgatoryPropertyAccessor->isReadable($object, $propertyPath));
142+
self::assertSame(
143+
expected: $expectedResult,
144+
actual: $this->purgatoryPropertyAccessor->getValue($object, $propertyPath),
145+
);
146+
}
147+
148+
public static function nullSafeProvider(): iterable
149+
{
150+
yield 'null-safe parent short-circuits to null' => [
151+
'object' => new Foo(
152+
id: 100,
153+
children: new ArrayCollection([]),
154+
linked: null,
155+
),
156+
'propertyPath' => 'linked?.children[*].id',
157+
'expectedResult' => null,
158+
];
159+
160+
yield 'null-safe parent present with populated collection yields values' => [
161+
'object' => new Foo(
162+
id: 100,
163+
children: new ArrayCollection([]),
164+
linked: new Foo(
165+
id: 1,
166+
children: new ArrayCollection([
167+
new Foo(id: 10, children: new ArrayCollection([])),
168+
new Foo(id: 11, children: new ArrayCollection([])),
169+
]),
170+
),
171+
),
172+
'propertyPath' => 'linked?.children[*].id',
173+
'expectedResult' => [10, 11],
174+
];
175+
176+
yield 'null-safe collection element resolving to null short-circuits to null' => [
177+
'object' => new Foo(
178+
id: 100,
179+
children: new ArrayCollection([]),
180+
nullableChildren: null,
181+
),
182+
'propertyPath' => 'nullableChildren?[*].id',
183+
'expectedResult' => null,
184+
];
185+
186+
yield 'nested null-safe short-circuit contributes a null entry, consistent with a scalar leaf' => [
187+
'object' => new Foo(
188+
id: 0,
189+
children: new ArrayCollection([
190+
new Foo(id: 1, children: new ArrayCollection([]), linked: null),
191+
new Foo(
192+
id: 2,
193+
children: new ArrayCollection([]),
194+
linked: new Foo(
195+
id: 20,
196+
children: new ArrayCollection([
197+
new Foo(id: 100, children: new ArrayCollection([])),
198+
new Foo(id: 101, children: new ArrayCollection([])),
199+
]),
200+
),
201+
),
202+
]),
203+
),
204+
'propertyPath' => 'children[*].linked?.children[*].id',
205+
'expectedResult' => [null, 100, 101],
206+
];
207+
}
208+
209+
#[DataProvider('notTraversableProvider')]
210+
public function testNotTraversableThrows(object $object, string $propertyPath, string $expectedMessage): void
211+
{
212+
self::assertFalse($this->purgatoryPropertyAccessor->isReadable($object, $propertyPath));
213+
135214
$this->expectException(ValueNotIterableException::class);
136-
$this->expectExceptionMessage('Expected an iterable, "int" given at property path "id[*]".');
215+
$this->expectExceptionMessage($expectedMessage);
216+
217+
$this->purgatoryPropertyAccessor->getValue($object, $propertyPath);
218+
}
137219

138-
$this->purgatoryPropertyAccessor->getValue(
139-
objectOrArray: new Foo(
220+
public static function notTraversableProvider(): iterable
221+
{
222+
yield 'scalar value is not iterable' => [
223+
'object' => new Foo(
140224
id: 1,
141225
children: new ArrayCollection([]),
142226
),
143-
propertyPath: 'id[*].id',
144-
);
227+
'propertyPath' => 'id[*].id',
228+
'expectedMessage' => 'Expected an iterable, "int" given at property path "id[*]".',
229+
];
230+
231+
yield 'non-null-safe collection resolving to null' => [
232+
'object' => new Foo(
233+
id: 100,
234+
children: new ArrayCollection([]),
235+
nullableChildren: null,
236+
),
237+
'propertyPath' => 'nullableChildren[*].id',
238+
'expectedMessage' => 'Expected an iterable, "null" given at property path "nullableChildren[*]".',
239+
];
240+
241+
if (Kernel::MAJOR_VERSION > 5) {
242+
yield 'null-safe parent present but non-null-safe collection resolving to null' => [
243+
'object' => new Foo(
244+
id: 100,
245+
children: new ArrayCollection([]),
246+
linked: new Foo(
247+
id: 1,
248+
children: new ArrayCollection([]),
249+
nullableChildren: null,
250+
),
251+
),
252+
'propertyPath' => 'linked?.nullableChildren[*].id',
253+
'expectedMessage' => 'Expected an iterable, "null" given at property path "linked?.nullableChildren[*]".',
254+
];
255+
}
145256
}
146257
}

0 commit comments

Comments
 (0)