symfony · fabpot · Apr 3, 2019 · Feb 12, 2019
diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
@@ -348,6 +348,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('returns-clone', 'true');
+                }
            }
        }


diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php
@@ -140,11 +140,41 @@ protected function processValue($value, $isRoot = false)
        $this->byConstructor = true;
        $this->processValue($value->getFactory());
        $this->processValue($value->getArguments());
+
+        $properties = $value->getProperties();
+        $setters = $value->getMethodCalls();
+
+        // Any references before a "wither" are part of the constructor-instantiation graph
+        $lastWitherIndex = null;
+        foreach ($setters as $k => $call) {
+            if ($call[2] ?? false) {
+                $lastWitherIndex = $k;
+            }
+        }
+
+        if (null !== $lastWitherIndex) {
+            $this->processValue($properties);
+            $setters = $properties = [];
+
+            foreach ($value->getMethodCalls() as $k => $call) {
+                if (null === $lastWitherIndex) {
+                    $setters[] = $call;
+                    continue;
+                }
+
+                if ($lastWitherIndex === $k) {
+                    $lastWitherIndex = null;
+                }
+
+                $this->processValue($call);
+            }
+        }
+
        $this->byConstructor = $byConstructor;

        if (!$this->onlyConstructorArguments) {
-            $this->processValue($value->getProperties());
-            $this->processValue($value->getMethodCalls());
+            $this->processValue($properties);
+            $this->processValue($setters);
            $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
@@ -35,6 +35,7 @@ protected function processValue($value, $isRoot = false)
        }

        $alreadyCalledMethods = [];
+        $withers = [];

        foreach ($value->getMethodCalls() as list($method)) {
            $alreadyCalledMethods[strtolower($method)] = true;
@@ -50,7 +51,11 @@ 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);
+                        if (preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@return\s++static[\s\*]#i', $doc)) {
+                            $withers[] = [$reflectionMethod->name, [], true];
+                        } else {
+                            $value->addMethodCall($reflectionMethod->name, []);
+                        }
                        break;
                    }
                    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)
            }
        }

+        if ($withers) {
+            // Prepend withers to prevent creating circular loops
+            $setters = $value->getMethodCalls();
+            $value->setMethodCalls($withers);
+            foreach ($setters as $call) {
+                $value->addMethodCall($call[0], $call[1], $call[2] ?? false);
+            }
+        }
+
        return $value;
    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -1139,8 +1139,15 @@ private function createService(Definition $definition, array &$inlineServices, $
            }
        }

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

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

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

        if ($callable = $definition->getConfigurator()) {
@@ -1568,16 +1580,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;
@@ -339,19 +339,20 @@ public function setMethodCalls(array $calls = [])
    /**
     * Adds a method to call after service initialization.
     *
-     * @param string $method    The method name to call
-     * @param array  $arguments An array of arguments to pass to the method call
+     * @param string $method       The method name to call
+     * @param array  $arguments    An array of arguments to pass to the method call
+     * @param bool   $returnsClone 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 $returnsClone = 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
@@ -506,7 +506,14 @@ private function addServiceInstance(string $id, Definition $definition, bool $is
        $isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition);
        $instantiation = '';

-        if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
+        $lastWitherIndex = null;
+        foreach ($definition->getMethodCalls() as $k => $call) {
+            if ($call[2] ?? false) {
+                $lastWitherIndex = $k;
+            }
+        }
+
+        if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id]) && null === $lastWitherIndex) {
            $instantiation = sprintf('$this->%s[\'%s\'] = %s', $this->container->getDefinition($id)->isPublic() ? 'services' : 'privates', $id, $isSimpleInstance ? '' : '$instance');
        } elseif (!$isSimpleInstance) {
            $instantiation = '$instance';
@@ -563,16 +570,32 @@ 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
    {
+        $lastWitherIndex = null;
+        foreach ($definition->getMethodCalls() as $k => $call) {
+            if ($call[2] ?? false) {
+                $lastWitherIndex = $k;
+            }
+        }
+
        $calls = '';
-        foreach ($definition->getMethodCalls() as $call) {
+        foreach ($definition->getMethodCalls() as $k => $call) {
            $arguments = [];
            foreach ($call[1] as $value) {
                $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 && $lastWitherIndex === $k) {
+                    $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 +837,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('returns-clone', '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('returns-clone')));
        }

        $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) : [];
+                    $returnsClone = $call['returns_clone'] ?? false;
                } else {
                    $method = $call[0];
                    $args = isset($call[1]) ? $this->resolveServices($call[1], $file) : [];
+                    $returnsClone = $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, $returnsClone);
            }
        }


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
@@ -243,6 +243,7 @@
      <xsd:element name="argument" type="argument" maxOccurs="unbounded" />
    </xsd:choice>
    <xsd:attribute name="method" type="xsd:string" />
+    <xsd:attribute name="returns-clone" 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,26 @@ 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();
+
+        $expected = [
+            ['withFoo1', [], true],
+            ['withFoo2', [], true],
+            ['setFoo', []],
+        ];
+        $this->assertSame($expected, $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');
    }

    /**