feature #27462 [FrameworkBundle] Deprecate auto-injection of the cont…

…ainer in AbstractController instances (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[FrameworkBundle] Deprecate auto-injection of the container in AbstractController instances

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Should enhance DX by preventing situations like #27436.

Commits
-------

e2f344f [FrameworkBundle] Deprecate auto-injection of the container in AbstractController instances

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

@@ -5,6 +5,7 @@ CHANGELOG
 -----
 
  * Allowed configuring taggable cache pools via a new `framework.cache.pools.tags` option (bool|service-id)
+ * Deprecated auto-injection of the container in AbstractController instances, register them as service subscribers instead
 
 4.1.0
 -----

src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php

@@ -51,16 +51,22 @@ protected function createController($controller)
      */
     protected function instantiateController($class)
     {
-        return $this->configureController(parent::instantiateController($class));
+        return $this->configureController(parent::instantiateController($class), $class);
     }
 
-    private function configureController($controller)
+    private function configureController($controller, string $class)
     {
         if ($controller instanceof ContainerAwareInterface) {
             $controller->setContainer($this->container);
         }
-        if ($controller instanceof AbstractController && null !== $previousContainer = $controller->setContainer($this->container)) {
-            $controller->setContainer($previousContainer);
+        if ($controller instanceof AbstractController) {
+            if (null === $previousContainer = $controller->setContainer($this->container)) {
+                @trigger_error(sprintf('Auto-injection of the container for "%s" is deprecated since Symfony 4.2. Configure it as a service instead.', $class), E_USER_DEPRECATED);
+                // To be uncommented on Symfony 5:
+                //throw new \LogicException(sprintf('"%s" has no container set, did you forget to define it as a service subscriber?', $class));
+            } else {
+                $controller->setContainer($previousContainer);
+            }
         }
 
         return $controller;

src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerResolverTest.php

@@ -92,6 +92,10 @@ class_exists(AbstractControllerTest::class);
         $this->assertSame($container, $controller->getContainer());
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Auto-injection of the container for "Symfony\Bundle\FrameworkBundle\Tests\Controller\TestAbstractController" is deprecated since Symfony 4.2. Configure it as a service instead.
+     */
     public function testAbstractControllerGetsContainerWhenNotSet()
     {
         class_exists(AbstractControllerTest::class);
@@ -110,6 +114,10 @@ class_exists(AbstractControllerTest::class);
         $this->assertSame($container, $controller->setContainer($container));
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Auto-injection of the container for "Symfony\Bundle\FrameworkBundle\Tests\Controller\DummyController" is deprecated since Symfony 4.2. Configure it as a service instead.
+     */
     public function testAbstractControllerServiceWithFcqnIdGetsContainerWhenNotSet()
     {
         class_exists(AbstractControllerTest::class);