Skip to content

Commit 88649d9

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#59761 [VarExporter] Fix lazy objects with hooked properties (nicolas-grekas)
This PR was merged into the 6.4 branch. Discussion ---------- [VarExporter] Fix lazy objects with hooked properties | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#59153 | License | MIT With hook proxies when needed so that lazy objects are fully compatible with non-final hooks. Commits ------- 540253a [VarExporter] Fix lazy objects with hooked properties
2 parents 08a6428 + 540253a commit 88649d9

File tree

7 files changed

+211
-8
lines changed

7 files changed

+211
-8
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ public static function getPropertyScopes($class)
287287

288288
if (\ReflectionProperty::IS_PROTECTED & $flags) {
289289
$propertyScopes["\0*\0$name"] = $propertyScopes[$name];
290+
} elseif (\PHP_VERSION_ID >= 80400 && $property->getHooks()) {
291+
$propertyScopes[$name][] = true;
290292
}
291293
}
292294

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class LazyObjectRegistry
5050
public static function getClassResetters($class)
5151
{
5252
$classProperties = [];
53+
$hookedProperties = [];
5354

5455
if ((self::$classReflectors[$class] ??= new \ReflectionClass($class))->isInternal()) {
5556
$propertyScopes = [];
@@ -60,7 +61,13 @@ public static function getClassResetters($class)
6061
foreach ($propertyScopes as $key => [$scope, $name, $readonlyScope]) {
6162
$propertyScopes[$k = "\0$scope\0$name"] ?? $propertyScopes[$k = "\0*\0$name"] ?? $k = $name;
6263

63-
if ($k === $key && "\0$class\0lazyObjectState" !== $k) {
64+
if ($k !== $key || "\0$class\0lazyObjectState" === $k) {
65+
continue;
66+
}
67+
68+
if ($k === $name && ($propertyScopes[$k][4] ?? false)) {
69+
$hookedProperties[$k] = true;
70+
} else {
6471
$classProperties[$readonlyScope ?? $scope][$name] = $key;
6572
}
6673
}
@@ -76,9 +83,13 @@ public static function getClassResetters($class)
7683
}, null, $scope);
7784
}
7885

79-
$resetters[] = static function ($instance, $skippedProperties, $onlyProperties = null) {
86+
$resetters[] = static function ($instance, $skippedProperties, $onlyProperties = null) use ($hookedProperties) {
8087
foreach ((array) $instance as $name => $value) {
81-
if ("\0" !== ($name[0] ?? '') && !\array_key_exists($name, $skippedProperties) && (null === $onlyProperties || \array_key_exists($name, $onlyProperties))) {
88+
if ("\0" !== ($name[0] ?? '')
89+
&& !\array_key_exists($name, $skippedProperties)
90+
&& (null === $onlyProperties || \array_key_exists($name, $onlyProperties))
91+
&& !isset($hookedProperties[$name])
92+
) {
8293
unset($instance->$name);
8394
}
8495
}

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

Lines changed: 104 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,37 @@ public static function generateLazyGhost(\ReflectionClass $class): string
5858
throw new LogicException(sprintf('Cannot generate lazy ghost: class "%s" extends "%s" which is internal.', $class->name, $parent->name));
5959
}
6060
}
61+
62+
$hooks = '';
63+
$propertyScopes = Hydrator::$propertyScopes[$class->name] ??= Hydrator::getPropertyScopes($class->name);
64+
foreach ($propertyScopes as $name => $scope) {
65+
if (!isset($scope[4]) || ($p = $scope[3])->isVirtual()) {
66+
continue;
67+
}
68+
69+
$type = self::exportType($p);
70+
$hooks .= "\n public {$type} \${$name} {\n";
71+
72+
foreach ($p->getHooks() as $hook => $method) {
73+
if ($method->isFinal()) {
74+
throw new LogicException(sprintf('Cannot generate lazy ghost: hook "%s::%s()" is final.', $class->name, $method->name));
75+
}
76+
77+
if ('get' === $hook) {
78+
$ref = ($method->returnsReference() ? '&' : '');
79+
$hooks .= " {$ref}get { \$this->initializeLazyObject(); return parent::\${$name}::get(); }\n";
80+
} elseif ('set' === $hook) {
81+
$parameters = self::exportParameters($method, true);
82+
$arg = '$'.$method->getParameters()[0]->name;
83+
$hooks .= " set({$parameters}) { \$this->initializeLazyObject(); parent::\${$name}::set({$arg}); }\n";
84+
} else {
85+
throw new LogicException(sprintf('Cannot generate lazy ghost: hook "%s::%s()" is not supported.', $class->name, $method->name));
86+
}
87+
}
88+
89+
$hooks .= " }\n";
90+
}
91+
6192
$propertyScopes = self::exportPropertyScopes($class->name);
6293

6394
return <<<EOPHP
@@ -66,7 +97,7 @@ public static function generateLazyGhost(\ReflectionClass $class): string
6697
use \Symfony\Component\VarExporter\LazyGhostTrait;
6798
6899
private const LAZY_OBJECT_PROPERTY_SCOPES = {$propertyScopes};
69-
}
100+
{$hooks}}
70101
71102
// Help opcache.preload discover always-needed symbols
72103
class_exists(\Symfony\Component\VarExporter\Internal\Hydrator::class);
@@ -95,14 +126,74 @@ public static function generateLazyProxy(?\ReflectionClass $class, array $interf
95126
throw new LogicException(sprintf('Cannot generate lazy proxy with PHP < 8.3: class "%s" is readonly.', $class->name));
96127
}
97128

129+
$hookedProperties = [];
130+
if (\PHP_VERSION_ID >= 80400 && $class) {
131+
$propertyScopes = Hydrator::$propertyScopes[$class->name] ??= Hydrator::getPropertyScopes($class->name);
132+
foreach ($propertyScopes as $name => $scope) {
133+
if (isset($scope[4]) && !($p = $scope[3])->isVirtual()) {
134+
$hookedProperties[$name] = [$p, $p->getHooks()];
135+
}
136+
}
137+
}
138+
98139
$methodReflectors = [$class?->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) ?? []];
99140
foreach ($interfaces as $interface) {
100141
if (!$interface->isInterface()) {
101142
throw new LogicException(sprintf('Cannot generate lazy proxy: "%s" is not an interface.', $interface->name));
102143
}
103144
$methodReflectors[] = $interface->getMethods();
145+
146+
if (\PHP_VERSION_ID >= 80400 && !$class) {
147+
foreach ($interface->getProperties() as $p) {
148+
$hookedProperties[$p->name] ??= [$p, []];
149+
$hookedProperties[$p->name][1] += $p->getHooks();
150+
}
151+
}
152+
}
153+
154+
$hooks = '';
155+
foreach ($hookedProperties as $name => [$p, $methods]) {
156+
$type = self::exportType($p);
157+
$hooks .= "\n public {$type} \${$p->name} {\n";
158+
159+
foreach ($methods as $hook => $method) {
160+
if ($method->isFinal()) {
161+
throw new LogicException(sprintf('Cannot generate lazy proxy: hook "%s::%s()" is final.', $class->name, $method->name));
162+
}
163+
164+
if ('get' === $hook) {
165+
$ref = ($method->returnsReference() ? '&' : '');
166+
$hooks .= <<<EOPHP
167+
{$ref}get {
168+
if (isset(\$this->lazyObjectState)) {
169+
return (\$this->lazyObjectState->realInstance ??= (\$this->lazyObjectState->initializer)())->{$p->name};
170+
}
171+
172+
return parent::\${$p->name}::get();
173+
}
174+
175+
EOPHP;
176+
} elseif ('set' === $hook) {
177+
$parameters = self::exportParameters($method, true);
178+
$arg = '$'.$method->getParameters()[0]->name;
179+
$hooks .= <<<EOPHP
180+
set({$parameters}) {
181+
if (isset(\$this->lazyObjectState)) {
182+
\$this->lazyObjectState->realInstance ??= (\$this->lazyObjectState->initializer)();
183+
\$this->lazyObjectState->realInstance->{$p->name} = {$arg};
184+
}
185+
186+
parent::\${$p->name}::set({$arg});
187+
}
188+
189+
EOPHP;
190+
} else {
191+
throw new LogicException(sprintf('Cannot generate lazy proxy: hook "%s::%s()" is not supported.', $class->name, $method->name));
192+
}
193+
}
194+
195+
$hooks .= " }\n";
104196
}
105-
$methodReflectors = array_merge(...$methodReflectors);
106197

107198
$extendsInternalClass = false;
108199
if ($parent = $class) {
@@ -112,6 +203,7 @@ public static function generateLazyProxy(?\ReflectionClass $class, array $interf
112203
}
113204
$methodsHaveToBeProxied = $extendsInternalClass;
114205
$methods = [];
206+
$methodReflectors = array_merge(...$methodReflectors);
115207

116208
foreach ($methodReflectors as $method) {
117209
if ('__get' !== strtolower($method->name) || 'mixed' === ($type = self::exportType($method) ?? 'mixed')) {
@@ -228,7 +320,7 @@ public function __unserialize(\$data): void
228320
{$lazyProxyTraitStatement}
229321
230322
private const LAZY_OBJECT_PROPERTY_SCOPES = {$propertyScopes};
231-
{$body}}
323+
{$hooks}{$body}}
232324
233325
// Help opcache.preload discover always-needed symbols
234326
class_exists(\Symfony\Component\VarExporter\Internal\Hydrator::class);
@@ -238,7 +330,7 @@ class_exists(\Symfony\Component\VarExporter\Internal\LazyObjectState::class);
238330
EOPHP;
239331
}
240332

241-
public static function exportSignature(\ReflectionFunctionAbstract $function, bool $withParameterTypes = true, ?string &$args = null): string
333+
public static function exportParameters(\ReflectionFunctionAbstract $function, bool $withParameterTypes = true, ?string &$args = null): string
242334
{
243335
$byRefIndex = 0;
244336
$args = '';
@@ -268,8 +360,15 @@ public static function exportSignature(\ReflectionFunctionAbstract $function, bo
268360
$args = implode(', ', $args);
269361
}
270362

363+
return implode(', ', $parameters);
364+
}
365+
366+
public static function exportSignature(\ReflectionFunctionAbstract $function, bool $withParameterTypes = true, ?string &$args = null): string
367+
{
368+
$parameters = self::exportParameters($function, $withParameterTypes, $args);
369+
271370
$signature = 'function '.($function->returnsReference() ? '&' : '')
272-
.($function->isClosure() ? '' : $function->name).'('.implode(', ', $parameters).')';
371+
.($function->isClosure() ? '' : $function->name).'('.$parameters.')';
273372

274373
if ($function instanceof \ReflectionMethod) {
275374
$signature = ($function->isPublic() ? 'public ' : ($function->isProtected() ? 'protected ' : 'private '))

‎src/Symfony/Component/VarExporter/Tests/Fixtures/Hooked.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\VarExporter\Tests\Fixtures;
13+
14+
class Hooked
15+
{
16+
public int $notBacked {
17+
get { return 123; }
18+
set { throw \LogicException('Cannot set value.'); }
19+
}
20+
21+
public int $backed {
22+
get { return $this->backed ??= 234; }
23+
set { $this->backed = $value; }
24+
}
25+
}

‎src/Symfony/Component/VarExporter/Tests/LazyGhostTraitTest.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
1818
use Symfony\Component\VarExporter\Internal\LazyObjectState;
1919
use Symfony\Component\VarExporter\ProxyHelper;
20+
use Symfony\Component\VarExporter\Tests\Fixtures\Hooked;
2021
use Symfony\Component\VarExporter\Tests\Fixtures\LazyGhost\ChildMagicClass;
2122
use Symfony\Component\VarExporter\Tests\Fixtures\LazyGhost\ChildStdClass;
2223
use Symfony\Component\VarExporter\Tests\Fixtures\LazyGhost\ChildTestClass;
@@ -478,6 +479,31 @@ public function testNormalization()
478479
$this->assertSame(['property' => 'property', 'method' => 'method'], $output);
479480
}
480481

482+
/**
483+
* @requires PHP 8.4
484+
*/
485+
public function testPropertyHooks()
486+
{
487+
$initialized = false;
488+
$object = $this->createLazyGhost(Hooked::class, function ($instance) use (&$initialized) {
489+
$initialized = true;
490+
});
491+
492+
$this->assertSame(123, $object->notBacked);
493+
$this->assertFalse($initialized);
494+
$this->assertSame(234, $object->backed);
495+
$this->assertTrue($initialized);
496+
497+
$initialized = false;
498+
$object = $this->createLazyGhost(Hooked::class, function ($instance) use (&$initialized) {
499+
$initialized = true;
500+
});
501+
502+
$object->backed = 345;
503+
$this->assertTrue($initialized);
504+
$this->assertSame(345, $object->backed);
505+
}
506+
481507
/**
482508
* @template T
483509
*

‎src/Symfony/Component/VarExporter/Tests/LazyProxyTraitTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\VarExporter\Exception\LogicException;
1919
use Symfony\Component\VarExporter\LazyProxyTrait;
2020
use Symfony\Component\VarExporter\ProxyHelper;
21+
use Symfony\Component\VarExporter\Tests\Fixtures\Hooked;
2122
use Symfony\Component\VarExporter\Tests\Fixtures\LazyProxy\FinalPublicClass;
2223
use Symfony\Component\VarExporter\Tests\Fixtures\LazyProxy\ReadOnlyClass;
2324
use Symfony\Component\VarExporter\Tests\Fixtures\LazyProxy\StringMagicGetClass;
@@ -298,6 +299,33 @@ public function testNormalization()
298299
$this->assertSame(['property' => 'property', 'method' => 'method'], $output);
299300
}
300301

302+
/**
303+
* @requires PHP 8.4
304+
*/
305+
public function testPropertyHooks()
306+
{
307+
$initialized = false;
308+
$object = $this->createLazyProxy(Hooked::class, function () use (&$initialized) {
309+
$initialized = true;
310+
return new Hooked();
311+
});
312+
313+
$this->assertSame(123, $object->notBacked);
314+
$this->assertFalse($initialized);
315+
$this->assertSame(234, $object->backed);
316+
$this->assertTrue($initialized);
317+
318+
$initialized = false;
319+
$object = $this->createLazyProxy(Hooked::class, function () use (&$initialized) {
320+
$initialized = true;
321+
return new Hooked();
322+
});
323+
324+
$object->backed = 345;
325+
$this->assertTrue($initialized);
326+
$this->assertSame(345, $object->backed);
327+
}
328+
301329
/**
302330
* @template T
303331
*

‎src/Symfony/Component/VarExporter/Tests/ProxyHelperTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\VarExporter\Exception\LogicException;
1616
use Symfony\Component\VarExporter\ProxyHelper;
17+
use Symfony\Component\VarExporter\Tests\Fixtures\Hooked;
1718
use Symfony\Component\VarExporter\Tests\Fixtures\LazyProxy\Php82NullStandaloneReturnType;
1819
use Symfony\Component\VarExporter\Tests\Fixtures\LazyProxy\StringMagicGetClass;
1920

@@ -246,6 +247,17 @@ public function testNullStandaloneReturnType()
246247
ProxyHelper::generateLazyProxy(new \ReflectionClass(Php82NullStandaloneReturnType::class))
247248
);
248249
}
250+
251+
/**
252+
* @requires PHP 8.4
253+
*/
254+
public function testPropertyHooks()
255+
{
256+
self::assertStringContainsString(
257+
"[parent::class, 'backed', null, 4 => true]",
258+
ProxyHelper::generateLazyProxy(new \ReflectionClass(Hooked::class))
259+
);
260+
}
249261
}
250262

251263
abstract class TestForProxyHelper

0 commit comments

Comments
 (0)