Skip to content

Commit ab189cb

Browse files
nicolas-grekasnicolas-grekas
committed
[VarExporter] Fix support for hooks and asymmetric visibility
1 parent 647dba0 commit ab189cb

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)