Skip to content

Commit 5ae227c

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#59965 [VarExporter] Fix support for hooks and asymmetric visibility (nicolas-grekas)
This PR was merged into the 6.4 branch. Discussion ---------- [VarExporter] Fix support for hooks and asymmetric visibility | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#59907 | License | MIT It took me several days to figure out this patch 😓 This PR fixes support for hooks and asymmetric visibility, which add quite some complexity for lazy objects but also for the exporter/hydrator. > [!NOTE] > Properties with a private "set" hook aren't supported and won't be in Symfony 6.4. To support them, we'll need help from native lazy objects, and this is my next PR, but for branch 7.3. Commits ------- ab189cb [VarExporter] Fix support for hooks and asymmetric visibility
2 parents 647dba0 + ab189cb commit 5ae227c

18 files changed

+251
-134
lines changed

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_wither_lazy.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class WitherProxy580fe0f extends \Symfony\Component\DependencyInjection\Tests\Co
7676
use \Symfony\Component\VarExporter\LazyProxyTrait;
7777

7878
private const LAZY_OBJECT_PROPERTY_SCOPES = [
79-
'foo' => [parent::class, 'foo', null],
79+
'foo' => [parent::class, 'foo', null, 4],
8080
];
8181
}
8282

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_wither_lazy_non_shared.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ class WitherProxyDd381be extends \Symfony\Component\DependencyInjection\Tests\Co
7878
use \Symfony\Component\VarExporter\LazyProxyTrait;
7979

8080
private const LAZY_OBJECT_PROPERTY_SCOPES = [
81-
'foo' => [parent::class, 'foo', null],
81+
'foo' => [parent::class, 'foo', null, 4],
8282
];
8383
}
8484

‎src/Symfony/Component/DependencyInjection/composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"psr/container": "^1.1|^2.0",
2121
"symfony/deprecation-contracts": "^2.5|^3",
2222
"symfony/service-contracts": "^2.5|^3.0",
23-
"symfony/var-exporter": "^6.2.10|^7.0"
23+
"symfony/var-exporter": "^6.4.20|^7.2.5"
2424
},
2525
"require-dev": {
2626
"symfony/yaml": "^5.4|^6.0|^7.0",

‎src/Symfony/Component/VarExporter/Internal/Exporter.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ public static function prepare($values, $objectsPool, &$refsPool, &$objectsCount
145145
$i = 0;
146146
$n = (string) $name;
147147
if ('' === $n || "\0" !== $n[0]) {
148-
$c = $reflector->hasProperty($n) && ($p = $reflector->getProperty($n))->isReadOnly() ? $p->class : 'stdClass';
148+
$p = $reflector->hasProperty($n) ? $reflector->getProperty($n) : null;
149+
$c = $p && (\PHP_VERSION_ID >= 80400 ? $p->isProtectedSet() || $p->isPrivateSet() : $p->isReadOnly()) ? $p->class : 'stdClass';
149150
} elseif ('*' === $n[1]) {
150151
$n = substr($n, 3);
151152
$c = $reflector->getProperty($n)->class;

‎src/Symfony/Component/VarExporter/Internal/Hydrator.php‎

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
*/
2121
class Hydrator
2222
{
23+
public const PROPERTY_HAS_HOOKS = 1;
24+
public const PROPERTY_NOT_BY_REF = 2;
25+
2326
public static array $hydrators = [];
2427
public static array $simpleHydrators = [];
2528
public static array $propertyScopes = [];
@@ -156,13 +159,16 @@ public static function getHydrator($class)
156159
public static function getSimpleHydrator($class)
157160
{
158161
$baseHydrator = self::$simpleHydrators['stdClass'] ??= (function ($properties, $object) {
159-
$readonly = (array) $this;
162+
$notByRef = (array) $this;
160163

161164
foreach ($properties as $name => &$value) {
162-
$object->$name = $value;
163-
164-
if (!($readonly[$name] ?? false)) {
165+
if (!$noRef = $notByRef[$name] ?? false) {
166+
$object->$name = $value;
165167
$object->$name = &$value;
168+
} elseif (true !== $noRef) {
169+
$notByRef($object, $value);
170+
} else {
171+
$object->$name = $value;
166172
}
167173
}
168174
})->bindTo(new \stdClass());
@@ -217,14 +223,19 @@ public static function getSimpleHydrator($class)
217223
}
218224

219225
if (!$classReflector->isInternal()) {
220-
$readonly = new \stdClass();
221-
foreach ($classReflector->getProperties(\ReflectionProperty::IS_READONLY) as $propertyReflector) {
222-
if ($class === $propertyReflector->class) {
223-
$readonly->{$propertyReflector->name} = true;
226+
$notByRef = new \stdClass();
227+
foreach ($classReflector->getProperties() as $propertyReflector) {
228+
if ($propertyReflector->isStatic()) {
229+
continue;
230+
}
231+
if (\PHP_VERSION_ID >= 80400 && !$propertyReflector->isAbstract() && $propertyReflector->getHooks()) {
232+
$notByRef->{$propertyReflector->name} = $propertyReflector->setRawValue(...);
233+
} elseif ($propertyReflector->isReadOnly()) {
234+
$notByRef->{$propertyReflector->name} = true;
224235
}
225236
}
226237

227-
return $baseHydrator->bindTo($readonly, $class);
238+
return $baseHydrator->bindTo($notByRef, $class);
228239
}
229240

230241
if ($classReflector->name !== $class) {
@@ -269,43 +280,47 @@ public static function getPropertyScopes($class)
269280
continue;
270281
}
271282
$name = $property->name;
283+
$access = ($flags << 2) | ($flags & \ReflectionProperty::IS_READONLY ? self::PROPERTY_NOT_BY_REF : 0);
284+
285+
if (\PHP_VERSION_ID >= 80400 && !$property->isAbstract() && $h = $property->getHooks()) {
286+
$access |= self::PROPERTY_HAS_HOOKS | (isset($h['get']) && !$h['get']->returnsReference() ? self::PROPERTY_NOT_BY_REF : 0);
287+
}
272288

273289
if (\ReflectionProperty::IS_PRIVATE & $flags) {
274-
$writeScope = null;
275-
if (\PHP_VERSION_ID >= 80400 ? $property->isPrivateSet() : ($flags & \ReflectionProperty::IS_READONLY)) {
276-
$writeScope = $class;
277-
}
278-
$propertyScopes["\0$class\0$name"] = $propertyScopes[$name] = [$class, $name, $writeScope, $property];
290+
$propertyScopes["\0$class\0$name"] = $propertyScopes[$name] = [$class, $name, null, $access, $property];
279291

280292
continue;
281293
}
282-
$writeScope = null;
283-
if (\PHP_VERSION_ID >= 80400 ? $property->isProtectedSet() || $property->isPrivateSet() : ($flags & \ReflectionProperty::IS_READONLY)) {
284-
$writeScope = $property->class;
294+
295+
$propertyScopes[$name] = [$class, $name, null, $access, $property];
296+
297+
if ($flags & (\PHP_VERSION_ID >= 80400 ? \ReflectionProperty::IS_PRIVATE_SET : \ReflectionProperty::IS_READONLY)) {
298+
$propertyScopes[$name][2] = $property->class;
285299
}
286-
$propertyScopes[$name] = [$class, $name, $writeScope, $property];
287300

288301
if (\ReflectionProperty::IS_PROTECTED & $flags) {
289302
$propertyScopes["\0*\0$name"] = $propertyScopes[$name];
290-
} elseif (\PHP_VERSION_ID >= 80400 && $property->getHooks()) {
291-
$propertyScopes[$name][4] = true;
292303
}
293304
}
294305

295306
while ($r = $r->getParentClass()) {
296307
$class = $r->name;
297308

298309
foreach ($r->getProperties(\ReflectionProperty::IS_PRIVATE) as $property) {
299-
if (!$property->isStatic()) {
300-
$name = $property->name;
301-
if (\PHP_VERSION_ID < 80400) {
302-
$writeScope = $property->isReadOnly() ? $class : null;
303-
} else {
304-
$writeScope = $property->isPrivateSet() ? $class : null;
305-
}
306-
$propertyScopes["\0$class\0$name"] = [$class, $name, $writeScope, $property];
307-
$propertyScopes[$name] ??= [$class, $name, $writeScope, $property];
310+
$flags = $property->getModifiers();
311+
312+
if (\ReflectionProperty::IS_STATIC & $flags) {
313+
continue;
314+
}
315+
$name = $property->name;
316+
$access = ($flags << 2) | ($flags & \ReflectionProperty::IS_READONLY ? self::PROPERTY_NOT_BY_REF : 0);
317+
318+
if (\PHP_VERSION_ID >= 80400 && $h = $property->getHooks()) {
319+
$access |= self::PROPERTY_HAS_HOOKS | (isset($h['get']) && !$h['get']->returnsReference() ? self::PROPERTY_NOT_BY_REF : 0);
308320
}
321+
322+
$propertyScopes["\0$class\0$name"] = [$class, $name, null, $access, $property];
323+
$propertyScopes[$name] ??= $propertyScopes["\0$class\0$name"];
309324
}
310325
}
311326

‎src/Symfony/Component/VarExporter/Internal/LazyObjectRegistry.php‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ public static function getClassResetters($class)
5858
$propertyScopes = Hydrator::$propertyScopes[$class] ??= Hydrator::getPropertyScopes($class);
5959
}
6060

61-
foreach ($propertyScopes as $key => [$scope, $name, $writeScope]) {
61+
foreach ($propertyScopes as $key => [$scope, $name, $writeScope, $access]) {
6262
$propertyScopes[$k = "\0$scope\0$name"] ?? $propertyScopes[$k = "\0*\0$name"] ?? $k = $name;
6363

6464
if ($k !== $key || "\0$class\0lazyObjectState" === $k) {
6565
continue;
6666
}
6767

68-
if ($k === $name && ($propertyScopes[$k][4] ?? false)) {
68+
if ($access & Hydrator::PROPERTY_HAS_HOOKS) {
6969
$hookedProperties[$k] = true;
7070
} else {
7171
$classProperties[$writeScope ?? $scope][$name] = $key;
@@ -101,8 +101,8 @@ public static function getClassResetters($class)
101101
public static function getClassAccessors($class)
102102
{
103103
return \Closure::bind(static fn () => [
104-
'get' => static function &($instance, $name, $readonly) {
105-
if (!$readonly) {
104+
'get' => static function &($instance, $name, $notByRef) {
105+
if (!$notByRef) {
106106
return $instance->$name;
107107
}
108108
$value = $instance->$name;
@@ -138,17 +138,37 @@ public static function getParentMethods($class)
138138
return $methods;
139139
}
140140

141-
public static function getScope($propertyScopes, $class, $property, $writeScope = null)
141+
public static function getScopeForRead($propertyScopes, $class, $property)
142142
{
143-
if (null === $writeScope && !isset($propertyScopes[$k = "\0$class\0$property"]) && !isset($propertyScopes[$k = "\0*\0$property"])) {
143+
if (!isset($propertyScopes[$k = "\0$class\0$property"]) && !isset($propertyScopes[$k = "\0*\0$property"])) {
144144
return null;
145145
}
146146
$frame = debug_backtrace(\DEBUG_BACKTRACE_PROVIDE_OBJECT | \DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2];
147147

148148
if (\ReflectionProperty::class === $scope = $frame['class'] ?? \Closure::class) {
149149
$scope = $frame['object']->class;
150150
}
151-
if (null === $writeScope && '*' === $k[1] && ($class === $scope || (is_subclass_of($class, $scope) && !isset($propertyScopes["\0$scope\0$property"])))) {
151+
if ('*' === $k[1] && ($class === $scope || (is_subclass_of($class, $scope) && !isset($propertyScopes["\0$scope\0$property"])))) {
152+
return null;
153+
}
154+
155+
return $scope;
156+
}
157+
158+
public static function getScopeForWrite($propertyScopes, $class, $property, $flags)
159+
{
160+
if (!($flags & (\ReflectionProperty::IS_PRIVATE | \ReflectionProperty::IS_PROTECTED | \ReflectionProperty::IS_READONLY | (\PHP_VERSION_ID >= 80400 ? \ReflectionProperty::IS_PRIVATE_SET | \ReflectionProperty::IS_PROTECTED_SET : 0)))) {
161+
return null;
162+
}
163+
$frame = debug_backtrace(\DEBUG_BACKTRACE_PROVIDE_OBJECT | \DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2];
164+
165+
if (\ReflectionProperty::class === $scope = $frame['class'] ?? \Closure::class) {
166+
$scope = $frame['object']->class;
167+
}
168+
if ($flags & (\ReflectionProperty::IS_PRIVATE | (\PHP_VERSION_ID >= 80400 ? \ReflectionProperty::IS_PRIVATE_SET : \ReflectionProperty::IS_READONLY))) {
169+
return $scope;
170+
}
171+
if ($flags & (\ReflectionProperty::IS_PROTECTED | (\PHP_VERSION_ID >= 80400 ? \ReflectionProperty::IS_PROTECTED_SET : 0)) && ($class === $scope || (is_subclass_of($class, $scope) && !isset($propertyScopes["\0$scope\0$property"])))) {
152172
return null;
153173
}
154174

‎src/Symfony/Component/VarExporter/Internal/LazyObjectState.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,21 @@ public function __construct(public readonly \Closure|array $initializer, $skippe
4545
$this->status = \is_array($initializer) ? self::STATUS_UNINITIALIZED_PARTIAL : self::STATUS_UNINITIALIZED_FULL;
4646
}
4747

48-
public function initialize($instance, $propertyName, $propertyScope)
48+
public function initialize($instance, $propertyName, $writeScope)
4949
{
5050
if (self::STATUS_INITIALIZED_FULL === $this->status) {
5151
return self::STATUS_INITIALIZED_FULL;
5252
}
5353

5454
if (\is_array($this->initializer)) {
5555
$class = $instance::class;
56-
$propertyScope ??= $class;
56+
$writeScope ??= $class;
5757
$propertyScopes = Hydrator::$propertyScopes[$class];
58-
$propertyScopes[$k = "\0$propertyScope\0$propertyName"] ?? $propertyScopes[$k = "\0*\0$propertyName"] ?? $k = $propertyName;
58+
$propertyScopes[$k = "\0$writeScope\0$propertyName"] ?? $propertyScopes[$k = "\0*\0$propertyName"] ?? $k = $propertyName;
5959

6060
if ($initializer = $this->initializer[$k] ?? null) {
61-
$value = $initializer(...[$instance, $propertyName, $propertyScope, LazyObjectRegistry::$defaultProperties[$class][$k] ?? null]);
62-
$accessor = LazyObjectRegistry::$classAccessors[$propertyScope] ??= LazyObjectRegistry::getClassAccessors($propertyScope);
61+
$value = $initializer(...[$instance, $propertyName, $writeScope, LazyObjectRegistry::$defaultProperties[$class][$k] ?? null]);
62+
$accessor = LazyObjectRegistry::$classAccessors[$writeScope] ??= LazyObjectRegistry::getClassAccessors($writeScope);
6363
$accessor['set']($instance, $propertyName, $value);
6464

6565
return $this->status = self::STATUS_INITIALIZED_PARTIAL;
@@ -72,7 +72,7 @@ public function initialize($instance, $propertyName, $propertyScope)
7272
$properties = (array) $instance;
7373
foreach ($values as $key => $value) {
7474
if (!\array_key_exists($key, $properties) && [$scope, $name, $writeScope] = $propertyScopes[$key] ?? null) {
75-
$scope = $writeScope ?? ('*' !== $scope ? $scope : $class);
75+
$scope = $writeScope ?? $scope;
7676
$accessor = LazyObjectRegistry::$classAccessors[$scope] ??= LazyObjectRegistry::getClassAccessors($scope);
7777
$accessor['set']($instance, $name, $value);
7878

@@ -116,10 +116,10 @@ public function reset($instance): void
116116
$properties = (array) $instance;
117117
$onlyProperties = \is_array($this->initializer) ? $this->initializer : null;
118118

119-
foreach ($propertyScopes as $key => [$scope, $name, $writeScope]) {
119+
foreach ($propertyScopes as $key => [$scope, $name, , $access]) {
120120
$propertyScopes[$k = "\0$scope\0$name"] ?? $propertyScopes[$k = "\0*\0$name"] ?? $k = $name;
121121

122-
if ($k === $key && (null !== $writeScope || !\array_key_exists($k, $properties))) {
122+
if ($k === $key && ($access & Hydrator::PROPERTY_HAS_HOOKS || ($access >> 2) & \ReflectionProperty::IS_READONLY || !\array_key_exists($k, $properties))) {
123123
$skippedProperties[$k] = true;
124124
}
125125
}

‎src/Symfony/Component/VarExporter/LazyGhostTrait.php‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public function initializeLazyObject(): static
116116
if (\array_key_exists($key, $properties) || ![$scope, $name, $writeScope] = $propertyScopes[$key] ?? null) {
117117
continue;
118118
}
119-
$scope = $writeScope ?? ('*' !== $scope ? $scope : $class);
119+
$scope = $writeScope ?? $scope;
120120

121121
if (null === $values) {
122122
if (!\is_array($values = ($state->initializer["\0"])($this, Registry::$defaultProperties[$class]))) {
@@ -160,20 +160,26 @@ public function &__get($name): mixed
160160
{
161161
$propertyScopes = Hydrator::$propertyScopes[$this::class] ??= Hydrator::getPropertyScopes($this::class);
162162
$scope = null;
163+
$notByRef = 0;
163164

164-
if ([$class, , $writeScope] = $propertyScopes[$name] ?? null) {
165-
$scope = Registry::getScope($propertyScopes, $class, $name);
165+
if ([$class, , $writeScope, $access] = $propertyScopes[$name] ?? null) {
166+
$scope = Registry::getScopeForRead($propertyScopes, $class, $name);
166167
$state = $this->lazyObjectState ?? null;
167168

168169
if ($state && (null === $scope || isset($propertyScopes["\0$scope\0$name"]))) {
170+
$notByRef = $access & Hydrator::PROPERTY_NOT_BY_REF;
171+
169172
if (LazyObjectState::STATUS_INITIALIZED_FULL === $state->status) {
170173
// Work around php/php-src#12695
171174
$property = null === $scope ? $name : "\0$scope\0$name";
172-
$property = $propertyScopes[$property][3]
173-
?? Hydrator::$propertyScopes[$this::class][$property][3] = new \ReflectionProperty($scope ?? $class, $name);
175+
$property = $propertyScopes[$property][4]
176+
?? Hydrator::$propertyScopes[$this::class][$property][4] = new \ReflectionProperty($scope ?? $class, $name);
174177
} else {
175178
$property = null;
176179
}
180+
if (\PHP_VERSION_ID >= 80400 && !$notByRef && ($access >> 2) & \ReflectionProperty::IS_PRIVATE_SET) {
181+
$scope ??= $writeScope;
182+
}
177183

178184
if ($property?->isInitialized($this) ?? LazyObjectState::STATUS_UNINITIALIZED_PARTIAL !== $state->initialize($this, $name, $writeScope ?? $scope)) {
179185
goto get_in_scope;
@@ -199,7 +205,7 @@ public function &__get($name): mixed
199205

200206
try {
201207
if (null === $scope) {
202-
if (null === $writeScope) {
208+
if (!$notByRef) {
203209
return $this->$name;
204210
}
205211
$value = $this->$name;
@@ -208,7 +214,7 @@ public function &__get($name): mixed
208214
}
209215
$accessor = Registry::$classAccessors[$scope] ??= Registry::getClassAccessors($scope);
210216

211-
return $accessor['get']($this, $name, null !== $writeScope);
217+
return $accessor['get']($this, $name, $notByRef);
212218
} catch (\Error $e) {
213219
if (\Error::class !== $e::class || !str_starts_with($e->getMessage(), 'Cannot access uninitialized non-nullable property')) {
214220
throw $e;
@@ -223,7 +229,7 @@ public function &__get($name): mixed
223229

224230
$accessor['set']($this, $name, []);
225231

226-
return $accessor['get']($this, $name, null !== $writeScope);
232+
return $accessor['get']($this, $name, $notByRef);
227233
} catch (\Error) {
228234
if (preg_match('/^Cannot access uninitialized non-nullable property ([^ ]++) by reference$/', $e->getMessage(), $matches)) {
229235
throw new \Error('Typed property '.$matches[1].' must not be accessed before initialization', $e->getCode(), $e->getPrevious());
@@ -239,8 +245,8 @@ public function __set($name, $value): void
239245
$propertyScopes = Hydrator::$propertyScopes[$this::class] ??= Hydrator::getPropertyScopes($this::class);
240246
$scope = null;
241247

242-
if ([$class, , $writeScope] = $propertyScopes[$name] ?? null) {
243-
$scope = Registry::getScope($propertyScopes, $class, $name, $writeScope);
248+
if ([$class, , $writeScope, $access] = $propertyScopes[$name] ?? null) {
249+
$scope = Registry::getScopeForWrite($propertyScopes, $class, $name, $access >> 2);
244250
$state = $this->lazyObjectState ?? null;
245251

246252
if ($state && ($writeScope === $scope || isset($propertyScopes["\0$scope\0$name"]))
@@ -275,7 +281,7 @@ public function __isset($name): bool
275281
$scope = null;
276282

277283
if ([$class, , $writeScope] = $propertyScopes[$name] ?? null) {
278-
$scope = Registry::getScope($propertyScopes, $class, $name);
284+
$scope = Registry::getScopeForRead($propertyScopes, $class, $name);
279285
$state = $this->lazyObjectState ?? null;
280286

281287
if ($state && (null === $scope || isset($propertyScopes["\0$scope\0$name"]))
@@ -305,8 +311,8 @@ public function __unset($name): void
305311
$propertyScopes = Hydrator::$propertyScopes[$this::class] ??= Hydrator::getPropertyScopes($this::class);
306312
$scope = null;
307313

308-
if ([$class, , $writeScope] = $propertyScopes[$name] ?? null) {
309-
$scope = Registry::getScope($propertyScopes, $class, $name, $writeScope);
314+
if ([$class, , $writeScope, $access] = $propertyScopes[$name] ?? null) {
315+
$scope = Registry::getScopeForWrite($propertyScopes, $class, $name, $access >> 2);
310316
$state = $this->lazyObjectState ?? null;
311317

312318
if ($state && ($writeScope === $scope || isset($propertyScopes["\0$scope\0$name"]))

0 commit comments

Comments
 (0)