deprecate non string requirement names

Author: xabbuh

Loader/AnnotationClassLoader.php

@@ -142,8 +142,16 @@ protected function addRoute(RouteCollection $collection, $annot, $globals, \Refl
         }
         $name = $globals['name'].$name;
 
+        $requirements = $annot->getRequirements();
+
+        foreach ($requirements as $placeholder => $requirement) {
+            if (is_int($placeholder)) {
+                @trigger_error(sprintf('A placeholder name must be a string (%d given). Did you forget to specify the placeholder key for the requirement "%s" of route "%s" in "%s::%s()"?', $placeholder, $requirement, $name, $class->getName(), $method->getName()), E_USER_DEPRECATED);
+            }
+        }
+
         $defaults = array_replace($globals['defaults'], $annot->getDefaults());
-        $requirements = array_replace($globals['requirements'], $annot->getRequirements());
+        $requirements = array_replace($globals['requirements'], $requirements);
         $options = array_replace($globals['options'], $annot->getOptions());
         $schemes = array_merge($globals['schemes'], $annot->getSchemes());
         $methods = array_merge($globals['methods'], $annot->getMethods());
@@ -305,6 +313,12 @@ protected function getGlobals(\ReflectionClass $class)
             if (null !== $annot->getCondition()) {
                 $globals['condition'] = $annot->getCondition();
             }
+
+            foreach ($globals['requirements'] as $placeholder => $requirement) {
+                if (is_int($placeholder)) {
+                    @trigger_error(sprintf('A placeholder name must be a string (%d given). Did you forget to specify the placeholder key for the requirement "%s" in "%s"?', $placeholder, $requirement, $class->getName()), E_USER_DEPRECATED);
+                }
+            }
         }
 
         return $globals;

Loader/YamlFileLoader.php

@@ -116,6 +116,12 @@ protected function parseRoute(RouteCollection $collection, $name, array $config,
         $methods = isset($config['methods']) ? $config['methods'] : array();
         $condition = isset($config['condition']) ? $config['condition'] : null;
 
+        foreach ($requirements as $placeholder => $requirement) {
+            if (is_int($placeholder)) {
+                @trigger_error(sprintf('A placeholder name must be a string (%d given). Did you forget to specify the placeholder key for the requirement "%s" of route "%s" in "%s"?', $placeholder, $requirement, $name, $path), E_USER_DEPRECATED);
+            }
+        }
+
         if (isset($config['controller'])) {
             $defaults['_controller'] = $config['controller'];
         }

Tests/Fixtures/AnnotationFixtures/RequirementsWithoutPlaceholderNameController.php

@@ -0,0 +1,27 @@
+<?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\Component\Routing\Tests\Fixtures\AnnotationFixtures;
+
+use Symfony\Component\Routing\Annotation\Route;
+
+/**
+ * @Route("/", requirements={"foo", "\d+"})
+ */
+class RequirementsWithoutPlaceholderNameController
+{
+    /**
+     * @Route("/{foo}", name="foo", requirements={"foo", "\d+"})
+     */
+    public function foo()
+    {
+    }
+}

Tests/Fixtures/requirements_without_placeholder_name.yml

@@ -0,0 +1,4 @@
+foo:
+    path: '/{foo}'
+    requirements:
+        - '\d+'

Tests/Loader/AnnotationClassLoaderTest.php

@@ -33,6 +33,7 @@
 use Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\NothingButNameController;
 use Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\PrefixedActionLocalizedRouteController;
 use Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\PrefixedActionPathController;
+use Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\RequirementsWithoutPlaceholderNameController;
 use Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\RouteWithPrefixController;
 
 class AnnotationClassLoaderTest extends AbstractAnnotationLoaderTest
@@ -87,6 +88,18 @@ public function testSimplePathRoute()
         $this->assertEquals('/path', $routes->get('action')->getPath());
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation A placeholder name must be a string (0 given). Did you forget to specify the placeholder key for the requirement "foo" in "Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\RequirementsWithoutPlaceholderNameController"?
+     * @expectedDeprecation A placeholder name must be a string (1 given). Did you forget to specify the placeholder key for the requirement "\d+" in "Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\RequirementsWithoutPlaceholderNameController"?
+     * @expectedDeprecation A placeholder name must be a string (0 given). Did you forget to specify the placeholder key for the requirement "foo" of route "foo" in "Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\RequirementsWithoutPlaceholderNameController::foo()"?
+     * @expectedDeprecation A placeholder name must be a string (1 given). Did you forget to specify the placeholder key for the requirement "\d+" of route "foo" in "Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\RequirementsWithoutPlaceholderNameController::foo()"?
+     */
+    public function testRequirementsWithoutPlaceholderName()
+    {
+        $this->loader->load(RequirementsWithoutPlaceholderNameController::class);
+    }
+
     public function testInvokableControllerLoader()
     {
         $routes = $this->loader->load(InvokableController::class);

Tests/Loader/YamlFileLoaderTest.php

@@ -304,4 +304,14 @@ public function testImportRouteWithNoTrailingSlash()
         $this->assertEquals('/slash/', $routeCollection->get('a_app_homepage')->getPath());
         $this->assertEquals('/no-slash', $routeCollection->get('b_app_homepage')->getPath());
     }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation A placeholder name must be a string (0 given). Did you forget to specify the placeholder key for the requirement "\d+" of route "foo" in "%srequirements_without_placeholder_name.yml"?
+     */
+    public function testRequirementsWithoutPlaceholderName()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
+        $loader->load('requirements_without_placeholder_name.yml');
+    }
 }