minor #25339 [DX][HttpKernel] Throw a sensible exception when control…

…ler has been removed (sroze)

This PR was merged into the 3.4 branch.

Discussion
----------

[DX][HttpKernel] Throw a sensible exception when controller has been removed

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

Following on #25201, we need to throw the same kind of sensible exception when the controller service is not found.

Commits
-------

458d63f Throw a sensible exception when controller has been removed

src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php

@@ -63,19 +63,28 @@ protected function createController($controller)
             return parent::createController($controller);
         }
 
+        $method = null;
         if (1 == substr_count($controller, ':')) {
             // controller in the "service:method" notation
-            list($service, $method) = explode(':', $controller, 2);
+            list($controller, $method) = explode(':', $controller, 2);
+        }
+
+        if (!$this->container->has($controller)) {
+            $this->throwExceptionIfControllerWasRemoved($controller);
+
+            throw new \LogicException(sprintf('Controller not found: service "%s" does not exist.', $controller));
+        }
 
-            return array($this->container->get($service), $method);
+        $service = $this->container->get($controller);
+        if (null !== $method) {
+            return array($service, $method);
         }
 
-        if ($this->container->has($controller) && method_exists($service = $this->container->get($controller), '__invoke')) {
-            // invokable controller in the "service" notation
-            return $service;
+        if (!method_exists($service, '__invoke')) {
+            throw new \LogicException(sprintf('Controller "%s" cannot be called without a method name. Did you forget an "__invoke" method?', $controller));
         }
 
-        throw new \LogicException(sprintf('Unable to parse the controller name "%s".', $controller));
+        return $service;
     }
 
     /**
@@ -94,10 +103,19 @@ protected function instantiateController($class)
         } catch (\TypeError $e) {
         }
 
-        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);
-        }
+        $this->throwExceptionIfControllerWasRemoved($class, $e);
 
         throw $e;
     }
+
+    /**
+     * @param string                     $controller
+     * @param \Exception|\Throwable|null $previous
+     */
+    private function throwExceptionIfControllerWasRemoved($controller, $previous = null)
+    {
+        if ($this->container instanceof Container && isset($this->container->getRemovedIds()[$controller])) {
+            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"?', $controller), 0, $previous);
+        }
+    }
 }

src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php

@@ -23,6 +23,10 @@ class ContainerControllerResolverTest extends ControllerResolverTest
     public function testGetControllerService()
     {
         $container = $this->createMockContainer();
+        $container->expects($this->once())
+            ->method('has')
+            ->with('foo')
+            ->will($this->returnValue(true));
         $container->expects($this->once())
             ->method('get')
             ->with('foo')
@@ -175,6 +179,57 @@ public function testNonInstantiableControllerWithCorrespondingService()
         $this->assertSame(array($service, 'action'), $controller);
     }
 
+    /**
+     * @expectedException \LogicException
+     * @expectedExceptionMessage Controller "app.my_controller" cannot be fetched from the container because it is private. Did you forget to tag the service with "controller.service_arguments"?
+     */
+    public function testExceptionWhenUsingRemovedControllerService()
+    {
+        $container = $this->getMockBuilder(Container::class)->getMock();
+        $container->expects($this->at(0))
+            ->method('has')
+            ->with('app.my_controller')
+            ->will($this->returnValue(false))
+        ;
+
+        $container->expects($this->atLeastOnce())
+            ->method('getRemovedIds')
+            ->with()
+            ->will($this->returnValue(array('app.my_controller' => true)))
+        ;
+
+        $resolver = $this->createControllerResolver(null, $container);
+
+        $request = Request::create('/');
+        $request->attributes->set('_controller', 'app.my_controller');
+        $resolver->getController($request);
+    }
+
+    /**
+     * @expectedException \LogicException
+     * @expectedExceptionMessage Controller "app.my_controller" cannot be called without a method name. Did you forget an "__invoke" method?
+     */
+    public function testExceptionWhenUsingControllerWithoutAnInvokeMethod()
+    {
+        $container = $this->getMockBuilder(Container::class)->getMock();
+        $container->expects($this->once())
+            ->method('has')
+            ->with('app.my_controller')
+            ->will($this->returnValue(true))
+        ;
+        $container->expects($this->once())
+            ->method('get')
+            ->with('app.my_controller')
+            ->will($this->returnValue(new ImpossibleConstructController('toto', 'controller')))
+        ;
+
+        $resolver = $this->createControllerResolver(null, $container);
+
+        $request = Request::create('/');
+        $request->attributes->set('_controller', 'app.my_controller');
+        $resolver->getController($request);
+    }
+
     /**
      * @dataProvider getUndefinedControllers
      */
@@ -197,9 +252,9 @@ public function testGetControllerOnNonUndefinedFunction($controller, $exceptionN
     public function getUndefinedControllers()
     {
         return array(
-            array('foo', \LogicException::class, '/Unable to parse the controller name "foo"\./'),
+            array('foo', \LogicException::class, '/Controller not found: service "foo" does not exist\./'),
             array('oof::bar', \InvalidArgumentException::class, '/Class "oof" does not exist\./'),
-            array('stdClass', \LogicException::class, '/Unable to parse the controller name "stdClass"\./'),
+            array('stdClass', \LogicException::class, '/Controller not found: service "stdClass" does not exist\./'),
             array(
                 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::bar',
                 \InvalidArgumentException::class,