From 8bb52665539c875a650fbf878c7e0fb57fe52c1f Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 6 Jul 2018 14:11:23 +0200 Subject: [PATCH] deprecate non string requirement names --- .../Routing/Loader/AnnotationClassLoader.php | 16 ++++++++++- .../Routing/Loader/YamlFileLoader.php | 6 +++++ ...ementsWithoutPlaceholderNameController.php | 27 +++++++++++++++++++ .../requirements_without_placeholder_name.yml | 4 +++ .../Loader/AnnotationClassLoaderTest.php | 13 +++++++++ .../Tests/Loader/YamlFileLoaderTest.php | 10 +++++++ 6 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/RequirementsWithoutPlaceholderNameController.php create mode 100644 src/Symfony/Component/Routing/Tests/Fixtures/requirements_without_placeholder_name.yml diff --git a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php b/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php index c909da17bbdb..227f40ef4291 100644 --- a/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php +++ b/src/Symfony/Component/Routing/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; diff --git a/src/Symfony/Component/Routing/Loader/YamlFileLoader.php b/src/Symfony/Component/Routing/Loader/YamlFileLoader.php index b401711032ac..62b463b9b774 100644 --- a/src/Symfony/Component/Routing/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/Routing/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']; } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/RequirementsWithoutPlaceholderNameController.php b/src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/RequirementsWithoutPlaceholderNameController.php new file mode 100644 index 000000000000..301f9691d138 --- /dev/null +++ b/src/Symfony/Component/Routing/Tests/Fixtures/AnnotationFixtures/RequirementsWithoutPlaceholderNameController.php @@ -0,0 +1,27 @@ + + * + * 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() + { + } +} diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/requirements_without_placeholder_name.yml b/src/Symfony/Component/Routing/Tests/Fixtures/requirements_without_placeholder_name.yml new file mode 100644 index 000000000000..be8f04dd650a --- /dev/null +++ b/src/Symfony/Component/Routing/Tests/Fixtures/requirements_without_placeholder_name.yml @@ -0,0 +1,4 @@ +foo: + path: '/{foo}' + requirements: + - '\d+' diff --git a/src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php index dd9af9db23f7..a9db985d74e7 100644 --- a/src/Symfony/Component/Routing/Tests/Loader/AnnotationClassLoaderTest.php +++ b/src/Symfony/Component/Routing/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); diff --git a/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php index 4218eb222f05..253621c2b914 100644 --- a/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/Routing/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'); + } }