From fdd2aaf88b8a575f8ea9061c85648a88b6cb986d Mon Sep 17 00:00:00 2001 From: Jeroen Thora Date: Thu, 16 Dec 2021 12:40:48 +0100 Subject: [PATCH 1/3] Allow symfony 6 and add requirement typehints --- .github/workflows/test-application.yaml | 5 ++++- composer.json | 12 ++++++------ src/ChainRouteCollection.php | 16 +++++++--------- src/ChainRouter.php | 10 +++++----- src/ContentAwareGenerator.php | 2 +- src/DynamicRouter.php | 10 +++++----- src/LazyRouteCollection.php | 14 +++++--------- src/NestedMatcher/NestedMatcher.php | 2 +- src/NestedMatcher/UrlMatcher.php | 2 +- src/PagedRouteCollection.php | 14 ++++++++------ src/ProviderBasedGenerator.php | 2 +- tests/Unit/Routing/ContentAwareGeneratorTest.php | 2 +- .../Unit/Routing/ProviderBasedGeneratorTest.php | 2 +- tests/Unit/Routing/RouteMock.php | 4 ++-- 14 files changed, 48 insertions(+), 49 deletions(-) diff --git a/.github/workflows/test-application.yaml b/.github/workflows/test-application.yaml index 39fdb967..979b9d55 100644 --- a/.github/workflows/test-application.yaml +++ b/.github/workflows/test-application.yaml @@ -31,7 +31,10 @@ jobs: symfony-require: "5.0.*" - php-version: "8.0" - symfony-require: "*" + symfony-require: "5.0.*" + + - php-version: "8.0" + symfony-require: "6.0.*" steps: - name: "Checkout project" diff --git a/composer.json b/composer.json index 68c24cee..99e1883d 100644 --- a/composer.json +++ b/composer.json @@ -16,15 +16,15 @@ ], "require": { "php": "^7.2 || ^8.0", - "symfony/routing": "^4.4 || ^5.0", - "symfony/http-kernel": "^4.4 || ^5.0", + "symfony/routing": "^4.4 || ^5.0 || ^6.0", + "symfony/http-kernel": "^4.4 || ^5.0 || ^6.0", "psr/log": "^1.0 || ^2.0 || ^3.0" }, "require-dev": { - "symfony/phpunit-bridge": "^5.0", - "symfony/dependency-injection": "^4.4 || ^5.0", - "symfony/config": "^4.4 || ^5.0", - "symfony/event-dispatcher": "^4.4 || ^5.0", + "symfony/phpunit-bridge": "^5.4 || ^6.0", + "symfony/dependency-injection": "^4.4 || ^5.0 || ^6.0", + "symfony/config": "^4.4 || ^5.0 || ^6.0", + "symfony/event-dispatcher": "^4.4 || ^5.0 || ^6.0", "symfony-cmf/testing": "^3@dev" }, "suggest": { diff --git a/src/ChainRouteCollection.php b/src/ChainRouteCollection.php index 1bdd088f..5a0bd0f0 100644 --- a/src/ChainRouteCollection.php +++ b/src/ChainRouteCollection.php @@ -43,17 +43,15 @@ public function __clone() * * @return \ArrayIterator An \ArrayIterator object for iterating over routes */ - public function getIterator() + public function getIterator(): \ArrayIterator { return new \ArrayIterator($this->all()); } /** * Gets the number of Routes in this collection. - * - * @return int The number of routes */ - public function count() + public function count(): int { $count = 0; foreach ($this->routeCollections as $routeCollection) { @@ -81,7 +79,7 @@ public function add($name, Route $route, int $priority = 0) * * @return Route[] An array of routes */ - public function all() + public function all(): array { $routeCollectionAll = new RouteCollection(); foreach ($this->routeCollections as $routeCollection) { @@ -95,10 +93,8 @@ public function all() * Gets a route by name. * * @param string $name The route name - * - * @return Route|null A Route instance or null when not found */ - public function get($name) + public function get($name): ?Route { foreach ($this->routeCollections as $routeCollection) { $route = $routeCollection->get($name); @@ -106,6 +102,8 @@ public function get($name) return $route; } } + + return null; } /** @@ -240,7 +238,7 @@ public function setMethods($methods) * * @return ResourceInterface[] An array of resources */ - public function getResources() + public function getResources(): array { if (0 === count($this->routeCollections)) { return []; diff --git a/src/ChainRouter.php b/src/ChainRouter.php index cf51058f..d3b14ca4 100644 --- a/src/ChainRouter.php +++ b/src/ChainRouter.php @@ -70,7 +70,7 @@ public function __construct(LoggerInterface $logger = null) /** * @return RequestContext */ - public function getContext() + public function getContext(): RequestContext { if (!$this->context) { $this->context = new RequestContext(); @@ -143,7 +143,7 @@ protected function sortRouters() * * Note: You should use matchRequest if you can. */ - public function match($pathinfo) + public function match($pathinfo): array { return $this->doMatch($pathinfo); } @@ -153,7 +153,7 @@ public function match($pathinfo) * * Loops through all routes and tries to match the passed request. */ - public function matchRequest(Request $request) + public function matchRequest(Request $request): array { return $this->doMatch($request->getPathInfo(), $request); } @@ -213,7 +213,7 @@ private function doMatch($pathinfo, Request $request = null) /** * {@inheritdoc} * - * @param mixed $name + * @param string $name * * The CMF routing system used to allow to pass route objects as $name to generate the route. * Since Symfony 5.0, the UrlGeneratorInterface declares $name as string. We widen the contract @@ -224,7 +224,7 @@ private function doMatch($pathinfo, Request $request = null) * Loops through all registered routers and returns a router if one is found. * It will always return the first route generated. */ - public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH) + public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH): string { if (is_object($name)) { @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`.', E_USER_DEPRECATED); diff --git a/src/ContentAwareGenerator.php b/src/ContentAwareGenerator.php index 327b9c87..b877a789 100644 --- a/src/ContentAwareGenerator.php +++ b/src/ContentAwareGenerator.php @@ -71,7 +71,7 @@ public function setContentRepository(ContentRepositoryInterface $contentReposito * * @throws RouteNotFoundException If there is no such route in the database */ - public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH) + public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH): string { if ($name instanceof SymfonyRoute) { @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT` resp the content id with content_id.', E_USER_DEPRECATED); diff --git a/src/DynamicRouter.php b/src/DynamicRouter.php index f0825257..f9fb0b4d 100644 --- a/src/DynamicRouter.php +++ b/src/DynamicRouter.php @@ -159,11 +159,11 @@ public function getGenerator() * Instead, Pass the RouteObjectInterface::OBJECT_BASED_ROUTE_NAME as route name and the object * in the parameters with key RouteObjectInterface::ROUTE_OBJECT. * - * @param string|Route $name The name of the route or the Route instance + * @param string $name The name of the route * * @throws RouteNotFoundException if route doesn't exist */ - public function generate($name, $parameters = [], $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH) + public function generate($name, $parameters = [], $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH): string { if (is_object($name)) { @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT', E_USER_DEPRECATED); @@ -213,7 +213,7 @@ public function supports($name) * * @api */ - public function match($pathinfo) + public function match($pathinfo): array { @trigger_error(__METHOD__.'() is deprecated since version 1.3 and will be removed in 2.0. Use matchRequest() instead.', E_USER_DEPRECATED); @@ -252,7 +252,7 @@ public function match($pathinfo) * @throws MethodNotAllowedException If a matching resource was found but * the request method is not allowed */ - public function matchRequest(Request $request) + public function matchRequest(Request $request): array { if ($this->eventDispatcher) { $event = new RouterMatchEvent($request); @@ -294,7 +294,7 @@ public function setContext(RequestContext $context) * * @api */ - public function getContext() + public function getContext(): RequestContext { return $this->context; } diff --git a/src/LazyRouteCollection.php b/src/LazyRouteCollection.php index e71390c7..a8359cfd 100644 --- a/src/LazyRouteCollection.php +++ b/src/LazyRouteCollection.php @@ -32,17 +32,15 @@ public function __construct(RouteProviderInterface $provider) /** * {@inheritdoc} */ - public function getIterator() + public function getIterator(): \ArrayIterator { return new \ArrayIterator($this->all()); } /** * Gets the number of Routes in this collection. - * - * @return int The number of routes */ - public function count() + public function count(): int { return count($this->all()); } @@ -52,7 +50,7 @@ public function count() * * @return Route[] An array of routes */ - public function all() + public function all(): array { return $this->provider->getRoutesByNames(null); } @@ -61,15 +59,13 @@ public function all() * Gets a route by name. * * @param string $name The route name - * - * @return Route|null A Route instance or null when not found */ - public function get($name) + public function get($name): ?Route { try { return $this->provider->getRouteByName($name); } catch (RouteNotFoundException $e) { - return; + return null; } } } diff --git a/src/NestedMatcher/NestedMatcher.php b/src/NestedMatcher/NestedMatcher.php index 2e5e2d2a..7cd91f3c 100644 --- a/src/NestedMatcher/NestedMatcher.php +++ b/src/NestedMatcher/NestedMatcher.php @@ -135,7 +135,7 @@ public function setFinalMatcher(FinalMatcherInterface $final) /** * {@inheritdoc} */ - public function matchRequest(Request $request) + public function matchRequest(Request $request): array { $collection = $this->routeProvider->getRouteCollectionForRequest($request); if (!count($collection)) { diff --git a/src/NestedMatcher/UrlMatcher.php b/src/NestedMatcher/UrlMatcher.php index b4fdb213..57d00cf3 100644 --- a/src/NestedMatcher/UrlMatcher.php +++ b/src/NestedMatcher/UrlMatcher.php @@ -43,7 +43,7 @@ public function finalMatch(RouteCollection $collection, Request $request) /** * {@inheritdoc} */ - protected function getAttributes(Route $route, $name, array $attributes) + protected function getAttributes(Route $route, $name, array $attributes): array { if ($route instanceof RouteObjectInterface && is_string($route->getRouteKey())) { $name = $route->getRouteKey(); diff --git a/src/PagedRouteCollection.php b/src/PagedRouteCollection.php index a9620bce..02bf30a5 100644 --- a/src/PagedRouteCollection.php +++ b/src/PagedRouteCollection.php @@ -69,8 +69,9 @@ protected function loadNextElements($offset) } /** - * {@inheritdoc} + * @return mixed */ + #[\ReturnTypeWillChange] public function current() { return current($this->currentRoutes); @@ -79,7 +80,7 @@ public function current() /** * {@inheritdoc} */ - public function next() + public function next(): void { $result = next($this->currentRoutes); if (false === $result) { @@ -89,8 +90,9 @@ public function next() } /** - * {@inheritdoc} + * @return mixed */ + #[\ReturnTypeWillChange] public function key() { return key($this->currentRoutes); @@ -99,15 +101,15 @@ public function key() /** * {@inheritdoc} */ - public function valid() + public function valid(): bool { - return key($this->currentRoutes); + return null !== key($this->currentRoutes); } /** * {@inheritdoc} */ - public function rewind() + public function rewind(): void { $this->current = 0; $this->currentRoutes = null; diff --git a/src/ProviderBasedGenerator.php b/src/ProviderBasedGenerator.php index 6dee8df5..ee1761c2 100644 --- a/src/ProviderBasedGenerator.php +++ b/src/ProviderBasedGenerator.php @@ -52,7 +52,7 @@ public function __construct(RouteProviderInterface $provider, LoggerInterface $l * * @param mixed $name */ - public function generate($name, $parameters = [], $referenceType = self::ABSOLUTE_PATH) + public function generate($name, $parameters = [], $referenceType = self::ABSOLUTE_PATH): string { if (is_object($name)) { @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`', E_USER_DEPRECATED); diff --git a/tests/Unit/Routing/ContentAwareGeneratorTest.php b/tests/Unit/Routing/ContentAwareGeneratorTest.php index 5a70efd1..3db4e071 100644 --- a/tests/Unit/Routing/ContentAwareGeneratorTest.php +++ b/tests/Unit/Routing/ContentAwareGeneratorTest.php @@ -559,7 +559,7 @@ public function testGetRouteDebugMessageLegacy(): void */ class TestableContentAwareGenerator extends ContentAwareGenerator { - protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, array $requiredSchemes = []) + protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, array $requiredSchemes = []): string { return 'result_url'; } diff --git a/tests/Unit/Routing/ProviderBasedGeneratorTest.php b/tests/Unit/Routing/ProviderBasedGeneratorTest.php index 13e39594..2916944b 100644 --- a/tests/Unit/Routing/ProviderBasedGeneratorTest.php +++ b/tests/Unit/Routing/ProviderBasedGeneratorTest.php @@ -180,7 +180,7 @@ public function testGenerateByRoute(): void */ class TestableProviderBasedGenerator extends ProviderBasedGenerator { - protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, array $requiredSchemes = []) + protected function doGenerate(array $variables, array $defaults, array $requirements, array $tokens, array $parameters, string $name, int $referenceType, array $hostTokens, array $requiredSchemes = []): string { $url = 'result_url'; if ($parameters && $query = http_build_query($parameters, '', '&', PHP_QUERY_RFC3986)) { diff --git a/tests/Unit/Routing/RouteMock.php b/tests/Unit/Routing/RouteMock.php index 7a95bd02..1f904cc0 100644 --- a/tests/Unit/Routing/RouteMock.php +++ b/tests/Unit/Routing/RouteMock.php @@ -28,7 +28,7 @@ public function getContent() return; } - public function getDefaults() + public function getDefaults(): array { $defaults = []; if (null !== $this->locale) { @@ -38,7 +38,7 @@ public function getDefaults() return $defaults; } - public function getRequirement($key) + public function getRequirement($key): ?string { if ('_locale' !== $key) { throw new \Exception(); From 3c65ef3cf2081064a5591eed5eed2990f7b338fe Mon Sep 17 00:00:00 2001 From: Jeroen Thora Date: Thu, 16 Dec 2021 15:38:00 +0100 Subject: [PATCH 2/3] Remove deprecated code in generate method --- src/ChainRouter.php | 13 --- src/ContentAwareGenerator.php | 37 ++---- src/DynamicRouter.php | 4 - src/ProviderBasedGenerator.php | 15 +-- tests/Unit/Candidates/CandidatesTest.php | 2 + tests/Unit/Routing/ChainRouterTest.php | 106 ------------------ .../Routing/ContentAwareGeneratorTest.php | 49 +------- .../Routing/ProviderBasedGeneratorTest.php | 24 +--- 8 files changed, 17 insertions(+), 233 deletions(-) diff --git a/src/ChainRouter.php b/src/ChainRouter.php index d3b14ca4..ee608b45 100644 --- a/src/ChainRouter.php +++ b/src/ChainRouter.php @@ -67,9 +67,6 @@ public function __construct(LoggerInterface $logger = null) $this->logger = $logger; } - /** - * @return RequestContext - */ public function getContext(): RequestContext { if (!$this->context) { @@ -215,21 +212,11 @@ private function doMatch($pathinfo, Request $request = null) * * @param string $name * - * The CMF routing system used to allow to pass route objects as $name to generate the route. - * Since Symfony 5.0, the UrlGeneratorInterface declares $name as string. We widen the contract - * for BC but deprecate passing non-strings. - * Instead, Pass the RouteObjectInterface::OBJECT_BASED_ROUTE_NAME as route name and the object - * in the parameters with key RouteObjectInterface::ROUTE_OBJECT. - * * Loops through all registered routers and returns a router if one is found. * It will always return the first route generated. */ public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH): string { - if (is_object($name)) { - @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`.', E_USER_DEPRECATED); - } - $debug = []; foreach ($this->all() as $router) { diff --git a/src/ContentAwareGenerator.php b/src/ContentAwareGenerator.php index b877a789..5242a9f1 100644 --- a/src/ContentAwareGenerator.php +++ b/src/ContentAwareGenerator.php @@ -56,28 +56,16 @@ public function setContentRepository(ContentRepositoryInterface $contentReposito /** * {@inheritdoc} * - * The CMF routing system used to allow to pass route objects as $name to generate the route. - * Since Symfony 5.0, the UrlGeneratorInterface declares $name as string. We widen the contract - * for BC but deprecate passing non-strings. - * Instead, Pass the RouteObjectInterface::OBJECT_BASED_ROUTE_NAME as route name and the object - * in the parameters with key RouteObjectInterface::ROUTE_OBJECT or the ID of a - * RouteReferrersReadInterface in 'content_id. - * - * @param mixed $name ignored - * @param array $parameters must either contain the field 'route' with a - * RouteObjectInterface or the field 'content_id' - * with the id of a document implementing - * RouteReferrersReadInterface + * @param string $name ignored + * @param array $parameters must either contain the field 'route' with a + * RouteObjectInterface or the field 'content_id' + * with the id of a document implementing RouteReferrersReadInterface * * @throws RouteNotFoundException If there is no such route in the database */ public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH): string { - if ($name instanceof SymfonyRoute) { - @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT` resp the content id with content_id.', E_USER_DEPRECATED); - - $route = $this->getBestLocaleRoute($name, $parameters); - } elseif (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name) { + if (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name) { if (array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof SymfonyRoute ) { @@ -85,7 +73,7 @@ public function generate($name, $parameters = [], $absolute = UrlGeneratorInterf } else { $route = $this->getRouteByContent($name, $parameters); } - } elseif (is_string($name) && $name) { + } elseif (!empty($name)) { $route = $this->getRouteByName($name, $parameters); } else { $route = $this->getRouteByContent($name, $parameters); @@ -167,7 +155,6 @@ protected function getBestLocaleRoute(SymfonyRoute $route, $parameters) * If no route with matching locale is found, falls back to just return the * first route. * - * @param mixed $name * @param array $parameters which should contain a content field containing * a RouteReferrersReadInterface object * @@ -175,13 +162,9 @@ protected function getBestLocaleRoute(SymfonyRoute $route, $parameters) * * @throws RouteNotFoundException if no route can be determined */ - protected function getRouteByContent($name, &$parameters) + protected function getRouteByContent(string $name, &$parameters) { - if ($name instanceof RouteReferrersReadInterface) { - @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`.', E_USER_DEPRECATED); - - $content = $name; - } elseif (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name + if (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof RouteReferrersReadInterface ) { @@ -197,9 +180,7 @@ protected function getRouteByContent($name, &$parameters) throw new RouteNotFoundException('Content repository did not return a RouteReferrersReadInterface instance for id '.$parameters['content_id']); } } else { - $hint = is_object($name) ? get_class($name) : gettype($name); - - throw new RouteNotFoundException("The route name argument '$hint' is not a RouteReferrersReadInterface instance and there is no 'content_id' parameter"); + throw new RouteNotFoundException(sprintf("The route name argument '%s' is not a RouteReferrersReadInterface instance and there is no 'content_id' parameter", gettype($name))); } $routes = $content->getRoutes(); diff --git a/src/DynamicRouter.php b/src/DynamicRouter.php index f9fb0b4d..cd6f295e 100644 --- a/src/DynamicRouter.php +++ b/src/DynamicRouter.php @@ -165,10 +165,6 @@ public function getGenerator() */ public function generate($name, $parameters = [], $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH): string { - if (is_object($name)) { - @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT', E_USER_DEPRECATED); - } - if ($this->eventDispatcher) { $event = new RouterGenerateEvent($name, $parameters, $referenceType); $this->eventDispatcher->dispatch($event, Events::PRE_DYNAMIC_GENERATE); diff --git a/src/ProviderBasedGenerator.php b/src/ProviderBasedGenerator.php index ee1761c2..ae77a0c7 100644 --- a/src/ProviderBasedGenerator.php +++ b/src/ProviderBasedGenerator.php @@ -43,23 +43,10 @@ public function __construct(RouteProviderInterface $provider, LoggerInterface $l /** * {@inheritdoc} - * - * The CMF routing system used to allow to pass route objects as $name to generate the route. - * Since Symfony 5.0, the UrlGeneratorInterface declares $name as string. We widen the contract - * for BC but deprecate passing non-strings. - * Instead, Pass the RouteObjectInterface::OBJECT_BASED_ROUTE_NAME as route name and the object - * in the parameters with key RouteObjectInterface::ROUTE_OBJECT. - * - * @param mixed $name */ public function generate($name, $parameters = [], $referenceType = self::ABSOLUTE_PATH): string { - if (is_object($name)) { - @trigger_error('Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`', E_USER_DEPRECATED); - } - if ($name instanceof SymfonyRoute) { - $route = $name; - } elseif (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name + if (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof SymfonyRoute ) { diff --git a/tests/Unit/Candidates/CandidatesTest.php b/tests/Unit/Candidates/CandidatesTest.php index 92e92dfa..c2fba519 100644 --- a/tests/Unit/Candidates/CandidatesTest.php +++ b/tests/Unit/Candidates/CandidatesTest.php @@ -29,6 +29,8 @@ public function testIsCandidate() /** * Nothing should be called on the query builder. + * + * @doesNotPerformAssertions */ public function testRestrictQuery() { diff --git a/tests/Unit/Routing/ChainRouterTest.php b/tests/Unit/Routing/ChainRouterTest.php index ef4bd568..ce091055 100644 --- a/tests/Unit/Routing/ChainRouterTest.php +++ b/tests/Unit/Routing/ChainRouterTest.php @@ -23,7 +23,6 @@ use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Exception\RouteNotFoundException; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; -use Symfony\Component\Routing\Loader\ObjectRouteLoader; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\Route; @@ -608,111 +607,6 @@ public function testGenerateNotFound() $this->router->generate($name, $parameters); } - /** - * Route is an object but no versatile generator around to do the debug message. - * - * @group legacy - * @expectedDeprecation Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. - */ - public function testGenerateObjectNotFound() - { - if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Symfony 5 would throw a TypeError.'); - } - - $name = new \stdClass(); - $parameters = ['test' => 'value']; - - $defaultRouter = $this->createMock(RouterInterface::class); - - $defaultRouter - ->expects($this->never()) - ->method('generate') - ; - - $this->router->add($defaultRouter, 200); - - $this->expectException(RouteNotFoundException::class); - $this->router->generate($name, $parameters); - } - - /** - * A versatile router will generate the debug message. - * - * @group legacy - * @expectedDeprecation Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. - */ - public function testGenerateObjectNotFoundVersatile() - { - if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Symfony 5 would throw a TypeError.'); - } - - $name = new \stdClass(); - $parameters = ['test' => 'value']; - - $chainedRouter = $this->createMock(VersatileRouter::class); - $chainedRouter - ->expects($this->once()) - ->method('supports') - ->will($this->returnValue(true)) - ; - $chainedRouter->expects($this->once()) - ->method('generate') - ->with($name, $parameters, UrlGeneratorInterface::ABSOLUTE_PATH) - ->will($this->throwException(new RouteNotFoundException())) - ; - $chainedRouter->expects($this->once()) - ->method('getRouteDebugMessage') - ->with($name, $parameters) - ->will($this->returnValue('message')) - ; - - $this->router->add($chainedRouter, 10); - - $this->expectException(RouteNotFoundException::class); - $this->router->generate($name, $parameters); - } - - /** - * @group legacy - * @expectedDeprecation Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. - */ - public function testGenerateObjectName() - { - if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Symfony 5 would throw a TypeError.'); - } - - $name = new \stdClass(); - $parameters = ['test' => 'value']; - - $defaultRouter = $this->createMock(RouterInterface::class); - $chainedRouter = $this->createMock(VersatileRouter::class); - - $defaultRouter - ->expects($this->never()) - ->method('generate') - ; - $chainedRouter - ->expects($this->once()) - ->method('supports') - ->will($this->returnValue(true)) - ; - $chainedRouter - ->expects($this->once()) - ->method('generate') - ->with($name, $parameters, UrlGeneratorInterface::ABSOLUTE_PATH) - ->will($this->returnValue($name)) - ; - - $this->router->add($defaultRouter, 200); - $this->router->add($chainedRouter, 100); - - $result = $this->router->generate($name, $parameters); - $this->assertEquals($name, $result); - } - /** * This test currently triggers a deprecation notice because of ChainRouter BC. */ diff --git a/tests/Unit/Routing/ContentAwareGeneratorTest.php b/tests/Unit/Routing/ContentAwareGeneratorTest.php index 3db4e071..3675a0fe 100644 --- a/tests/Unit/Routing/ContentAwareGeneratorTest.php +++ b/tests/Unit/Routing/ContentAwareGeneratorTest.php @@ -21,7 +21,6 @@ use Symfony\Cmf\Component\Routing\Tests\Unit\Routing\RouteMock; use Symfony\Component\Routing\CompiledRoute; use Symfony\Component\Routing\Exception\RouteNotFoundException; -use Symfony\Component\Routing\Loader\ObjectRouteLoader; use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\Route; @@ -38,7 +37,7 @@ class ContentAwareGeneratorTest extends TestCase private $routeDocument; /** - * @var CompiledRoute|MockObject + * @var CompiledRoute */ private $routeCompiled; @@ -65,38 +64,13 @@ public function setUp(): void ->setMethods(['compile', 'getContent']) ->getMock(); - $this->routeCompiled = $this->createMock(CompiledRoute::class); + $this->routeCompiled = new CompiledRoute('', '', [], []); $this->provider = $this->createMock(RouteProviderInterface::class); $this->context = $this->createMock(RequestContext::class); $this->generator = new TestableContentAwareGenerator($this->provider); } - /** - * @group legacy - * @expectedDeprecation Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. - */ - public function testGenerateFromContent(): void - { - if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Symfony 5 would throw a TypeError.'); - } - - $this->provider->expects($this->never()) - ->method('getRouteByName') - ; - $this->contentDocument->expects($this->once()) - ->method('getRoutes') - ->willReturn([$this->routeDocument]) - ; - $this->routeDocument->expects($this->once()) - ->method('compile') - ->willReturn($this->routeCompiled) - ; - - $this->assertEquals('result_url', $this->generator->generate($this->contentDocument)); - } - public function testGenerateFromContentInParameters(): void { $this->provider->expects($this->never()) @@ -233,10 +207,6 @@ public function testGenerateRouteMultilangDefaultLocale() ->with('_locale') ->willReturn('en') ; - $this->routeCompiled->expects($this->any()) - ->method('getVariables') - ->willReturn([]) - ; $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['_locale' => 'en', RouteObjectInterface::ROUTE_OBJECT => $route]); $this->assertEquals('result_url', $generated); @@ -403,21 +373,6 @@ public function testGenerateInvalidContent(): void $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, [RouteObjectInterface::ROUTE_OBJECT => $this]); } - /** - * Generate with an object that is neither a route nor route aware. - * - * @group legacy - */ - public function testGenerateInvalidContentLegacy(): void - { - if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Symfony 5 would throw a TypeError.'); - } - - $this->expectException(RouteNotFoundException::class); - $this->generator->generate($this); - } - /** * Generate with a content_id but there is no content repository. */ diff --git a/tests/Unit/Routing/ProviderBasedGeneratorTest.php b/tests/Unit/Routing/ProviderBasedGeneratorTest.php index 2916944b..effb3c3d 100644 --- a/tests/Unit/Routing/ProviderBasedGeneratorTest.php +++ b/tests/Unit/Routing/ProviderBasedGeneratorTest.php @@ -31,7 +31,7 @@ class ProviderBasedGeneratorTest extends TestCase private $routeDocument; /** - * @var CompiledRoute|MockObject + * @var CompiledRoute */ private $routeCompiled; @@ -53,7 +53,7 @@ class ProviderBasedGeneratorTest extends TestCase public function setUp(): void { $this->routeDocument = $this->createMock(Route::class); - $this->routeCompiled = $this->createMock(CompiledRoute::class); + $this->routeCompiled = new CompiledRoute('', '', [], []); $this->provider = $this->createMock(RouteProviderInterface::class); $this->context = $this->createMock(RequestContext::class); @@ -116,24 +116,6 @@ public function testRemoveRouteObject(): void $this->assertEquals('result_url', $url); } - /** - * @group legacy - * - * @expectedDeprecation Passing an object as route name is deprecated since version 2.3. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT` - */ - public function testGenerateFromRouteLegacy(): void - { - $this->provider->expects($this->never()) - ->method('getRouteByName') - ; - $this->routeDocument->expects($this->once()) - ->method('compile') - ->willReturn($this->routeCompiled) - ; - - $this->assertEquals('result_url', $this->generator->generate($this->routeDocument)); - } - public function testSupports(): void { $this->assertTrue($this->generator->supports('foo/bar')); @@ -180,7 +162,7 @@ public function testGenerateByRoute(): void */ class TestableProviderBasedGenerator extends ProviderBasedGenerator { - protected function doGenerate(array $variables, array $defaults, array $requirements, array $tokens, array $parameters, string $name, int $referenceType, array $hostTokens, array $requiredSchemes = []): string + protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, $requiredSchemes = []): string { $url = 'result_url'; if ($parameters && $query = http_build_query($parameters, '', '&', PHP_QUERY_RFC3986)) { From 89ae391e7765e6a2277158d8aececf0919b3fd28 Mon Sep 17 00:00:00 2001 From: Jeroen Thora Date: Mon, 20 Dec 2021 22:00:16 +0100 Subject: [PATCH 3/3] Add generator param type check --- src/ChainRouter.php | 4 ++++ src/ContentAwareGenerator.php | 4 ++++ src/DynamicRouter.php | 4 ++++ src/ProviderBasedGenerator.php | 4 ++++ tests/Unit/Routing/ChainRouterTest.php | 17 +++++++++++++++++ .../Unit/Routing/ContentAwareGeneratorTest.php | 7 +++++++ .../Unit/Routing/ProviderBasedGeneratorTest.php | 7 +++++++ 7 files changed, 47 insertions(+) diff --git a/src/ChainRouter.php b/src/ChainRouter.php index ee608b45..1f9218ed 100644 --- a/src/ChainRouter.php +++ b/src/ChainRouter.php @@ -217,6 +217,10 @@ private function doMatch($pathinfo, Request $request = null) */ public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH): string { + if (!is_string($name)) { + throw new \InvalidArgumentException('The "$name" parameter should of type string.'); + } + $debug = []; foreach ($this->all() as $router) { diff --git a/src/ContentAwareGenerator.php b/src/ContentAwareGenerator.php index 5242a9f1..4e79e7ff 100644 --- a/src/ContentAwareGenerator.php +++ b/src/ContentAwareGenerator.php @@ -65,6 +65,10 @@ public function setContentRepository(ContentRepositoryInterface $contentReposito */ public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH): string { + if (!is_string($name)) { + throw new \InvalidArgumentException('The "$name" parameter should of type string.'); + } + if (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name) { if (array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof SymfonyRoute diff --git a/src/DynamicRouter.php b/src/DynamicRouter.php index cd6f295e..893401b9 100644 --- a/src/DynamicRouter.php +++ b/src/DynamicRouter.php @@ -165,6 +165,10 @@ public function getGenerator() */ public function generate($name, $parameters = [], $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH): string { + if (!is_string($name)) { + throw new \InvalidArgumentException('The "$name" parameter should of type string.'); + } + if ($this->eventDispatcher) { $event = new RouterGenerateEvent($name, $parameters, $referenceType); $this->eventDispatcher->dispatch($event, Events::PRE_DYNAMIC_GENERATE); diff --git a/src/ProviderBasedGenerator.php b/src/ProviderBasedGenerator.php index ae77a0c7..409f9479 100644 --- a/src/ProviderBasedGenerator.php +++ b/src/ProviderBasedGenerator.php @@ -46,6 +46,10 @@ public function __construct(RouteProviderInterface $provider, LoggerInterface $l */ public function generate($name, $parameters = [], $referenceType = self::ABSOLUTE_PATH): string { + if (!is_string($name)) { + throw new \InvalidArgumentException('The "$name" parameter should of type string.'); + } + if (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof SymfonyRoute diff --git a/tests/Unit/Routing/ChainRouterTest.php b/tests/Unit/Routing/ChainRouterTest.php index ce091055..e783032b 100644 --- a/tests/Unit/Routing/ChainRouterTest.php +++ b/tests/Unit/Routing/ChainRouterTest.php @@ -709,6 +709,23 @@ public function testRouteCollection() $this->assertEquals(['high', 'low'], $names); } + public function testGenerateWithNameParameterObject() + { + $this->expectException(\InvalidArgumentException::class); + + $parameters = ['test' => 'value']; + $defaultRouter = $this->createMock(RouterInterface::class); + + $defaultRouter + ->expects($this->never()) + ->method('generate') + ; + + $this->router->add($defaultRouter, 200); + + $this->router->generate(new \stdClass(), $parameters); + } + /** * @group legacy */ diff --git a/tests/Unit/Routing/ContentAwareGeneratorTest.php b/tests/Unit/Routing/ContentAwareGeneratorTest.php index 3675a0fe..b2107fae 100644 --- a/tests/Unit/Routing/ContentAwareGeneratorTest.php +++ b/tests/Unit/Routing/ContentAwareGeneratorTest.php @@ -507,6 +507,13 @@ public function testGetRouteDebugMessageLegacy(): void $this->assertStringContainsString('Route aware content Symfony\Cmf\Component\Routing\Tests\Routing\RouteAware', $this->generator->getRouteDebugMessage(new RouteAware())); $this->assertStringContainsString('/some/content', $this->generator->getRouteDebugMessage('/some/content')); } + + public function testGenerateWithNameParameterObject(): void + { + $this->expectException(\InvalidArgumentException::class); + + $this->generator->generate(new \stdClass()); + } } /** diff --git a/tests/Unit/Routing/ProviderBasedGeneratorTest.php b/tests/Unit/Routing/ProviderBasedGeneratorTest.php index effb3c3d..fbaa0ac7 100644 --- a/tests/Unit/Routing/ProviderBasedGeneratorTest.php +++ b/tests/Unit/Routing/ProviderBasedGeneratorTest.php @@ -155,6 +155,13 @@ public function testGenerateByRoute(): void 'number' => 'string', ]); } + + public function testGenerateWithNameParameterObject(): void + { + $this->expectException(\InvalidArgumentException::class); + + $this->generator->generate(new \stdClass()); + } } /**