[HttpKernel] Add a better error messages when passing a private or non-tagged controller

Amrouche Hamza · Amrouche Hamza · commit 7ce94f316fae · 2017-11-29T20:09:43.000+01:00
diff --git a/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php
@@ -13,6 +13,8 @@
 
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Debug\Exception\FatalThrowableError;
+use Symfony\Component\DependencyInjection\Container;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
@@ -82,10 +84,24 @@ protected function createController($controller)
      */
     protected function instantiateController($class)
     {
-        if ($this->container->has($class)) {
-            return $this->container->get($class);
+        try {
+            if ($this->container->has($class)) {
+                return $this->container->get($class);
+            }
+            return parent::instantiateController($class);
+        } catch (FatalThrowableError $e) {
+            $this->throwWhenControllerIsNotInContainer($class);
+        } catch (\ArgumentCountError $e) {
+            $this->throwWhenControllerIsNotInContainer($class);
         }
+    }
 
-        return parent::instantiateController($class);
+    private function throwWhenControllerIsNotInContainer(string $class)
+    {
+        if ($this->container instanceof Container) {
+            if (in_array($class, $this->container->getRemovedIds())) {
+                throw new \InvalidArgumentException(sprintf('The controller %s seems to be removed from the container, which means that he maybe private, add the tag \'controller.service_arguments\' or made it public.', $class));
+            }
+        }
     }
 }
diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php
@@ -13,6 +13,7 @@
 
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Controller\ContainerControllerResolver;
 
@@ -106,6 +107,44 @@ public function testNonInstantiableController()
         $this->assertSame(array(NonInstantiableController::class, 'action'), $controller);
     }
 
+    public function testNonConstructController()
+    {
+        $container = $this->getMockBuilder(ContainerBuilder::class)->getMock();
+        $container->expects($this->at(0))
+            ->method('has')
+            ->with(ImpossibleConstructController::class)
+            ->will($this->returnValue(true))
+        ;
+
+        $container->expects($this->at(1))
+            ->method('has')
+            ->with(ImpossibleConstructController::class)
+            ->will($this->returnValue(false))
+        ;
+
+        $container->expects($this->atLeastOnce())
+            ->method('getRemovedIds')
+            ->with()
+            ->will($this->returnValue([ImpossibleConstructController::class]))
+        ;
+
+        if (method_exists($this, 'expectException')) {
+            $this->expectException(\InvalidArgumentException::class);
+            $this->expectExceptionMessage('The controller Symfony\Component\HttpKernel\Tests\Controller\ImpossibleConstructController seems to be removed from the container, which means that he maybe private, add the tag \'controller.service_arguments\' or made it public.');
+        } else {
+            $this->setExpectedException(\InvalidArgumentException::class, 'The controller Symfony\Component\HttpKernel\Tests\Controller\ImpossibleConstructController seems to be removed from the container, which means that he maybe private, add the tag \'controller.service_arguments\' or made it public.');
+        }
+
+
+        $resolver = $this->createControllerResolver(null, $container);
+        $request = Request::create('/');
+        $request->attributes->set('_controller', array(ImpossibleConstructController::class, 'action'));
+
+        $controller = $resolver->getController($request);
+
+        $this->assertSame(array(ImpossibleConstructController::class, 'action'), $controller);
+    }
+
     public function testNonInstantiableControllerWithCorrespondingService()
     {
         $service = new \stdClass();
@@ -196,3 +235,14 @@ public static function action()
     {
     }
 }
+
+class ImpossibleConstructController
+{
+    public function __construct(string $toto, $controller)
+    {
+    }
+
+    public function action()
+    {
+    }
+}
