bug #27830 [Dependency-Injection] Fix issue where non-defined services were attempted to be removed (ciaranmcnulty)

nicolas-grekas · nicolas-grekas · commit 26989d4e736b · 2018-07-03T20:20:23.000+02:00
This PR was squashed before being merged into the 4.2-dev branch (closes #27830). Discussion ---------- [Dependency-Injection] Fix issue where non-defined services were attempted to be removed | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | #27802  | License | MIT | Doc PR | symfony/symfony-docs#...  The RemoveUnusedServicesPass generates a graph of services that are connected, then removes services that are not part of the graph. To build this graph it creates a list of services then examines their definitions for more services. The current scenario was causing an error, thrown by getDefinition (triggered in Behat): * Service was referenced by another service definition * Service was _not_ defined * Service was subsequently set() on the container Commits ------- 53155c9 [Dependency-Injection] Fix issue where non-defined services were attempted to be removed
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php
@@ -60,7 +60,7 @@ public function process(ContainerBuilder $container)
                 $ids = $this->connectedIds;
                 $this->connectedIds = array();
                 foreach ($ids as $id) {
-                    if (!isset($connectedIds[$id]) && $container->has($id)) {
+                    if (!isset($connectedIds[$id]) && $container->hasDefinition($id)) {
                         $connectedIds[$id] = true;
                         $this->processValue($container->getDefinition($id));
                     }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/RemoveUnusedDefinitionsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/RemoveUnusedDefinitionsPassTest.php
@@ -127,6 +127,21 @@ public function testProcessConsiderEnvVariablesAsUsedEvenInPrivateServices()
         $this->assertSame(1, $envCounters['FOOBAR']);
     }
 
+    public function testProcessDoesNotErrorOnServicesThatDoNotHaveDefinitions()
+    {
+        $container = new ContainerBuilder();
+        $container
+            ->register('defined')
+            ->addArgument(new Reference('not.defined'))
+            ->setPublic(true);
+
+        $container->set('not.defined', new \StdClass());
+
+        $this->process($container);
+
+        $this->assertFalse($container->hasDefinition('not.defined'));
+    }
+
     protected function process(ContainerBuilder $container)
     {
         (new RemoveUnusedDefinitionsPass())->process($container);

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ public function process(ContainerBuilder $container)`
`60`	`60`	`$ids = $this->connectedIds;`
`61`	`61`	`$this->connectedIds = array();`
`62`	`62`	`foreach ($ids as $id) {`
`63`		`- if (!isset($connectedIds[$id]) && $container->has($id)) {`
	`63`	`+ if (!isset($connectedIds[$id]) && $container->hasDefinition($id)) {`
`64`	`64`	`$connectedIds[$id] = true;`
`65`	`65`	`$this->processValue($container->getDefinition($id));`
`66`	`66`	`}`