feature symfony#49358 [Routing] Deprecate annotations in favor of attributes (derrabus)

This PR was merged into the 6.4 branch.

Discussion
----------

[Routing] Deprecate annotations in favor of attributes

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | yes
| Tickets       | Follows symfony#50888
| License       | MIT
| Doc PR        | TODO, see symfony/symfony-docs#18589

This PR deprecates the integration of Doctrine Annotations for the Routing component.

Attributes are the way to go now if we want to annotate controllers with routing information. Existing applications can be migrated easily using [Rector](https://getrector.com/blog/how-to-upgrade-annotations-to-attributes).

Thus I believe that keeping support for oldschool Doctrine Annotations is not necessary anymore.

If this PR is accepted, I would work on a follow-up that renames all `Annotation*Loader` classes because the names of those classes are probably misleading as soon as they support attributes only.

Commits
-------

6ce15f2 [Routing] Deprecate annotations in favor of attributes

Author: nicolas-grekas

UPGRADE-6.4.md

@@ -64,6 +64,8 @@ Routing
 -------
 
  * Add native return type to `AnnotationClassLoader::setResolver()`
+ * Deprecate Doctrine annotations support in favor of native attributes
+ * Change the constructor signature of `AnnotationClassLoader` to `__construct(?string $env = null)`, passing an annotation reader as first argument is deprecated
 
 Security
 --------

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -144,6 +144,7 @@
 use Symfony\Component\RateLimiter\Storage\CacheStorage;
 use Symfony\Component\RemoteEvent\Attribute\AsRemoteEventConsumer;
 use Symfony\Component\RemoteEvent\RemoteEvent;
+use Symfony\Component\Routing\Loader\AnnotationClassLoader;
 use Symfony\Component\Routing\Loader\Psr4DirectoryLoader;
 use Symfony\Component\Scheduler\Attribute\AsSchedule;
 use Symfony\Component\Scheduler\Messenger\SchedulerTransportFactory;
@@ -1180,6 +1181,13 @@ private function registerRouterConfiguration(array $config, ContainerBuilder $co
         if (!class_exists(Psr4DirectoryLoader::class)) {
             $container->removeDefinition('routing.loader.psr4');
         }
+
+        if ($this->isInitializedConfigEnabled('annotations') && (new \ReflectionClass(AnnotationClassLoader::class))->hasProperty('reader')) {
+            $container->getDefinition('routing.loader.annotation')->setArguments([
+                new Reference('annotation_reader'),
+                '%kernel.environment%',
+            ]);
+        }
     }
 
     private function registerSessionConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader): void

src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.php

@@ -94,7 +94,6 @@
 
         ->set('routing.loader.annotation', AnnotatedRouteControllerLoader::class)
             ->args([
-                service('annotation_reader')->nullOnInvalid(),
                 '%kernel.environment%',
             ])
             ->tag('routing.loader', ['priority' => -10])

src/Symfony/Bundle/FrameworkBundle/composer.json

@@ -30,7 +30,7 @@
         "symfony/polyfill-mbstring": "~1.0",
         "symfony/filesystem": "^5.4|^6.0|^7.0",
         "symfony/finder": "^5.4|^6.0|^7.0",
-        "symfony/routing": "^6.1|^7.0"
+        "symfony/routing": "^6.4|^7.0"
     },
     "require-dev": {
         "doctrine/annotations": "^1.13.1|^2",

src/Symfony/Component/Routing/CHANGELOG.md

@@ -6,6 +6,8 @@ CHANGELOG
 
  * Add FQCN and FQCN::method aliases for routes loaded from attributes/annotations when applicable
  * Add native return type to `AnnotationClassLoader::setResolver()`
+ * Deprecate Doctrine annotations support in favor of native attributes
+ * Change the constructor signature of `AnnotationClassLoader` to `__construct(?string $env = null)`, passing an annotation reader as first argument is deprecated
 
 6.2
 ---

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

@@ -26,33 +26,14 @@
  * time, this method should define some PHP callable to be called for the route
  * (a controller in MVC speak).
  *
- * The @Route annotation can be set on the class (for global parameters),
+ * The #[Route] attribute can be set on the class (for global parameters),
  * and on each method.
  *
- * The @Route annotation main value is the route path. The annotation also
+ * The #[Route] attribute main value is the route path. The attribute also
  * recognizes several parameters: requirements, options, defaults, schemes,
  * methods, host, and name. The name parameter is mandatory.
  * Here is an example of how you should be able to use it:
- *     /**
- *      * @Route("/Blog")
- *      * /
- *     class Blog
- *     {
- *         /**
- *          * @Route("/", name="blog_index")
- *          * /
- *         public function index()
- *         {
- *         }
- *         /**
- *          * @Route("/{id}", name="blog_post", requirements = {"id" = "\d+"})
- *          * /
- *         public function show()
- *         {
- *         }
- *     }
  *
- * On PHP 8, the annotation class can be used as an attribute as well:
  *     #[Route('/Blog')]
  *     class Blog
  *     {
@@ -71,7 +52,16 @@
  */
 abstract class AnnotationClassLoader implements LoaderInterface
 {
+    /**
+     * @var Reader|null
+     *
+     * @deprecated in Symfony 6.4, this property will be removed in Symfony 7.
+     */
     protected $reader;
+
+    /**
+     * @var string|null
+     */
     protected $env;
 
     /**
@@ -84,10 +74,27 @@ abstract class AnnotationClassLoader implements LoaderInterface
      */
     protected $defaultRouteIndex = 0;
 
-    public function __construct(Reader $reader = null, string $env = null)
+    private bool $hasDeprecatedAnnotations = false;
+
+    /**
+     * @param string|null $env
+     */
+    public function __construct($env = null)
     {
-        $this->reader = $reader;
-        $this->env = $env;
+        if ($env instanceof Reader || null === $env && \func_num_args() > 1 && null !== func_get_arg(1)) {
+            trigger_deprecation('symfony/routing', '6.4', 'Passing an instance of "%s" as first and the environment as second argument to "%s" is deprecated. Pass the environment as first argument instead.', Reader::class, __METHOD__);
+
+            $this->reader = $env;
+            $env = \func_num_args() > 1 ? func_get_arg(1) : null;
+        }
+
+        if (\is_string($env) || null === $env) {
+            $this->env = $env;
+        } elseif ($env instanceof \Stringable || \is_scalar($env)) {
+            $this->env = (string) $env;
+        } else {
+            throw new \TypeError(__METHOD__.sprintf(': Parameter $env was expected to be a string or null, "%s" given.', get_debug_type($env)));
+        }
     }
 
     /**
@@ -116,43 +123,48 @@ public function load(mixed $class, string $type = null): RouteCollection
             throw new \InvalidArgumentException(sprintf('Annotations from class "%s" cannot be read as it is abstract.', $class->getName()));
         }
 
-        $globals = $this->getGlobals($class);
+        $this->hasDeprecatedAnnotations = false;
 
-        $collection = new RouteCollection();
-        $collection->addResource(new FileResource($class->getFileName()));
-
-        if ($globals['env'] && $this->env !== $globals['env']) {
-            return $collection;
-        }
+        try {
+            $globals = $this->getGlobals($class);
+            $collection = new RouteCollection();
+            $collection->addResource(new FileResource($class->getFileName()));
+            if ($globals['env'] && $this->env !== $globals['env']) {
+                return $collection;
+            }
+            $fqcnAlias = false;
+            foreach ($class->getMethods() as $method) {
+                $this->defaultRouteIndex = 0;
+                $routeNamesBefore = array_keys($collection->all());
+                foreach ($this->getAnnotations($method) as $annot) {
+                    $this->addRoute($collection, $annot, $globals, $class, $method);
+                    if ('__invoke' === $method->name) {
+                        $fqcnAlias = true;
+                    }
+                }
 
-        $fqcnAlias = false;
-        foreach ($class->getMethods() as $method) {
-            $this->defaultRouteIndex = 0;
-            $routeNamesBefore = array_keys($collection->all());
-            foreach ($this->getAnnotations($method) as $annot) {
-                $this->addRoute($collection, $annot, $globals, $class, $method);
-                if ('__invoke' === $method->name) {
+                if (1 === $collection->count() - \count($routeNamesBefore)) {
+                    $newRouteName = current(array_diff(array_keys($collection->all()), $routeNamesBefore));
+                    $collection->addAlias(sprintf('%s::%s', $class->name, $method->name), $newRouteName);
+                }
+            }
+            if (0 === $collection->count() && $class->hasMethod('__invoke')) {
+                $globals = $this->resetGlobals();
+                foreach ($this->getAnnotations($class) as $annot) {
+                    $this->addRoute($collection, $annot, $globals, $class, $class->getMethod('__invoke'));
                     $fqcnAlias = true;
                 }
             }
-
-            if (1 === $collection->count() - \count($routeNamesBefore)) {
-                $newRouteName = current(array_diff(array_keys($collection->all()), $routeNamesBefore));
-                $collection->addAlias(sprintf('%s::%s', $class->name, $method->name), $newRouteName);
+            if ($fqcnAlias && 1 === $collection->count()) {
+                $collection->addAlias($class->name, $invokeRouteName = key($collection->all()));
+                $collection->addAlias(sprintf('%s::__invoke', $class->name), $invokeRouteName);
             }
-        }
 
-        if (0 === $collection->count() && $class->hasMethod('__invoke')) {
-            $globals = $this->resetGlobals();
-            foreach ($this->getAnnotations($class) as $annot) {
-                $this->addRoute($collection, $annot, $globals, $class, $class->getMethod('__invoke'));
-                $fqcnAlias = true;
+            if ($this->hasDeprecatedAnnotations) {
+                trigger_deprecation('symfony/routing', '6.4', 'Class "%s" uses Doctrine Annotations to configure routes, which is deprecated. Use PHP attributes instead.', $class->getName());
             }
-        }
-
-        if ($fqcnAlias && 1 === $collection->count()) {
-            $collection->addAlias($class->name, $invokeRouteName = key($collection->all()));
-            $collection->addAlias(sprintf('%s::__invoke', $class->name), $invokeRouteName);
+        } finally {
+            $this->hasDeprecatedAnnotations = false;
         }
 
         return $collection;
@@ -279,7 +291,7 @@ protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMetho
     }
 
     /**
-     * @return array
+     * @return array<string, mixed>
      */
     protected function getGlobals(\ReflectionClass $class)
     {
@@ -289,8 +301,8 @@ protected function getGlobals(\ReflectionClass $class)
         if ($attribute = $class->getAttributes($this->routeAnnotationClass, \ReflectionAttribute::IS_INSTANCEOF)[0] ?? null) {
             $annot = $attribute->newInstance();
         }
-        if (!$annot && $this->reader) {
-            $annot = $this->reader->getClassAnnotation($class, $this->routeAnnotationClass);
+        if (!$annot && $annot = $this->reader?->getClassAnnotation($class, $this->routeAnnotationClass)) {
+            $this->hasDeprecatedAnnotations = true;
         }
 
         if ($annot) {
@@ -377,11 +389,9 @@ protected function createRoute(string $path, array $defaults, array $requirement
     abstract protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot);
 
     /**
-     * @param \ReflectionClass|\ReflectionMethod $reflection
-     *
      * @return iterable<int, RouteAnnotation>
      */
-    private function getAnnotations(object $reflection): iterable
+    private function getAnnotations(\ReflectionClass|\ReflectionMethod $reflection): iterable
     {
         foreach ($reflection->getAttributes($this->routeAnnotationClass, \ReflectionAttribute::IS_INSTANCEOF) as $attribute) {
             yield $attribute->newInstance();
@@ -397,6 +407,8 @@ private function getAnnotations(object $reflection): iterable
 
         foreach ($annotations as $annotation) {
             if ($annotation instanceof $this->routeAnnotationClass) {
+                $this->hasDeprecatedAnnotations = true;
+
                 yield $annotation;
             }
         }

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

@@ -11,10 +11,13 @@
 
 namespace Symfony\Component\Routing\Tests\Fixtures\AnnotatedClasses;
 
+use Symfony\Component\Routing\Annotation\Route;
+
 abstract class AbstractClass
 {
     abstract public function abstractRouteAction();
 
+    #[Route('/path/to/route/{arg1}')]
     public function routeAction($arg1, $arg2 = 'defaultValue2', $arg3 = 'defaultValue3')
     {
     }

src/Symfony/Component/Routing/Tests/Fixtures/OtherAnnotatedClasses/VariadicClass.php

@@ -11,8 +11,11 @@
 
 namespace Symfony\Component\Routing\Tests\Fixtures\OtherAnnotatedClasses;
 
+use Symfony\Component\Routing\Annotation\Route;
+
 class VariadicClass
 {
+    #[Route('/path/to/{id}')]
     public function routeAction(...$params)
     {
     }

src/Symfony/Component/Routing/Tests/Fixtures/TraceableAnnotationClassLoader.php

@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Routing\Tests\Fixtures;
+
+use Symfony\Component\Routing\Loader\AnnotationClassLoader;
+use Symfony\Component\Routing\Route;
+use Symfony\Component\Routing\RouteCollection;
+
+final class TraceableAnnotationClassLoader extends AnnotationClassLoader
+{
+    /** @var list<string> */
+    public array $foundClasses = [];
+
+    public function load(mixed $class, string $type = null): RouteCollection
+    {
+        if (!is_string($class)) {
+            throw new \InvalidArgumentException(sprintf('Expected string, got "%s"', get_debug_type($class)));
+        }
+
+        $this->foundClasses[] = $class;
+
+        return parent::load($class, $type);
+    }
+
+    protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $annot): void
+    {
+    }
+}

src/Symfony/Component/Routing/Tests/Loader/AbstractAnnotationLoaderTestCase.php

@@ -18,10 +18,7 @@
 
 abstract class AnnotationClassLoaderTestCase extends TestCase
 {
-    /**
-     * @var AnnotationClassLoader
-     */
-    protected $loader;
+    protected AnnotationClassLoader $loader;
 
     /**
      * @dataProvider provideTestSupportsChecksResource
@@ -31,7 +28,7 @@ public function testSupportsChecksResource($resource, $expectedSupports)
         $this->assertSame($expectedSupports, $this->loader->supports($resource), '->supports() returns true if the resource is loadable');
     }
 
-    public static function provideTestSupportsChecksResource()
+    public static function provideTestSupportsChecksResource(): array
     {
         return [
             ['class', true],