[DependencyInjection] Properly ignore invalid reference arguments in collection arguments

ogizanagi · ogizanagi · commit cfc487950878 · 2015-12-28T14:56:54.000+01:00
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php b/src/Symfony/Component/DependencyInjection/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;
     }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInvalidReferencesPassTest.php b/src/Symfony/Component/DependencyInjection/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();

Original file line number	Diff line number	Diff line change
`@@ -71,16 +71,19 @@ public function process(ContainerBuilder $container)`
`71`	`71`	`*`
`72`	`72`	`* @param array $arguments An array of Reference objects`
`73`	`73`	`* @param bool $inMethodCall`
	`74`	`+ * @param bool $inCollection`
`74`	`75`	`*`
`75`	`76`	`* @return array`
`76`	`77`	`*`
`77`	`78`	`* @throws RuntimeException When the config is invalid`
`78`	`79`	`*/`
`79`		`- private function processArguments(array $arguments, $inMethodCall = false)`
	`80`	`+ private function processArguments(array $arguments, $inMethodCall = false, $inCollection = false)`
`80`	`81`	`{`
	`82`	`+ $isNumeric = array_keys($arguments) === range(0, count($arguments) - 1);`
	`83`	`+`
`81`	`84`	`foreach ($arguments as $k => $argument) {`
`82`	`85`	`if (is_array($argument)) {`
`83`		`- $arguments[$k] = $this->processArguments($argument, $inMethodCall);`
	`86`	`+ $arguments[$k] = $this->processArguments($argument, $inMethodCall, true);`
`84`	`87`	`} elseif ($argument instanceof Reference) {`
`85`	`88`	`$id = (string) $argument;`
`86`	`89`
`@@ -91,6 +94,10 @@ private function processArguments(array $arguments, $inMethodCall = false)`
`91`	`94`	`if (!$exists && ContainerInterface::NULL_ON_INVALID_REFERENCE === $invalidBehavior) {`
`92`	`95`	`$arguments[$k] = null;`
`93`	`96`	`} elseif (!$exists && ContainerInterface::IGNORE_ON_INVALID_REFERENCE === $invalidBehavior) {`
	`97`	`+ if ($inCollection) {`
	`98`	`+ unset($arguments[$k]);`
	`99`	`+ continue;`
	`100`	`+ }`
`94`	`101`	`if ($inMethodCall) {`
`95`	`102`	`throw new RuntimeException('Method shouldn\'t be called.');`
`96`	`103`	`}`
`@@ -100,6 +107,11 @@ private function processArguments(array $arguments, $inMethodCall = false)`
`100`	`107`	`}`
`101`	`108`	`}`
`102`	`109`
	`110`	`+ // Ensure numerically indexed arguments have sequential numeric keys.`
	`111`	`+ if ($isNumeric) {`
	`112`	`+ $arguments = array_values($arguments);`
	`113`	`+ }`
	`114`	`+`
`103`	`115`	`return $arguments;`
`104`	`116`	`}`
`105`	`117`	`}`