Merge pull request #31 from mezzio/psalm-hack

Improve type inference, add missing type annotations
mezzio · Jan 11, 2023 · e165b3d · e165b3d
2 parents 482bbd5 + 6388728
commit e165b3d
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 100 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.0.0@4e177bf0c9f03c17d2fbfd83b7cc9c47605274d8">
+<files psalm-version="5.4.0@62db5d4f6a7ae0a20f7cc5a4952d730272fc0863">
   <file src="src/BodyParams/JsonStrategy.php">
     <MixedAssignment occurrences="1">
       <code>$parsedBody</code>
@@ -10,84 +10,20 @@
       <code>(string) $specification</code>
     </RedundantCast>
   </file>
-  <file src="src/ServerUrlMiddlewareFactory.php">
-    <MixedArgument occurrences="1">
-      <code>$container-&gt;get(ServerUrlHelper::class)</code>
-    </MixedArgument>
-  </file>
-  <file src="src/Template/RouteTemplateVariableMiddleware.php">
-    <MixedAssignment occurrences="2">
-      <code>$container</code>
-      <code>$routeResult</code>
-    </MixedAssignment>
-    <MixedMethodCall occurrences="1">
-      <code>with</code>
-    </MixedMethodCall>
-  </file>
-  <file src="src/Template/TemplateVariableContainer.php">
-    <MixedPropertyTypeCoercion occurrences="1">
-      <code>array_merge($this-&gt;variables, $values)</code>
-    </MixedPropertyTypeCoercion>
-  </file>
   <file src="src/Template/TemplateVariableContainerMiddleware.php">
     <MixedAssignment occurrences="1">
       <code>$container</code>
     </MixedAssignment>
   </file>
   <file src="src/UrlHelper.php">
-    <MixedArgument occurrences="2">
-      <code>$routerOptions</code>
-      <code>$routerOptions</code>
-    </MixedArgument>
-    <MixedAssignment occurrences="1">
-      <code>$routerOptions</code>
-    </MixedAssignment>
-    <PossiblyFalseArgument occurrences="1">
-      <code>$name</code>
-    </PossiblyFalseArgument>
-    <PossiblyNullReference occurrences="1">
-      <code>getQueryParams</code>
-    </PossiblyNullReference>
-  </file>
-  <file src="src/UrlHelperFactory.php">
-    <MixedArgument occurrences="3">
-      <code>$container-&gt;get($this-&gt;routerServiceName)</code>
-      <code>$data['basePath'] ?? '/'</code>
-      <code>$data['routerServiceName'] ?? RouterInterface::class</code>
-    </MixedArgument>
+    <RedundantCastGivenDocblockType occurrences="2">
+      <code>(bool) $options['reuse_query_params']</code>
+      <code>(bool) $options['reuse_result_params']</code>
+    </RedundantCastGivenDocblockType>
   </file>
   <file src="src/UrlHelperMiddleware.php">
     <MixedAssignment occurrences="1">
       <code>$result</code>
     </MixedAssignment>
   </file>
-  <file src="src/UrlHelperMiddlewareFactory.php">
-    <MixedArgument occurrences="2">
-      <code>$container-&gt;get($this-&gt;urlHelperServiceName)</code>
-      <code>$data['urlHelperServiceName'] ?? UrlHelper::class</code>
-    </MixedArgument>
-  </file>
-  <file src="test/ConfigProviderTest.php">
-    <RedundantCondition occurrences="1">
-      <code>assertIsArray</code>
-    </RedundantCondition>
-  </file>
-  <file src="test/ExceptionTest.php">
-    <MixedInferredReturnType occurrences="1">
-      <code>Generator</code>
-    </MixedInferredReturnType>
-    <PossiblyFalseOperand occurrences="1">
-      <code>strrpos(ExceptionInterface::class, '\\')</code>
-    </PossiblyFalseOperand>
-  </file>
-  <file src="test/ServerUrlHelperTest.php">
-    <DocblockTypeContradiction occurrences="1">
-      <code>assertTrue</code>
-    </DocblockTypeContradiction>
-  </file>
-  <file src="test/UrlHelperTest.php">
-    <DocblockTypeContradiction occurrences="1">
-      <code>assertTrue</code>
-    </DocblockTypeContradiction>
-  </file>
 </files>
diff --git 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
@@ -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
@@ -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<string, mixed> $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<string, mixed> $values
+     * @return array<string, mixed>
      */
     public function mergeForTemplate(array $values): array
     {

diff --git 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<array-key, mixed>,
+ *     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<string, mixed> $routeParams
+     * @param array<string, mixed> $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<string, mixed> $routeParams
+     * @param array<string, mixed> $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
@@ -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
@@ -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
@@ -18,29 +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())();
 
-        self::assertIsArray($config);
-
-        return $config;
-    }
-
-    /**
-     * @depends testInvocationReturnsArray
-     */
-    public function testReturnedArrayContainsDependencies(array $config): void
-    {
         self::assertSame([
             'dependencies' => [
                 'invokables' => [

diff --git 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<string, array{0: string}> */
+    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
@@ -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<string, array{0: array, 1: string|null, 2: string}> */
+    /** @return array<string, array{0: array<string, mixed>, 1: string|null, 2: string}> */
     public static function queryParametersAndFragmentProvider(): array
     {
         return [
@@ -484,6 +484,7 @@ public static function queryParametersAndFragmentProvider(): array
 
     /**
      * @dataProvider queryParametersAndFragmentProvider
+     * @param array<string, mixed> $queryParams
      */
     public function testQueryParametersAndFragment(
         array $queryParams,