From 3a12ee47e1ecf10d3bc4675d76b44ad1839cf644 Mon Sep 17 00:00:00 2001 From: George Steel Date: Tue, 10 Jan 2023 22:02:55 +0000 Subject: [PATCH 1/2] Improve type inference, add missing type annotations Reduces the psalm baseline to a handful of `MixedAssignment` and `RedundantCast` issues. Type coverage is now around 99.8% Signed-off-by: George Steel --- psalm-baseline.xml | 74 ++----------------- src/ServerUrlMiddlewareFactory.php | 6 +- .../RouteTemplateVariableMiddleware.php | 5 ++ src/Template/TemplateVariableContainer.php | 4 + src/UrlHelper.php | 22 +++++- src/UrlHelperFactory.php | 7 +- src/UrlHelperMiddlewareFactory.php | 8 +- test/ConfigProviderTest.php | 1 + test/ExceptionTest.php | 9 ++- test/UrlHelperTest.php | 5 +- 10 files changed, 62 insertions(+), 79 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8a3c244..77d12d7 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + $parsedBody @@ -10,84 +10,20 @@ (string) $specification - - - $container->get(ServerUrlHelper::class) - - - - - $container - $routeResult - - - with - - - - - array_merge($this->variables, $values) - - $container - - $routerOptions - $routerOptions - - - $routerOptions - - - $name - - - getQueryParams - - - - - $container->get($this->routerServiceName) - $data['basePath'] ?? '/' - $data['routerServiceName'] ?? RouterInterface::class - + + (bool) $options['reuse_query_params'] + (bool) $options['reuse_result_params'] + $result - - - $container->get($this->urlHelperServiceName) - $data['urlHelperServiceName'] ?? UrlHelper::class - - - - - assertIsArray - - - - - Generator - - - strrpos(ExceptionInterface::class, '\\') - - - - - assertTrue - - - - - assertTrue - - diff --git a/src/ServerUrlMiddlewareFactory.php b/src/ServerUrlMiddlewareFactory.php index 80aede4..bb269bb 100644 --- a/src/ServerUrlMiddlewareFactory.php +++ b/src/ServerUrlMiddlewareFactory.php @@ -6,6 +6,7 @@ use Psr\Container\ContainerInterface; +use function assert; use function sprintf; class ServerUrlMiddlewareFactory @@ -25,6 +26,9 @@ public function __invoke(ContainerInterface $container): ServerUrlMiddleware )); } - return new ServerUrlMiddleware($container->get(ServerUrlHelper::class)); + $helper = $container->get(ServerUrlHelper::class); + assert($helper instanceof ServerUrlHelper); + + return new ServerUrlMiddleware($helper); } } diff --git a/src/Template/RouteTemplateVariableMiddleware.php b/src/Template/RouteTemplateVariableMiddleware.php index 7da0bfc..66ba7b8 100644 --- a/src/Template/RouteTemplateVariableMiddleware.php +++ b/src/Template/RouteTemplateVariableMiddleware.php @@ -10,6 +10,8 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use function assert; + /** * Inject the currently matched route into the template variable container. * @@ -36,7 +38,10 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface new TemplateVariableContainer() ); + assert($container instanceof TemplateVariableContainer); + $routeResult = $request->getAttribute(RouteResult::class, null); + assert($routeResult instanceof RouteResult || $routeResult === null); return $handler->handle($request->withAttribute( TemplateVariableContainer::class, diff --git a/src/Template/TemplateVariableContainer.php b/src/Template/TemplateVariableContainer.php index 363f385..f9d58cb 100644 --- a/src/Template/TemplateVariableContainer.php +++ b/src/Template/TemplateVariableContainer.php @@ -121,6 +121,7 @@ public function without(string $key): self * This method will overwrite any existing value with the same key with the * new value if it occurs in $values. * + * @param array $values * @return self Returns a new instance with the merged values. */ public function merge(array $values): self @@ -136,6 +137,9 @@ public function merge(array $values): self * Use this method to merge handler-specific template values with those in * the container in order to pass the result to the renderer's `render()` * method. + * + * @param array $values + * @return array */ public function mergeForTemplate(array $values): array { diff --git a/src/UrlHelper.php b/src/UrlHelper.php index 5e307ca..d5f6073 100644 --- a/src/UrlHelper.php +++ b/src/UrlHelper.php @@ -10,12 +10,21 @@ use Psr\Http\Message\ServerRequestInterface; use function array_merge; +use function assert; use function count; use function http_build_query; +use function is_string; use function ltrim; use function preg_match; use function sprintf; +/** + * @psalm-type UrlGeneratorOptions = array{ + * router?: array, + * reuse_result_params?: bool, + * reuse_query_params?: bool, + * } + */ class UrlHelper { /** @@ -38,7 +47,9 @@ public function __construct(private RouterInterface $router) /** * Generate a URL based on a given route. * - * @param array $options Can have the following keys: + * @param array $routeParams + * @param array $queryParams + * @param UrlGeneratorOptions $options Can have the following keys: * - router (array): contains options to be passed to the router * - reuse_result_params (bool): indicates if the current RouteResult * parameters will be used, defaults to true @@ -107,6 +118,10 @@ public function __invoke( * Proxies to __invoke(). * * @see UrlHelper::__invoke() + * + * @param array $routeParams + * @param array $queryParams + * @param UrlGeneratorOptions $options */ public function generate( ?string $routeName = null, @@ -174,7 +189,8 @@ private function generateUriFromResult(array $params, RouteResult $result, array ); } - $name = $result->getMatchedRouteName(); + $name = $result->getMatchedRouteName(); + assert(is_string($name)); // Cannot be false if the result is not a failure $params = array_merge($result->getMatchedParams(), $params); return $this->router->generateUri($name, $params, $routerOptions); } @@ -234,7 +250,7 @@ private function mergeQueryParams(string $route, RouteResult $result, array $par return $params; } - return array_merge($this->getRequest()->getQueryParams(), $params); + return array_merge($this->getRequest()?->getQueryParams() ?? [], $params); } /** diff --git a/src/UrlHelperFactory.php b/src/UrlHelperFactory.php index 8d9ff55..da3ffb7 100644 --- a/src/UrlHelperFactory.php +++ b/src/UrlHelperFactory.php @@ -7,12 +7,15 @@ use Mezzio\Router\RouterInterface; use Psr\Container\ContainerInterface; +use function assert; use function sprintf; class UrlHelperFactory { /** * Allow serialization + * + * @param array{basePath?: string, routerServiceName?: string} $data */ public static function __set_state(array $data): self { @@ -50,7 +53,9 @@ public function __invoke(ContainerInterface $container): UrlHelper )); } - $helper = new UrlHelper($container->get($this->routerServiceName)); + $router = $container->get($this->routerServiceName); + assert($router instanceof RouterInterface); + $helper = new UrlHelper($router); $helper->setBasePath($this->basePath); return $helper; } diff --git a/src/UrlHelperMiddlewareFactory.php b/src/UrlHelperMiddlewareFactory.php index 2e96cb2..a4505b4 100644 --- a/src/UrlHelperMiddlewareFactory.php +++ b/src/UrlHelperMiddlewareFactory.php @@ -6,12 +6,15 @@ use Psr\Container\ContainerInterface; +use function assert; use function sprintf; class UrlHelperMiddlewareFactory { /** * Allow serialization + * + * @param array{urlHelperServiceName?: string} $data */ public static function __set_state(array $data): self { @@ -42,6 +45,9 @@ public function __invoke(ContainerInterface $container): UrlHelperMiddleware )); } - return new UrlHelperMiddleware($container->get($this->urlHelperServiceName)); + $helper = $container->get($this->urlHelperServiceName); + assert($helper instanceof UrlHelper); + + return new UrlHelperMiddleware($helper); } } diff --git a/test/ConfigProviderTest.php b/test/ConfigProviderTest.php index f016dc8..da2bf68 100644 --- a/test/ConfigProviderTest.php +++ b/test/ConfigProviderTest.php @@ -31,6 +31,7 @@ public function testInvocationReturnsArray(): array { $config = ($this->provider)(); + /** @psalm-suppress RedundantCondition */ self::assertIsArray($config); return $config; diff --git a/test/ExceptionTest.php b/test/ExceptionTest.php index a24a046..9a16dd8 100644 --- a/test/ExceptionTest.php +++ b/test/ExceptionTest.php @@ -8,17 +8,22 @@ use Mezzio\Helper\Exception\ExceptionInterface; use PHPUnit\Framework\TestCase; +use function assert; use function basename; use function glob; use function is_a; +use function is_int; use function strrpos; use function substr; final class ExceptionTest extends TestCase { - public function exception(): Generator + /** @return Generator */ + public static function exception(): Generator { - $namespace = substr(ExceptionInterface::class, 0, strrpos(ExceptionInterface::class, '\\') + 1); + $endPos = strrpos(ExceptionInterface::class, '\\'); + assert(is_int($endPos)); + $namespace = substr(ExceptionInterface::class, 0, $endPos + 1); $exceptions = glob(__DIR__ . '/../src/Exception/*.php'); foreach ($exceptions as $exception) { diff --git a/test/UrlHelperTest.php b/test/UrlHelperTest.php index a95b790..101f7e7 100644 --- a/test/UrlHelperTest.php +++ b/test/UrlHelperTest.php @@ -362,7 +362,7 @@ public function testBasePathIsPrependedToGeneratedPathWhenUsingRouteResult(): vo public function testGenerateProxiesToInvokeMethod(): void { $routeName = 'foo'; - $routeParams = ['bar']; + $routeParams = ['route' => 'bar']; $queryParams = ['foo' => 'bar']; $fragmentIdentifier = 'foobar'; $options = ['router' => ['foobar' => 'baz'], 'reuse_result_params' => false]; @@ -471,7 +471,7 @@ public function testOptionsArePassedToRouter(): void self::assertSame('URL', $helper('foo', [], [], null, ['router' => ['bar' => 'baz']])); } - /** @return array */ + /** @return array, 1: string|null, 2: string}> */ public static function queryParametersAndFragmentProvider(): array { return [ @@ -484,6 +484,7 @@ public static function queryParametersAndFragmentProvider(): array /** * @dataProvider queryParametersAndFragmentProvider + * @param array $queryParams */ public function testQueryParametersAndFragment( array $queryParams, From 6388728675824f1769a4e0f8a14fca89f5f7fb57 Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 11 Jan 2023 08:21:31 +0000 Subject: [PATCH 2/2] Remove redundant test Signed-off-by: George Steel --- test/ConfigProviderTest.php | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/test/ConfigProviderTest.php b/test/ConfigProviderTest.php index da2bf68..fd0da2c 100644 --- a/test/ConfigProviderTest.php +++ b/test/ConfigProviderTest.php @@ -18,30 +18,10 @@ /** @covers \Mezzio\Helper\ConfigProvider */ final class ConfigProviderTest extends TestCase { - private ConfigProvider $provider; - - protected function setUp(): void - { - parent::setUp(); - - $this->provider = new ConfigProvider(); - } - - public function testInvocationReturnsArray(): array + public function testReturnedArrayContainsDependencies(): void { - $config = ($this->provider)(); + $config = (new ConfigProvider())(); - /** @psalm-suppress RedundantCondition */ - self::assertIsArray($config); - - return $config; - } - - /** - * @depends testInvocationReturnsArray - */ - public function testReturnedArrayContainsDependencies(array $config): void - { self::assertSame([ 'dependencies' => [ 'invokables' => [