8000 feature #30212 [DI] Add support for "wither" methods - for greater im… · symfony/symfony@539f4ca · GitHub
[go: up one dir, main page]
Skip to content

Commit 539f4ca

Browse files
fabpotfabpot
committed
feature #30212 [DI] Add support for "wither" methods - for greater immutable services (nicolas-grekas)
This PR was merged into the 4.3-dev branch. Discussion ---------- [DI] Add support for "wither" methods - for greater immutable services | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#10991 Let's say we want to define an immutable service while still using traits for composing its optional features. A nice way to do so without hitting [the downsides of setters](https://symfony.com/doc/current/service_container/injection_types.html#setter-injection) is to use withers. Here would be an example: ```php class MyService { use LoggerAwareTrait; } trait LoggerAwareTrait { private $logger; /** * @required * @return static */ public function withLogger(LoggerInterface $logger) { $new = clone $this; $new->logger = $logger; return $new; } } $service = new MyService(); $service = $service->withLogger($logger); ``` As you can see, this nicely solves the setter issues. BUT how do you make the service container create such a service? Right now, you need to resort to complex gymnastic using the "factory" setting - manageable for only one wither, but definitely not when more are involved and not compatible with autowiring. So here we are: this PR allows configuring such services seamlessly. Using explicit configuration, it adds a 3rd parameter to method calls configuration: after the method name and its parameters, you can pass `true` and done, you just declared a wither: ```yaml services: MyService: calls: - [withLogger, ['@logger'], true] ``` In XML, you could use the new `returns-clone` attribute on the `<call>` tag. And when using autowiring, the code looks for the `@return static` annotation and turns the flag on if found. There is only one limitation: unlike services with regular setters, services with withers cannot be part of circular loops that involve calls to wither methods (unless they're declared lazy of course). Commits ------- f455d1b [DI] Add support for "wither" methods - for greater immutable services
2 parents bce6124 + f455d1b commit 539f4ca

16 files changed

+286
-22
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,9 @@ private function getContainerDefinitionDocument(Definition $definition, string $
357357
foreach ($calls as $callData) {
358358
$callsXML->appendChild($callXML = $dom->createElement('call'));
359359
$callXML->setAttribute('method', $callData[0]);
360+
if ($callData[2] ?? false) {
361+
$callXML->setAttribute('returns-clone', 'true');
362+
}
360363
}
361364
}
362365

‎src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,41 @@ protected function processValue($value, $isRoot = false)
140140
$this->byConstructor = true;
141141
$this->processValue($value->getFactory());
142142
$this->processValue($value->getArguments());
143+
144+
$properties = $value->getProperties();
145+
$setters = $value->getMethodCalls();
146+
147+
// Any references before a "wither" are part of the constructor-instantiation graph
148+
$lastWitherIndex = null;
149+
foreach ($setters as $k => $call) {
150+
if ($call[2] ?? false) {
151+
$lastWitherIndex = $k;
152+
}
153+
}
154+
155+
if (null !== $lastWitherIndex) {
156+
$this->processValue($properties);
157+
$setters = $properties = [];
158+
159+
foreach ($value->getMethodCalls() as $k => $call) {
160+
if (null === $lastWitherIndex) {
161+
$setters[] = $call;
162+
continue;
163+
}
164+
165+
if ($lastWitherIndex === $k) {
166+
$lastWitherIndex = null;
167+
}
168+
169+
$this->processValue($call);
170+
}
171+
}
172+
143173
$this->byConstructor = $byConstructor;
144174

145175
if (!$this->onlyConstructorArguments) {
146-
$this->processValue($value->getProperties());
147-
$this->processValue($value->getMethodCalls());
176+
$this->processValue($properties);
177+
$this->processValue($setters);
148178
$this->processValue($value->getConfigurator());
149179
}
150180
$this->lazy = $lazy;

‎src/Symfony/Component/DependencyInjection/Compiler/AutowireRequiredMethodsPass.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ protected function processValue($value, $isRoot = false)
3535
}
3636

3737
$alreadyCalledMethods = [];
38+
$withers = [];
3839

3940
foreach ($value->getMethodCalls() as list($method)) {
4041
$alreadyCalledMethods[strtolower($method)] = true;
@@ -50,7 +51,11 @@ protected function processValue($value, $isRoot = false)
5051
while (true) {
5152
if (false !== $doc = $r->getDocComment()) {
5253
if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
53-
$value->addMethodCall($reflectionMethod->name);
54+
if (preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@return\s++static[\s\*]#i', $doc)) {
55+
$withers[] = [$reflectionMethod->name, [], true];
56+
} else {
57+
$value->addMethodCall($reflectionMethod->name, []);
58+
}
5459
break;
5560
}
5661
if (false === stripos($doc, '@inheritdoc') || !preg_match('#(?:^/\*\*|\n\s*+\*)\s*+(?:\{@inheritdoc\}|@inheritdoc)(?:\s|\*/$)#i', $doc)) {
@@ -65,6 +70,15 @@ protected function processValue($value, $isRoot = false)
6570
}
6671
}
6772

73+
if ($withers) {
74+
// Prepend withers to prevent creating circular loops
75+
$setters = $value->getMethodCalls();
76+
$value->setMethodCalls($withers);
77+
foreach ($setters as $call) {
78+
$value->addMethodCall($call[0], $call[1], $call[2] ?? false);
79+
}
80+
}
81+
6882
return $value;
6983
}
7084
}

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,8 +1139,15 @@ private function createService(Definition $definition, array &$inlineServices, $
11391139
}
11401140
}
11411141

1142-
if ($tryProxy || !$definition->isLazy()) {
1143-
// share only if proxying failed, or if not a proxy
1142+
$lastWitherIndex = null;
1143+
foreach ($definition->getMethodCalls() as $k => $call) {
1144+
if ($call[2] ?? false) {
1145+
$lastWitherIndex = $k;
1146+
}
1147+
}
1148+
1149+
if (null === $lastWitherIndex && ($tryProxy || !$definition->isLazy())) {
1150+
// share only if proxying failed, or if not a proxy, and if no withers are found
11441151
$this->shareService($definition, $service, $id, $inlineServices);
11451152
}
11461153

@@ -1149,8 +1156,13 @@ private function createService(Definition $definition, array &$inlineServices, $
11491156
$service->$name = $value;
11501157
}
11511158

1152-
foreach ($definition->getMethodCalls() as $call) {
1153-
$this->callMethod($service, $call, $inlineServices);
1159+
foreach ($definition->getMethodCalls() as $k => $call) {
1160+
$service = $this->callMethod($service, $call, $inlineServices);
1161+
1162+
if ($lastWitherIndex === $k && ($tryProxy || !$definition->isLazy())) {
1163+
// share only if proxying failed, or if not a proxy, and this is the last wither
1164+
$this->shareService($definition, $service, $id, $inlineServices);
1165+
}
11541166
}
11551167

11561168
if ($callable = $definition->getConfigurator()) {
@@ -1569,16 +1581,18 @@ private function callMethod($service, $call, array &$inlineServices)
15691581
{
15701582
foreach (self::getServiceConditionals($call[1]) as $s) {
15711583
if (!$this->has($s)) {
1572-
return;
1584+
return $service;
15731585
}
15741586
}
15751587
foreach (self::getInitializedConditionals($call[1]) as $s) {
15761588
if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE, $inlineServices)) {
1577-
return;
1589+
return $service;
15781590
}
15791591
}
15801592

1581-
$service->{$call[0]}(...$this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlineServices));
1593+
$result = $service->{$call[0]}(...$this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlineServices));
1594+
1595+
return empty($call[2]) ? $service : $result;
15821596
}
15831597

15841598
/**

‎src/Symfony/Component/DependencyInjection/Definition.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ public function setMethodCalls(array $calls = [])
330330
{
331331
$this->calls = [];
332332
foreach ($calls as $call) {
333-
$this->addMethodCall($call[0], $call[1]);
333+
$this->addMethodCall($call[0], $call[1], $call[2] ?? false);
334334
}
335335

336336
return $this;
@@ -339,19 +339,20 @@ public function setMethodCalls(array $calls = [])
339339
/**
340340
* Adds a method to call after service initialization.
341341
*
342-
* @param string $method The method name to call
343-
* @param array $arguments An array of arguments to pass to the method call
342+
* @param string $method The method name to call
343+
* @param array $arguments An array of arguments to pass to the method call
344+
* @param bool $returnsClone Whether the call returns the service instance or not
344345
*
345346
* @return $this
346347
*
347348
* @throws InvalidArgumentException on empty $method param
348349
*/
349-
public function addMethodCall($method, array $arguments = [])
350+
public function addMethodCall($method, array $arguments = []/*, bool $returnsClone = false*/)
350351
{
351352
if (empty($method)) {
352353
throw new InvalidArgumentException('Method name cannot be empty.');
353354
}
354-
$this->calls[] = [$method, $arguments];
355+
$this->calls[] = 2 < \func_num_args() && \func_get_arg(2) ? [$method, $arguments, true] : [$method, $arguments];
355356

356357
return $this;
357358
}

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,14 @@ private function addServiceInstance(string $id, Definition $definition, bool $is
506506
$isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition);
507507
$instantiation = '';
508508

509-
if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
509+
$lastWitherIndex = null;
510+
foreach ($definition->getMethodCalls() as $k => $call) {
511+
if ($call[2] ?? false) {
512+
$lastWitherIndex = $k;
513+
}
514+
}
515+
516+
if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id]) && null === $lastWitherIndex) {
510517
$instantiation = sprintf('$this->%s[\'%s\'] = %s', $this->container->getDefinition($id)->isPublic() ? 'services' : 'privates', $id, $isSimpleInstance ? '' : '$instance');
511518
} elseif (!$isSimpleInstance) {
512519
$instantiation = '$instance';
@@ -563,16 +570,32 @@ private function isTrivialInstance(Definition $definition): bool
563570
return true;
564571
}
565572

566-
private function addServiceMethodCalls(Definition $definition, string $variableName = 'instance'): string
573+
private function addServiceMethodCalls(Definition $definition, string $variableName, ?string $sharedNonLazyId): string
567574
{
575+
$lastWitherIndex = null;
576+
foreach ($definition->getMethodCalls() as $k => $call) {
577+
if ($call[2] ?? false) {
578+
$lastWitherIndex = $k;
579+
}
580+
}
581+
568582
$calls = '';
569-
foreach ($definition->getMethodCalls() as $call) {
583+
foreach ($definition->getMethodCalls() as $k => $call) {
570584
$arguments = [];
571585
foreach ($call[1] as $value) {
572586
$arguments[] = $this->dumpValue($value);
573587
}
574588

575-
$calls .= $this->wrapServiceConditionals($call[1], sprintf(" \$%s->%s(%s);\n", $variableName, $call[0], implode(', ', $arguments)));
589+
$witherAssignation = '';
590+
591+
if ($call[2] ?? false) {
592+
if (null !== $sharedNonLazyId && $lastWitherIndex === $k) {
593+
$witherAssignation = sprintf('$this->%s[\'%s\'] = ', $definition->isPublic() ? 'services' : 'privates', $sharedNonLazyId);
594+
}
595+
$witherAssignation .= sprintf('$%s = ', $variableName);
596+
}
597+
598+
$calls .= $this->wrapServiceConditionals($call[1], sprintf(" %s\$%s->%s(%s);\n", $witherAssignation, $variableName, $call[0], implode(', ', $arguments)));
576599
}
577600

578601
return $calls;
@@ -814,7 +837,7 @@ private function addInlineService(string $id, Definition $definition, Definition
814837
}
815838

816839
$code .= $this->addServiceProperties($inlineDef, $name);
817-
$code .= $this->addServiceMethodCalls($inlineDef, $name);
840+
$code .= $this->addServiceMethodCalls($inlineDef, $name, !$this->getProxyDumper()->isProxyCandidate($inlineDef) && $inlineDef->isShared() && !isset($this->singleUsePrivateIds[$id]) ? $id : null);
818841
$code .= $this->addServiceConfigurator($inlineDef, $name);
819842
}
820843

‎src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ private function addMethodCalls(array $methodcalls, \DOMElement $parent)
8484
if (\count($methodcall[1])) {
8585
$this->convertParameters($methodcall[1], 'argument', $call);
8686
}
87+
if ($methodcall[2] ?? false) {
88+
$call->setAttribute('returns-clone', 'true');
89+
}
8790
$parent->appendChild($call);
8891
}
8992
}

‎src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults)
337337
}
338338

339339
foreach ($this->getChildren($service, 'call') as $call) {
340-
$definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument', $file));
340+
$definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument', $file), XmlUtils::phpize($call->getAttribute('returns-clone')));
341341
}
342342

343343
$tags = $this->getChildren($service, 'tag');

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,15 +463,17 @@ private function parseDefinition($id, $service, $file, array $defaults)
463463
if (isset($call['method'])) {
464464
$method = $call['method'];
465465
$args = isset($call['arguments']) ? $this->resolveServices($call['arguments'], $file) : [];
466+
$returnsClone = $call['returns_clone'] ?? false;
466467
} else {
467468
$method = $call[0];
468469
$args = isset($call[1]) ? $this->resolveServices($call[1], $file) : [];
470+
$returnsClone = $call[2] ?? false;
469471
}
470472

471473
if (!\is_array($args)) {
472474
throw new InvalidArgumentException(sprintf('The second parameter for function call "%s" must be an array of its arguments for service "%s" in %s. Check your YAML syntax.', $method, $id, $file));
473475
}
474-
$definition->addMethodCall($method, $args);
476+
$definition->addMethodCall($method, $args, $returnsClone);
475477
}
476478
}
477479

‎src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@
243243
<xsd:element name="argument" type="argument" maxOccurs="unbounded" />
244244
</xsd:choice>
245245
<xsd:attribute name="method" type="xsd:string" />
246+
<xsd:attribute name="returns-clone" type="boolean" />
246247
</xsd:complexType>
247248

248249
<xsd:simpleType name="parameter_type">

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireRequiredMethodsPassTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,26 @@ public function testExplicitMethodInjection()
7777
);
7878
$this->assertEquals([], $methodCalls[0][1]);
7979
}
80+
81+
public function testWitherInjection()
82+
{
83+
$container = new ContainerBuilder();
84+
$container->register(Foo::class);
85+
86+
$container
87+
->register('wither', Wither::class)
88+
->setAutowired(true);
89+
90+
(new ResolveClassPass())->process($container);
91+
(new AutowireRequiredMethodsPass())->process($container);
92+
93+
$methodCalls = $container->getDefinition('wither')->getMethodCalls();
94+
95+
$expected = [
96+
['withFoo1', [], true],
97+
['withFoo2', [], true],
98+
['setFoo', []],
99+
];
100+
$this->assertSame($expected, $methodCalls);
101+
}
80102
}

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Tests;
1313

14+
require_once __DIR__.'/Fixtures/includes/autowiring_classes.php';
1415
require_once __DIR__.'/Fixtures/includes/classes.php';
1516
require_once __DIR__.'/Fixtures/includes/ProjectExtension.php';
1617

@@ -36,6 +37,8 @@
3637
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
3738
use Symfony\Component\DependencyInjection\Reference;
3839
use Symfony\Component\DependencyInjection\ServiceLocator;
40+
use Symfony\Component\DependencyInjection\Tests\Compiler\Foo;
41+
use Symfony\Component\DependencyInjection\Tests\Compiler\Wither;
3942
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
4043
use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
4144
use Symfony\Component\DependencyInjection\Tests\Fixtures\SimilarArgumentsDummy;
@@ -1565,6 +1568,22 @@ public function testDecoratedSelfReferenceInvolvingPrivateServices()
15651568

15661569
$this->assertSame(['service_container'], array_keys($container->getDefinitions()));
15671570
}
1571+
1572+
public function testWither()
1573+
{
1574+
$container = new ContainerBuilder();
1575+
$container->register(Foo::class);
1576+
1577+
$container
1578+
->register('wither', Wither::class)
1579+
->setPublic(true)
1580+
->setAutowired(true);
1581+
1582+
$container->compile();
1583+
1584+
$wither = $container->get('wither');
1585+
$this->assertInstanceOf(Foo::class, $wither->foo);
1586+
}
15681587
}
15691588

15701589
class FooClass

‎src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,16 @@ public function testMethodCalls()
9595
$this->assertEquals([['foo', ['foo']]], $def->getMethodCalls(), '->getMethodCalls() returns the methods to call');
9696
$this->assertSame($def, $def->addMethodCall('bar', ['bar']), '->addMethodCall() implements a fluent interface');
9797
$this->assertEquals([['foo', ['foo']], ['bar', ['bar']]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
98+
$this->assertSame($def, $def->addMethodCall('foobar', ['foobar'], true), '->addMethodCall() implements a fluent interface with third parameter');
99+
$this->assertEquals([['foo', ['foo']], ['bar', ['bar']], ['foobar', ['foobar'], true]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
98100
$this->assertTrue($def->hasMethodCall('bar'), '->hasMethodCall() returns true if first argument is a method to call registered');
99101
$this->assertFalse($def->hasMethodCall('no_registered'), '->hasMethodCall() returns false if first argument is not a method to call registered');
100102
$this->assertSame($def, $def->removeMethodCall('bar'), '->removeMethodCall() implements a fluent interface');
103+
$this->assertTrue($def->hasMethodCall('foobar'), '->hasMethodCall() returns true if first argument is a method to call registered');
104+
$this->assertSame($def, $def->removeMethodCall('foobar'), '->removeMethodCall() implements a fluent interface');
101105
$this->assertEquals([['foo', ['foo']]], $def->getMethodCalls(), '->removeMethodCall() removes a method to call');
106+
$this->assertSame($def, $def->setMethodCalls([['foobar', ['foobar'], true]]), '->setMethodCalls() implements a fluent interface with third parameter');
107+
$this->assertEquals([['foobar', ['foobar'], true]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
102108
}
103109

104110
/**

0 commit comments

Comments
 (0)
0