[FrameworkBundle] Giving a more specific message when a Bundle:Controller:Action controller class cannot be found

It's a detail, but it hits usability. For normal bundles (those without children), we're able to actually print the namespace where we're looking for the Controller. For bundles with children, this would be a very verbose message, but we can at least print all of the bundles that we looked inside of.

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

@@ -58,7 +58,7 @@ class ControllerNameParser
             $try = $b->getNamespace().'\\Controller\\'.$controller.'Controller';
             if (!class_exists($try)) {
                 if (null !== $this->logger) {
-                    $logs[] = sprintf('Failed finding controller "%s:%s" from namespace "%s" (%s)', $bundle, $controller, $b->getNamespace(), $try);
+                    $logs[] = sprintf('Unable to find controller "%s:%s" - class "%s" does not exist.', $bundle, $controller, $try);
                 }
             } else {
                 $class = $try;
@@ -68,15 +68,32 @@ class ControllerNameParser
         }
 
         if (null === $class) {
-            if (null !== $this->logger) {
+            $this->handleControllerNotFoundException($bundle, $controller, $logs);
-                foreach ($logs as $log) {
-                    $this->logger->info($log);
-                }
-            }
-
-            throw new \InvalidArgumentException(sprintf('Unable to find controller "%s:%s".', $bundle, $controller));
         }
 
         return $class.'::'.$action.'Action';
     }
+
+    private function handleControllerNotFoundException($bundle, $controller, array $logs)
+    {
+        if (null !== $this->logger) {
+            foreach ($logs as $log) {
+                $this->logger->info($log);
+            }
+        }
+
+        // just one log, return it as the exception
+        if (1 == count($logs)) {
+            throw new \InvalidArgumentException($logs[0]);
+        }
+
+        // many logs, use a message that mentions each searched bundle
+        $names = array();
+        foreach ($this->kernel->getBundle($bundle, false) as $b) {
+            $names[] = $b->getName();
+        }
+        $msg = sprintf('Unable to find controller "%s:%s" in bundles %s.', $bundle, $controller, implode(', ', $names));
+
+        throw new \InvalidArgumentException($msg);
+    }
 }

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

@@ -23,10 +23,7 @@ class ControllerNameParserTest extends TestCase
 {
     public function testParse()
     {
-        $kernel = new Kernel();
+        $parser = $this->createParser();
-        $kernel->boot();
-        $logger = new Logger();
-        $parser = new ControllerNameParser($kernel, $logger);
 
         $this->assertEquals('TestBundle\FooBundle\Controller\DefaultController::indexAction', $parser->parse('FooBundle:Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
         $this->assertEquals('TestBundle\FooBundle\Controller\Sub\DefaultController::indexAction', $parser->parse('FooBundle:Sub\Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
@@ -39,12 +36,37 @@ class ControllerNameParserTest extends TestCase
         } catch (\Exception $e) {
             $this->assertInstanceOf('\InvalidArgumentException', $e, '->parse() throws an \InvalidArgumentException if the controller is not an a:b:c string');
         }
+    }
+
+    /**
+     * @dataProvider getMissingControllersTest
+     */
+    public function testMissingControllers($name)
+    {
+        $parser = $this->createParser();
 
         try {
-            $parser->parse('BarBundle:Default:index');
+            $parser->parse($name);
-            $this->fail('->parse() throws a \InvalidArgumentException if the class is found but does not exist');
+            $this->fail('->parse() throws a \InvalidArgumentException if the string is in the valid format, but not matching class can be found');
         } catch (\Exception $e) {
-            $this->assertInstanceOf('\InvalidArgumentException', $e, '->parse() throws a \LogicException if the class is found but does not exist');
+            $this->assertInstanceOf('\InvalidArgumentException', $e, '->parse() throws a \InvalidArgumentException if the class is found but does not exist');
         }
     }
+
+    public function getMissingControllersTest()
+    {
+        return array(
+            array('FooBundle:Fake:index'),          // a normal bundle
+            array('SensioFooBundle:Fake:index'),    // a bundle with children
+        );
+    }
+
+    private function createParser()
+    {
+        $kernel = new Kernel();
+        $kernel->boot();
+        $logger = new Logger();
+
+        return new ControllerNameParser($kernel, $logger);
+    }
 }