[Routing] fixed annotation loaders for abstract classes, added more unit tests

src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php

@@ -105,6 +105,10 @@ abstract class AnnotationClassLoader implements LoaderInterface
         );
 
         $class = new \ReflectionClass($class);
+        if ($class->isAbstract()) {
+            throw new \InvalidArgumentException(sprintf('Annotations from class "%s" cannot be read as it is abstract.', $class));
+        }
+
         if ($annot = $this->reader->getClassAnnotation($class, $this->routeAnnotationClass)) {
             if (null !== $annot->getPattern()) {
                 $globals['pattern'] = $annot->getPattern();

src/Symfony/Component/Routing/Loader/AnnotationDirectoryLoader.php

@@ -44,6 +44,11 @@ class AnnotationDirectoryLoader extends AnnotationFileLoader
             }
 
             if ($class = $this->findClass($file)) {
+                $refl = new \ReflectionClass($class);
+                if ($refl->isAbstract()) {
+                    continue;
+                }
+
                 $collection->addCollection($this->loader->load($class, $type));
             }
         }

tests/Symfony/Tests/Component/Routing/Fixtures/AnnotatedClasses/AbstractClass.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Tests\Component\Routing\Fixtures\AnnotatedClasses;
+
+abstract class AbstractClass
+{
+}

tests/Symfony/Tests/Component/Routing/Fixtures/AnnotatedClasses/FooClass.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Tests\Component\Routing\Fixtures\AnnotatedClasses;
+
+class FooClass
+{
+}

tests/Symfony/Tests/Component/Routing/Loader/AbstractAnnotationLoaderTest.php

@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Tests\Component\Routing\Loader;
+
+use Symfony\Component\Config\Loader\LoaderResolver;
+use Symfony\Component\Routing\Loader\AnnotationFileLoader;
+use Symfony\Component\Routing\Route;
+use Symfony\Component\Routing\RouteCollection;
+
+abstract class AbstractAnnotationLoaderTest extends \PHPUnit_Framework_TestCase
+{
+    public function getReader()
+    {
+        return $this->getMockBuilder('Doctrine\Common\Annotations\Reader')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+    }
+
+    public function getClassLoader($reader)
+    {
+        return $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader')
+            ->setConstructorArgs(array($reader))
+            ->getMockForAbstractClass()
+        ;
+    }
+}

tests/Symfony/Tests/Component/Routing/Loader/AnnotationClassLoaderTest.php

@@ -16,20 +16,32 @@ use Symfony\Component\Routing\Loader\AnnotationClassLoader;
 use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\RouteCollection;
 
-class AnnotationClassLoaderTest extends \PHPUnit_Framework_TestCase
+require_once __DIR__.'/../Fixtures/AnnotatedClasses/AbstractClass.php';
+require_once __DIR__.'/AbstractAnnotationLoaderTest.php';
+
+class AnnotationClassLoaderTest extends AbstractAnnotationLoaderTest
 {
     protected $loader;
 
-    protected function setUp()
+    public function setUp()
+    {
+        $this->loader = $this->getClassLoader($this->getReader());
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testLoadMissingClass()
     {
-        $this->loader = $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader')
-            ->disableOriginalConstructor()
-            ->getMockForAbstractClass();
+        $this->loader->load('MissingClass');
     }
 
-    protected function tearDown()
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testLoadAbstractClass()
     {
-        $this->loader = null;
+        $this->loader->load('Symfony\Tests\Component\Routing\Fixtures\AnnotatedClasses\AbstractClass');
     }
 
     /**

tests/Symfony/Tests/Component/Routing/Loader/AnnotationDirectoryLoaderTest.php

@@ -17,25 +17,37 @@ use Symfony\Component\Routing\RouteCollection;
 use Symfony\Component\Config\Loader\LoaderResolver;
 use Symfony\Component\Config\FileLocator;
 
-class AnnotationDirectoryLoaderTest extends \PHPUnit_Framework_TestCase
+require_once __DIR__.'/AbstractAnnotationLoaderTest.php';
+
+class AnnotationDirectoryLoaderTest extends AbstractAnnotationLoaderTest
 {
+    protected $loader;
+    protected $reader;
+
+    public function setUp()
+    {
+        $this->reader = $this->getReader();
+        $this->loader = new AnnotationDirectoryLoader(new FileLocator(), $this->getClassLoader($this->reader));
+    }
+
+    public function testLoad()
+    {
+        $this->reader->expects($this->once())->method('getClassAnnotation');
+
+        $this->loader->load(__DIR__.'/../Fixtures/AnnotatedClasses');
+    }
+
     /**
      * @covers Symfony\Component\Routing\Loader\AnnotationDirectoryLoader::supports
      */
     public function testSupports()
     {
-        $annotationClassLoader = $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader')
-           ->disableOriginalConstructor()
-           ->getMockForAbstractClass();
-
-        $loader = new AnnotationDirectoryLoader(new FileLocator(), $annotationClassLoader);
-
         $fixturesDir = __DIR__.'/../Fixtures';
 
-        $this->assertTrue($loader->supports($fixturesDir), '->supports() returns true if the resource is loadable');
-        $this->assertFalse($loader->supports('foo.foo'), '->supports() returns true if the resource is loadable');
+        $this->assertTrue($this->loader->supports($fixturesDir), '->supports() returns true if the resource is loadable');
+        $this->assertFalse($this->loader->supports('foo.foo'), '->supports() returns true if the resource is loadable');
 
-        $this->assertTrue($loader->supports($fixturesDir, 'annotation'), '->supports() checks the resource type if specified');
-        $this->assertFalse($loader->supports($fixturesDir, 'foo'), '->supports() checks the resource type if specified');
+        $this->assertTrue($this->loader->supports($fixturesDir, 'annotation'), '->supports() checks the resource type if specified');
+        $this->assertFalse($this->loader->supports($fixturesDir, 'foo'), '->supports() checks the resource type if specified');
     }
 }

tests/Symfony/Tests/Component/Routing/Loader/AnnotationFileLoaderTest.php

@@ -15,26 +15,39 @@ use Symfony\Component\Config\Loader\LoaderResolver;
 use Symfony\Component\Routing\Loader\AnnotationFileLoader;
 use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\RouteCollection;
+use Symfony\Component\Config\FileLocator;
 
-class AnnotationFileLoaderTest extends \PHPUnit_Framework_TestCase
+require_once __DIR__.'/AbstractAnnotationLoaderTest.php';
+
+class AnnotationFileLoaderTest extends AbstractAnnotationLoaderTest
 {
+    protected $loader;
+    protected $reader;
+
+    public function setUp()
+    {
+        $this->reader = $this->getReader();
+        $this->loader = new AnnotationFileLoader(new FileLocator(), $this->getClassLoader($this->reader));
+    }
+
+    public function testLoad()
+    {
+        $this->reader->expects($this->once())->method('getClassAnnotation');
+
+        $this->loader->load(__DIR__.'/../Fixtures/AnnotatedClasses/FooClass.php');
+    }
+
     /**
      * @covers Symfony\Component\Routing\Loader\AnnotationFileLoader::supports
      */
     public function testSupports()
     {
-        $annotationClassLoader = $this->getMockBuilder('Symfony\Component\Routing\Loader\AnnotationClassLoader')
-           ->disableOriginalConstructor()
-           ->getMockForAbstractClass();
-
-        $loader = new AnnotationFileLoader($this->getMock('Symfony\Component\Config\FileLocator'), $annotationClassLoader);
-
         $fixture = __DIR__.'/../Fixtures/annotated.php';
 
-        $this->assertTrue($loader->supports($fixture), '->supports() returns true if the resource is loadable');
-        $this->assertFalse($loader->supports('foo.foo'), '->supports() returns true if the resource is loadable');
+        $this->assertTrue($this->loader->supports($fixture), '->supports() returns true if the resource is loadable');
+        $this->assertFalse($this->loader->supports('foo.foo'), '->supports() returns true if the resource is loadable');
 
-        $this->assertTrue($loader->supports($fixture, 'annotation'), '->supports() checks the resource type if specified');
-        $this->assertFalse($loader->supports($fixture, 'foo'), '->supports() checks the resource type if specified');
+        $this->assertTrue($this->loader->supports($fixture, 'annotation'), '->supports() checks the resource type if specified');
+        $this->assertFalse($this->loader->supports($fixture, 'foo'), '->supports() checks the resource type if specified');
     }
 }