bug #25244 [DI] Add missing deprecation when fetching private service…

…s from ContainerBuilder (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Add missing deprecation when fetching private services from ContainerBuilder | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25242 | License | MIT | Doc PR | - Commits ------- 93c0b38 [DI] Add missing deprecation when fetching private services from ContainerBuilder
symfony · Dec 1, 2017 · 9401afd · 9401afd
2 parents 41e6f8e + 93c0b38
commit 9401afd
Show file tree

Hide file tree

Showing 15 changed files with 32 additions and 12 deletions.
diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php
@@ -232,9 +232,10 @@ public function testStopwatchExtensionAvailability($debug, $stopwatchEnabled, $e
         }
         $container->registerExtension(new TwigExtension());
         $container->loadFromExtension('twig', array());
+        $container->setAlias('test.twig.extension.debug.stopwatch', 'twig.extension.debug.stopwatch')->setPublic(true);
         $this->compileContainer($container);
 
-        $tokenParsers = $container->get('twig.extension.debug.stopwatch')->getTokenParsers();
+        $tokenParsers = $container->get('test.twig.extension.debug.stopwatch')->getTokenParsers();
         $stopwatchIsAvailable = new \ReflectionProperty($tokenParsers[0], 'stopwatchIsAvailable');
         $stopwatchIsAvailable->setAccessible(true);
 

diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -556,15 +556,8 @@ public function has($id)
      */
     public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE)
     {
-        if ($this->isCompiled()) {
-            $id = $this->normalizeId($id);
-
-            if (isset($this->definitions[$id]) && $this->definitions[$id]->isPrivate()) {
-                @trigger_error(sprintf('Fetching the "%s" private service is deprecated and will fail in Symfony 4.0. Make the service public instead.', $id), E_USER_DEPRECATED);
-            }
-            if (isset($this->aliasDefinitions[$id]) && $this->aliasDefinitions[$id]->isPrivate()) {
-                @trigger_error(sprintf('Fetching the "%s" private alias is deprecated and will fail in Symfony 4.0. Make the alias public instead.', $id), E_USER_DEPRECATED);
-            }
+        if ($this->isCompiled() && isset($this->removedIds[$id = $this->normalizeId($id)])) {
+            @trigger_error(sprintf('Fetching the "%s" private service or alias is deprecated since Symfony 3.4 and will fail in 4.0. Make it public instead.', $id), E_USER_DEPRECATED);
         }
 
         return $this->doGet($id, $invalidBehavior);
@@ -776,6 +769,12 @@ public function compile(/*$resolveEnvPlaceholders = false*/)
         }
 
         parent::compile();
+
+        foreach ($this->definitions + $this->aliasDefinitions as $id => $definition) {
+            if (!$definition->isPublic() || $definition->isPrivate()) {
+                $this->removedIds[$id] = true;
+            }
+        }
     }
 
     /**

diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@@ -733,6 +733,7 @@ public function testEnvInId()
             PsrContainerInterface::class => true,
             ContainerInterface::class => true,
             'baz_%env(BAR)%' => true,
+            'bar_%env(BAR)%' => true,
         );
         $this->assertSame($expected, $container->getRemovedIds());
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt
@@ -9,8 +9,10 @@ return array(
     'configurator_service_simple' => true,
     'decorated.pif-pouf' => true,
     'decorator_service.inner' => true,
+    'factory_simple' => true,
     'inlined' => true,
     'new_factory' => true,
+    'tagged_iterator_foo' => true,
 );
 
     [Container%s/getBazService.php] => <?php

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
@@ -70,8 +70,10 @@ public function getRemovedIds()
             'configurator_service_simple' => true,
             'decorated.pif-pouf' => true,
             'decorator_service.inner' => true,
+            'factory_simple' => true,
             'inlined' => true,
             'new_factory' => true,
+            'tagged_iterator_foo' => true,
         );
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_env_in_id.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_env_in_id.php
@@ -44,6 +44,7 @@ public function getRemovedIds()
         return array(
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
+            'bar_%env(BAR)%' => true,
             'baz_%env(BAR)%' => true,
         );
     }

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php
@@ -57,6 +57,7 @@ public function getRemovedIds()
         return array(
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
+            'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\includes\\HotPath\\C3' => true,
         );
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_legacy_privates.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_legacy_privates.php
@@ -60,9 +60,17 @@ public function getRemovedIds()
         return array(
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
+            'decorated_private' => true,
+            'decorated_private_alias' => true,
             'foo' => true,
+            'private' => true,
+            'private_alias' => true,
             'private_alias_decorator.inner' => true,
+            'private_child' => true,
             'private_decorator.inner' => true,
+            'private_not_inlined' => true,
+            'private_not_removed' => true,
+            'private_parent' => true,
         );
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_locator.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_locator.php
@@ -45,6 +45,7 @@ public function getRemovedIds()
         return array(
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
+            'baz_service' => true,
             'translator.loader_1_locator' => true,
             'translator.loader_2_locator' => true,
             'translator.loader_3_locator' => true,

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_frozen.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_frozen.php
@@ -39,6 +39,7 @@ public function getRemovedIds()
         return array(
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
+            'baz_service' => true,
         );
     }
 

diff --git a/...mfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_in_expression.php b/...mfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_in_expression.php
@@ -39,6 +39,7 @@ public function getRemovedIds()
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
             'private_bar' => true,
+            'private_foo' => true,
         );
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php
@@ -43,6 +43,7 @@ public function getRemovedIds()
         return array(
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
+            'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => true,
             'service_locator.jmktfsv' => true,
         );
     }

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_uninitialized_ref.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_uninitialized_ref.php
@@ -41,6 +41,7 @@ public function getRemovedIds()
             'Psr\\Container\\ContainerInterface' => true,
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
             'foo2' => true,
+            'foo3' => true,
         );
     }
 

diff --git a/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php
@@ -94,7 +94,7 @@ protected function instantiateController($class)
         } catch (\TypeError $e) {
         }
 
-        if ($this->container instanceof Container && in_array($class, $this->container->getRemovedIds(), true)) {
+        if ($this->container instanceof Container && isset($this->container->getRemovedIds()[$class])) {
             throw new \LogicException(sprintf('Controller "%s" cannot be fetched from the container because it is private. Did you forget to tag the service with "controller.service_arguments"?', $class), 0, $e);
         }
 

diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php
@@ -130,7 +130,7 @@ public function testNonConstructController()
         $container->expects($this->atLeastOnce())
             ->method('getRemovedIds')
             ->with()
-            ->will($this->returnValue(array(ImpossibleConstructController::class)))
+            ->will($this->returnValue(array(ImpossibleConstructController::class => true)))
         ;
 
         $resolver = $this->createControllerResolver(null, $container);