[DI] Add support for "wither" methods - for greater immutable services

symfony · Feb 13, 2019 · 1e41685 · 1e41685
1 parent b9b8f9d
commit 1e41685
Show file tree

Hide file tree

Showing 16 changed files with 195 additions and 14 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
@@ -343,6 +343,9 @@ private function getContainerDefinitionDocument(Definition $definition, string $
             foreach ($calls as $callData) {
                 $callsXML->appendChild($callXML = $dom->createElement('call'));
                 $callXML->setAttribute('method', $callData[0]);
+                if ($callData[2] ?? false) {
+                    $callXML->setAttribute('use-result', 'true');
+                }
             }
         }
 

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php
@@ -136,15 +136,27 @@ protected function processValue($value, $isRoot = false)
         }
         $this->lazy = false;
 
+        // Any calls before a "wither" are part of the constructor-instantiation graph
+        $witherCalls = [];
+        $setterCalls = $value->getMethodCalls();
+        for ($i = \count($setterCalls) - 1; 0 <= $i; --$i) {
+            if ($setterCalls[$i][2] ?? false) {
+                $witherCalls = \array_slice($setterCalls, 0, 1 + $i);
+                $setterCalls = \array_slice($setterCalls, 1 + $i);
+                break;
+            }
+        }
+
         $byConstructor = $this->byConstructor;
         $this->byConstructor = true;
         $this->processValue($value->getFactory());
         $this->processValue($value->getArguments());
+        $this->processValue($witherCalls);
         $this->byConstructor = $byConstructor;
 
         if (!$this->onlyConstructorArguments) {
             $this->processValue($value->getProperties());
-            $this->processValue($value->getMethodCalls());
+            $this->processValue($setterCalls);
             $this->processValue($value->getConfigurator());
         }
         $this->lazy = $lazy;

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowireRequiredMethodsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowireRequiredMethodsPass.php
@@ -50,7 +50,7 @@ protected function processValue($value, $isRoot = false)
             while (true) {
                 if (false !== $doc = $r->getDocComment()) {
                     if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
-                        $value->addMethodCall($reflectionMethod->name);
+                        $value->addMethodCall($reflectionMethod->name, [], preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@return\s++static[\s\*]#i', $doc));
                         break;
                     }
                     if (false === stripos($doc, '@inheritdoc') || !preg_match('#(?:^/\*\*|\n\s*+\*)\s*+(?:\{@inheritdoc\}|@inheritdoc)(?:\s|\*/$)#i', $doc)) {

diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -1150,7 +1150,7 @@ private function createService(Definition $definition, array &$inlineServices, $
         }
 
         foreach ($definition->getMethodCalls() as $call) {
-            $this->callMethod($service, $call, $inlineServices);
+            $service = $this->callMethod($service, $call, $inlineServices);
         }
 
         if ($callable = $definition->getConfigurator()) {
@@ -1568,16 +1568,18 @@ private function callMethod($service, $call, array &$inlineServices)
     {
         foreach (self::getServiceConditionals($call[1]) as $s) {
             if (!$this->has($s)) {
-                return;
+                return $service;
             }
         }
         foreach (self::getInitializedConditionals($call[1]) as $s) {
             if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE, $inlineServices)) {
-                return;
+                return $service;
             }
         }
 
-        $service->{$call[0]}(...$this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlineServices));
+        $result = $service->{$call[0]}(...$this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlineServices));
+
+        return empty($call[2]) ? $service : $result;
     }
 
     /**

diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php
@@ -330,7 +330,7 @@ public function setMethodCalls(array $calls = [])
     {
         $this->calls = [];
         foreach ($calls as $call) {
-            $this->addMethodCall($call[0], $call[1]);
+            $this->addMethodCall($call[0], $call[1], $call[2] ?? false);
         }
 
         return $this;
@@ -341,17 +341,18 @@ public function setMethodCalls(array $calls = [])
      *
      * @param string $method    The method name to call
      * @param array  $arguments An array of arguments to pass to the method call
+     * @param bool   $useResult Whether the call returns the service instance or not
      *
      * @return $this
      *
      * @throws InvalidArgumentException on empty $method param
      */
-    public function addMethodCall($method, array $arguments = [])
+    public function addMethodCall($method, array $arguments = []/*, bool $useResult = false*/)
     {
         if (empty($method)) {
             throw new InvalidArgumentException('Method name cannot be empty.');
         }
-        $this->calls[] = [$method, $arguments];
+        $this->calls[] = 2 < \func_num_args() && \func_get_arg(2) ? [$method, $arguments, true] : [$method, $arguments];
 
         return $this;
     }

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -563,7 +563,7 @@ private function isTrivialInstance(Definition $definition): bool
         return true;
     }
 
-    private function addServiceMethodCalls(Definition $definition, string $variableName = 'instance'): string
+    private function addServiceMethodCalls(Definition $definition, string $variableName, ?string $sharedNonLazyId): string
     {
         $calls = '';
         foreach ($definition->getMethodCalls() as $call) {
@@ -572,7 +572,16 @@ private function addServiceMethodCalls(Definition $definition, string $variableN
                 $arguments[] = $this->dumpValue($value);
             }
 
-            $calls .= $this->wrapServiceConditionals($call[1], sprintf("        \$%s->%s(%s);\n", $variableName, $call[0], implode(', ', $arguments)));
+            $witherAssignation = '';
+
+            if ($call[2] ?? false) {
+                if (null !== $sharedNonLazyId) {
+                    $witherAssignation = sprintf('$this->%s[\'%s\'] = ', $definition->isPublic() ? 'services' : 'privates', $sharedNonLazyId);
+                }
+                $witherAssignation .= sprintf('$%s = ', $variableName);
+            }
+
+            $calls .= $this->wrapServiceConditionals($call[1], sprintf("        %s\$%s->%s(%s);\n", $witherAssignation, $variableName, $call[0], implode(', ', $arguments)));
         }
 
         return $calls;
@@ -814,7 +823,7 @@ private function addInlineService(string $id, Definition $definition, Definition
             }
 
             $code .= $this->addServiceProperties($inlineDef, $name);
-            $code .= $this->addServiceMethodCalls($inlineDef, $name);
+            $code .= $this->addServiceMethodCalls($inlineDef, $name, !$this->getProxyDumper()->isProxyCandidate($inlineDef) && $inlineDef->isShared() && !isset($this->singleUsePrivateIds[$id]) ? $id : null);
             $code .= $this->addServiceConfigurator($inlineDef, $name);
         }
 

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
@@ -84,6 +84,9 @@ private function addMethodCalls(array $methodcalls, \DOMElement $parent)
             if (\count($methodcall[1])) {
                 $this->convertParameters($methodcall[1], 'argument', $call);
             }
+            if ($methodcall[2] ?? false) {
+                $call->setAttribute('use-result', 'true');
+            }
             $parent->appendChild($call);
         }
     }

diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@@ -337,7 +337,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults)
         }
 
         foreach ($this->getChildren($service, 'call') as $call) {
-            $definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument', $file));
+            $definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument', $file), XmlUtils::phpize($call->getAttribute('use-result')));
         }
 
         $tags = $this->getChildren($service, 'tag');

diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
@@ -463,15 +463,17 @@ private function parseDefinition($id, $service, $file, array $defaults)
                 if (isset($call['method'])) {
                     $method = $call['method'];
                     $args = isset($call['arguments']) ? $this->resolveServices($call['arguments'], $file) : [];
+                    $useResult = $call['use_result'] ?? false;
                 } else {
                     $method = $call[0];
                     $args = isset($call[1]) ? $this->resolveServices($call[1], $file) : [];
+                    $useResult = $call[2] ?? false;
                 }
 
                 if (!\is_array($args)) {
                     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));
                 }
-                $definition->addMethodCall($method, $args);
+                $definition->addMethodCall($method, $args, $useResult);
             }
         }
 

diff --git a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
@@ -241,6 +241,7 @@
       <xsd:element name="argument" type="argument" maxOccurs="unbounded" />
     </xsd:choice>
     <xsd:attribute name="method" type="xsd:string" />
+    <xsd:attribute name="use-result" type="boolean" />
   </xsd:complexType>
 
   <xsd:simpleType name="parameter_type">

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireRequiredMethodsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireRequiredMethodsPassTest.php
@@ -77,4 +77,21 @@ public function testExplicitMethodInjection()
         );
         $this->assertEquals([], $methodCalls[0][1]);
     }
+
+    public function testWitherInjection()
+    {
+        $container = new ContainerBuilder();
+        $container->register(Foo::class);
+
+        $container
+            ->register('wither', Wither::class)
+            ->setAutowired(true);
+
+        (new ResolveClassPass())->process($container);
+        (new AutowireRequiredMethodsPass())->process($container);
+
+        $methodCalls = $container->getDefinition('wither')->getMethodCalls();
+
+        $this->assertSame([['withFoo', [], true]], $methodCalls);
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\DependencyInjection\Tests;
 
+require_once __DIR__.'/Fixtures/includes/autowiring_classes.php';
 require_once __DIR__.'/Fixtures/includes/classes.php';
 require_once __DIR__.'/Fixtures/includes/ProjectExtension.php';
 
@@ -36,6 +37,8 @@
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\ServiceLocator;
+use Symfony\Component\DependencyInjection\Tests\Compiler\Foo;
+use Symfony\Component\DependencyInjection\Tests\Compiler\Wither;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\SimilarArgumentsDummy;
@@ -1565,6 +1568,22 @@ public function testDecoratedSelfReferenceInvolvingPrivateServices()
 
         $this->assertSame(['service_container'], array_keys($container->getDefinitions()));
     }
+
+    public function testWither()
+    {
+        $container = new ContainerBuilder();
+        $container->register(Foo::class);
+
+        $container
+            ->register('wither', Wither::class)
+            ->setPublic(true)
+            ->setAutowired(true);
+
+        $container->compile();
+
+        $wither = $container->get('wither');
+        $this->assertInstanceOf(Foo::class, $wither->foo);
+    }
 }
 
 class FooClass

diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php
@@ -95,10 +95,16 @@ public function testMethodCalls()
         $this->assertEquals([['foo', ['foo']]], $def->getMethodCalls(), '->getMethodCalls() returns the methods to call');
         $this->assertSame($def, $def->addMethodCall('bar', ['bar']), '->addMethodCall() implements a fluent interface');
         $this->assertEquals([['foo', ['foo']], ['bar', ['bar']]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
+        $this->assertSame($def, $def->addMethodCall('foobar', ['foobar'], true), '->addMethodCall() implements a fluent interface with third parameter');
+        $this->assertEquals([['foo', ['foo']], ['bar', ['bar']], ['foobar', ['foobar'], true]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
         $this->assertTrue($def->hasMethodCall('bar'), '->hasMethodCall() returns true if first argument is a method to call registered');
         $this->assertFalse($def->hasMethodCall('no_registered'), '->hasMethodCall() returns false if first argument is not a method to call registered');
         $this->assertSame($def, $def->removeMethodCall('bar'), '->removeMethodCall() implements a fluent interface');
+        $this->assertTrue($def->hasMethodCall('foobar'), '->hasMethodCall() returns true if first argument is a method to call registered');
+        $this->assertSame($def, $def->removeMethodCall('foobar'), '->removeMethodCall() implements a fluent interface');
         $this->assertEquals([['foo', ['foo']]], $def->getMethodCalls(), '->removeMethodCall() removes a method to call');
+        $this->assertSame($def, $def->setMethodCalls([['foobar', ['foobar'], true]]), '->setMethodCalls() implements a fluent interface with third parameter');
+        $this->assertEquals([['foobar', ['foobar'], true]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
     }
 
     /**

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -30,13 +30,16 @@
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\ServiceLocator;
+use Symfony\Component\DependencyInjection\Tests\Compiler\Foo;
+use Symfony\Component\DependencyInjection\Tests\Compiler\Wither;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber;
 use Symfony\Component\DependencyInjection\TypedReference;
 use Symfony\Component\DependencyInjection\Variable;
 use Symfony\Component\ExpressionLanguage\Expression;
 
+require_once __DIR__.'/../Fixtures/includes/autowiring_classes.php';
 require_once __DIR__.'/../Fixtures/includes/classes.php';
 
 class PhpDumperTest extends TestCase
@@ -1170,6 +1173,28 @@ public function testServiceLocatorArgument()
         $container->set('foo5', $foo5 = new \stdClass());
         $this->assertSame($foo5, $locator->get('foo5'));
     }
+
+    public function testWither()
+    {
+        $container = new ContainerBuilder();
+        $container->register(Foo::class);
+
+        $container
+            ->register('wither', Wither::class)
+            ->setPublic(true)
+            ->setAutowired(true);
+
+        $container->compile();
+        $dumper = new PhpDumper($container);
+        $dump = $dumper->dump(['class' => 'Symfony_DI_PhpDumper_Service_Wither']);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_wither.php', $dump);
+        eval('?>'.$dump);
+
+        $container = new \Symfony_DI_PhpDumper_Service_Wither();
+
+        $wither = $container->get('wither');
+        $this->assertInstanceOf(Foo::class, $wither->foo);
+    }
 }
 
 class Rot13EnvVarProcessor implements EnvVarProcessorInterface

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php
@@ -278,6 +278,23 @@ public function setChildMethodWithoutDocBlock(A $a)
     }
 }
 
+class Wither
+{
+    public $foo;
+
+    /**
+     * @required
+     * @return static
+     */
+    public function withFoo(Foo $foo)
+    {
+        $new = clone $this;
+        $new->foo = $foo;
+
+        return $new;
+    }
+}
+
 class SetterInjectionParent
 {
     /** @required*/