[DependencyInjection] Properly ignore invalid reference arguments in collection arguments

Author: ogizanagi

Compiler/ResolveInvalidReferencesPass.php

@@ -71,16 +71,19 @@ public function process(ContainerBuilder $container)
      *
      * @param array $arguments    An array of Reference objects
      * @param bool  $inMethodCall
+     * @param bool  $inCollection
      *
      * @return array
      *
      * @throws RuntimeException When the config is invalid
      */
-    private function processArguments(array $arguments, $inMethodCall = false)
+    private function processArguments(array $arguments, $inMethodCall = false, $inCollection = false)
     {
+        $isNumeric = array_keys($arguments) === range(0, count($arguments) - 1);
+
         foreach ($arguments as $k => $argument) {
             if (is_array($argument)) {
-                $arguments[$k] = $this->processArguments($argument, $inMethodCall);
+                $arguments[$k] = $this->processArguments($argument, $inMethodCall, true);
             } elseif ($argument instanceof Reference) {
                 $id = (string) $argument;
 
@@ -91,6 +94,10 @@ private function processArguments(array $arguments, $inMethodCall = false)
                 if (!$exists && ContainerInterface::NULL_ON_INVALID_REFERENCE === $invalidBehavior) {
                     $arguments[$k] = null;
                 } elseif (!$exists && ContainerInterface::IGNORE_ON_INVALID_REFERENCE === $invalidBehavior) {
+                    if ($inCollection) {
+                        unset($arguments[$k]);
+                        continue;
+                    }
                     if ($inMethodCall) {
                         throw new RuntimeException('Method shouldn\'t be called.');
                     }
@@ -100,6 +107,11 @@ private function processArguments(array $arguments, $inMethodCall = false)
             }
         }
 
+        // Ensure numerically indexed arguments have sequential numeric keys.
+        if ($isNumeric) {
+            $arguments = array_values($arguments);
+        }
+
         return $arguments;
     }
 }

Tests/Compiler/ResolveInvalidReferencesPassTest.php

@@ -23,17 +23,63 @@ public function testProcess()
         $container = new ContainerBuilder();
         $def = $container
             ->register('foo')
-            ->setArguments(array(new Reference('bar', ContainerInterface::NULL_ON_INVALID_REFERENCE)))
+            ->setArguments(array(
+                new Reference('bar', ContainerInterface::NULL_ON_INVALID_REFERENCE),
+                new Reference('baz', ContainerInterface::IGNORE_ON_INVALID_REFERENCE),
+            ))
             ->addMethodCall('foo', array(new Reference('moo', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)))
         ;
 
         $this->process($container);
 
         $arguments = $def->getArguments();
-        $this->assertNull($arguments[0]);
+        $this->assertSame(array(null, null), $arguments);
         $this->assertCount(0, $def->getMethodCalls());
     }
 
+    public function testProcessIgnoreInvalidArgumentInCollectionArgument()
+    {
+        $container = new ContainerBuilder();
+        $container->register('baz');
+        $def = $container
+            ->register('foo')
+            ->setArguments(array(
+                array(
+                    new Reference('bar', ContainerInterface::IGNORE_ON_INVALID_REFERENCE),
+                    $baz = new Reference('baz', ContainerInterface::IGNORE_ON_INVALID_REFERENCE),
+                    new Reference('moo', ContainerInterface::NULL_ON_INVALID_REFERENCE),
+                ),
+            ))
+        ;
+
+        $this->process($container);
+
+        $arguments = $def->getArguments();
+        $this->assertSame(array($baz, null), $arguments[0]);
+    }
+
+    public function testProcessKeepMethodCallOnInvalidArgumentInCollectionArgument()
+    {
+        $container = new ContainerBuilder();
+        $container->register('baz');
+        $def = $container
+            ->register('foo')
+            ->addMethodCall('foo', array(
+                array(
+                    new Reference('bar', ContainerInterface::IGNORE_ON_INVALID_REFERENCE),
+                    $baz = new Reference('baz', ContainerInterface::IGNORE_ON_INVALID_REFERENCE),
+                    new Reference('moo', ContainerInterface::NULL_ON_INVALID_REFERENCE),
+                ),
+            ))
+        ;
+
+        $this->process($container);
+
+        $calls = $def->getMethodCalls();
+        $this->assertCount(1, $def->getMethodCalls());
+        $this->assertSame(array($baz, null), $calls[0][1][0]);
+    }
+
     public function testProcessIgnoreNonExistentServices()
     {
         $container = new ContainerBuilder();