Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 539f4ca

Browse filesBrowse 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
Copy full SHA for 539f4ca

16 files changed

+286
-22
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
+3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php
+32-2Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowireRequiredMethodsPass.php
+15-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+21-7Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Definition.php
+6-5Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+28-5Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
+3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
+3-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
+1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireRequiredMethodsPassTest.php
+22Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
+19Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php
+6Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.