[DependencyInjection] made some improvments to the container compiler

- inline private services which are references multiple times, but where all references originate from the same definition
- bug fix for non-shared services which were considered shared within the scope in which they were inlined

src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php

@@ -3,7 +3,6 @@
 namespace Symfony\Component\DependencyInjection\Compiler;
 
 use Symfony\Component\DependencyInjection\Definition;
-
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
@@ -57,8 +56,15 @@ class InlineServiceDefinitionsPass implements RepeatablePassInterface
                 }
 
                 if ($this->isInlinableDefinition($container, $id, $definition = $container->getDefinition($id))) {
-                    $arguments[$k] = $definition;
+                    if ($definition->isShared()) {
+                        $arguments[$k] = $definition;
+                    } else {
+                        $arguments[$k] = clone $definition;
+                    }
                 }
+            } else if ($argument instanceof Definition) {
+                $argument->setArguments($this->inlineArguments($container, $argument->getArguments()));
+                $argument->setMethodCalls($this->inlineArguments($container, $argument->getMethodCalls()));
             }
         }
 

src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php

@@ -3,7 +3,6 @@
 namespace Symfony\Component\DependencyInjection\Compiler;
 
 use Symfony\Component\DependencyInjection\Alias;
-
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -45,13 +44,16 @@ class RemoveUnusedDefinitionsPass implements RepeatablePassInterface
             if ($this->graph->hasNode($id)) {
                 $edges = $this->graph->getNode($id)->getInEdges();
                 $referencingAliases = array();
+                $sourceIds = array();
                 foreach ($edges as $edge) {
                     $node = $edge->getSourceNode();
+                    $sourceIds[] = $node->getId();
+
                     if ($node->isAlias()) {
                         $referencingAlias[] = $node->getValue();
                     }
                 }
-                $isReferenced = (count($edges) - count($referencingAliases)) > 0;
+                $isReferenced = (count(array_unique($sourceIds)) - count($referencingAliases)) > 0;
             } else {
                 $referencingAliases = array();
                 $isReferenced = false;

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

@@ -182,11 +182,27 @@ EOF;
     {
         $code = '';
         $variableMap = $this->definitionVariables;
+        $nbOccurrences = new \SplObjectStorage();
+        $processed = new \SplObjectStorage();
+        $inlinedDefinitions = $this->getInlinedDefinitions($definition);
+
+        foreach ($inlinedDefinitions as $definition) {
+            if (false === $nbOccurrences->contains($definition)) {
+                $nbOccurrences->offsetSet($definition, 1);
+            } else {
+                $i = $nbOccurrences->offsetGet($definition);
+                $nbOccurrences->offsetSet($definition, $i+1);
+            }
+        }
+
+        foreach ($inlinedDefinitions as $sDefinition) {
+            if ($processed->contains($sDefinition)) {
+                continue;
+            }
+            $processed->offsetSet($sDefinition);
 
-        $c = 1;
-        foreach ($this->getInlinedDefinitions($definition) as $sDefinition) {
             $class = $this->dumpValue($sDefinition->getClass());
-            if (count($sDefinition->getMethodCalls()) > 0 || null !== $sDefinition->getConfigurator() || false !== strpos($class, '$')) {
+            if ($nbOccurrences->offsetGet($sDefinition) > 1 || count($sDefinition->getMethodCalls()) > 0 || null !== $sDefinition->getConfigurator() || false !== strpos($class, '$')) {
                 $name = $this->getNextVariableName();
                 $variableMap->offsetSet($sDefinition, new Variable($name));
 
@@ -326,7 +342,13 @@ EOF;
         $this->referenceVariables[$id] = new Variable('instance');
 
         $code = '';
+        $processed = new \SplObjectStorage();
         foreach ($this->getInlinedDefinitions($definition) as $iDefinition) {
+            if ($processed->contains($iDefinition)) {
+                continue;
+            }
+            $processed->offsetSet($iDefinition);
+
             if (!$this->hasReference($id, $iDefinition->getMethodCalls())) {
                 continue;
             }

tests/Symfony/Tests/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPassTest.php

@@ -2,6 +2,8 @@
 
 namespace Symfony\Tests\Component\DependencyInjection\Compiler;
 
+use Symfony\Component\DependencyInjection\Definition;
+
 use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass;
 
 use Symfony\Component\DependencyInjection\Compiler\Compiler;
@@ -76,9 +78,31 @@ class InlineServiceDefinitionsPassTest extends \PHPUnit_Framework_TestCase
         $this->process($container);
 
         $arguments = $container->getDefinition('service')->getArguments();
-        $this->assertSame($container->getDefinition('foo'), $arguments[0]);
+        $this->assertEquals($container->getDefinition('foo'), $arguments[0]);
+        $this->assertNotSame($container->getDefinition('foo'), $arguments[0]);
         $this->assertSame($ref, $arguments[1]);
-        $this->assertSame($container->getDefinition('bar'), $arguments[2]);
+        $this->assertEquals($container->getDefinition('bar'), $arguments[2]);
+        $this->assertNotSame($container->getDefinition('bar'), $arguments[2]);
+    }
+
+    public function testProcessInlinesIfMultipleReferencesButAllFromTheSameDefinition()
+    {
+        $container = new ContainerBuilder();
+
+        $a = $container->register('a')->setPublic(false);
+        $b = $container
+            ->register('b')
+            ->addArgument(new Reference('a'))
+            ->addArgument(new Definition(null, array(new Reference('a'))))
+        ;
+
+        $this->process($container);
+
+        $arguments = $b->getArguments();
+        $this->assertSame($a, $arguments[0]);
+
+        $inlinedArguments = $arguments[1]->getArguments();
+        $this->assertSame($a, $inlinedArguments[0]);
     }
 
     protected function process(ContainerBuilder $container)

tests/Symfony/Tests/Component/DependencyInjection/Compiler/IntegrationTest.php

@@ -72,4 +72,32 @@ class IntegrationTest extends \PHPUnit_Framework_TestCase
         $this->assertFalse($container->hasAlias('b'));
         $this->assertFalse($container->hasDefinition('c'));
     }
+
+    public function testProcessInlinesWhenThereAreMultipleReferencesButFromTheSameDefinition()
+    {
+        $container = new ContainerBuilder();
+
+        $container
+            ->register('a')
+            ->addArgument(new Reference('b'))
+            ->addMethodCall('setC', array(new Reference('c')))
+        ;
+
+        $container
+            ->register('b')
+            ->addArgument(new Reference('c'))
+            ->setPublic(false)
+        ;
+
+        $container
+            ->register('c')
+            ->setPublic(false)
+        ;
+
+        $container->freeze();
+
+        $this->assertTrue($container->hasDefinition('a'));
+        $this->assertFalse($container->hasDefinition('b'));
+        $this->assertFalse($container->hasDefinition('c'), 'Service C was not inlined.');
+    }
 }