[DependencyInjection] Fix serialization of \Closure in RemoveUnusedDefinitionsPass

Anthony MARTIN · Anthony MARTIN · commit b0925025f8dc · 2019-02-01T12:06:24.000+01:00
Signed-off-by: Anthony MARTIN <anthony.martin@sensiolabs.com> | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29694 | License | MIT | Doc PR | n/a Fix the issue #29694
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php
@@ -70,7 +70,7 @@ public function process(ContainerBuilder $container)
             foreach ($container->getDefinitions() as $id => $definition) {
                 if (!isset($connectedIds[$id])) {
                     $container->removeDefinition($id);
-                    $container->resolveEnvPlaceholders(serialize($definition));
+                    $container->resolveEnvPlaceholders(!$definition->hasErrors() ? serialize($definition) : $definition);
                     $container->log($this, sprintf('Removed service "%s"; reason: unused.', $id));
                 }
             }
diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -1394,6 +1394,10 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
             $value = $bag->resolveValue($value);
         }
 
+        if ($value instanceof Definition) {
+            $value = (array) $value;
+        }
+
         if (\is_array($value)) {
             $result = [];
             foreach ($value as $k => $v) {
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/RemoveUnusedDefinitionsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/RemoveUnusedDefinitionsPassTest.php
@@ -142,6 +142,24 @@ public function testProcessDoesNotErrorOnServicesThatDoNotHaveDefinitions()
         $this->assertFalse($container->hasDefinition('not.defined'));
     }
 
+    public function testProcessWorksWithClosureErrorsInDefinitions()
+    {
+        $definition = new Definition();
+        $definition->addError(function () {
+            return 'foo bar';
+        });
+
+        $container = new ContainerBuilder();
+        $container
+            ->setDefinition('foo', $definition)
+            ->setPublic(false)
+        ;
+
+        $this->process($container);
+
+        $this->assertFalse($container->hasDefinition('foo'));
+    }
+
     protected function process(ContainerBuilder $container)
     {
         (new RemoveUnusedDefinitionsPass())->process($container);

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ public function process(ContainerBuilder $container)`
`70`	`70`	`foreach ($container->getDefinitions() as $id => $definition) {`
`71`	`71`	`if (!isset($connectedIds[$id])) {`
`72`	`72`	`$container->removeDefinition($id);`
`73`		`- $container->resolveEnvPlaceholders(serialize($definition));`
	`73`	`+ $container->resolveEnvPlaceholders(!$definition->hasErrors() ? serialize($definition) : $definition);`
`74`	`74`	`$container->log($this, sprintf('Removed service "%s"; reason: unused.', $id));`
`75`	`75`	`}`
`76`	`76`	`}`