Merge pull request #15 from timdev/topic/3-psalm-integration

Psalm Integration
mezzio · Sep 14, 2022 · 5133c59 · 5133c59
2 parents db8f6f6 + 4100b64
commit 5133c59
Show file tree

Hide file tree

Showing 10 changed files with 2,904 additions and 884 deletions.
diff --git a/composer.json b/composer.json
@@ -41,7 +41,9 @@
     },
     "require-dev": {
         "laminas/laminas-coding-standard": "~2.4.0",
-        "phpunit/phpunit": "^9.5.24"
+        "phpunit/phpunit": "^9.5.24",
+        "psalm/plugin-phpunit": "^0.16.1",
+        "vimeo/psalm": "^4.9"
     },
     "autoload": {
         "psr-4": {
@@ -61,7 +63,8 @@
         "cs-check": "phpcs",
         "cs-fix": "phpcbf",
         "test": "phpunit --colors=always",
-        "test-coverage": "phpunit --colors=always --coverage-clover clover.xml"
+        "test-coverage": "phpunit --colors=always --coverage-clover clover.xml",
+        "static-analysis": "psalm --shepherd --stats"
     },
     "conflict": {
         "zendframework/zend-expressive-flash": "*"

diff --git a/composer.lock b/composer.lock
diff --git a/psalm.xml b/psalm.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0"?>
+<psalm
+        errorLevel="1"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xmlns="https://getpsalm.org/schema/config"
+        xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
+>
+    <projectFiles>
+        <directory name="src"/>
+        <directory name="test"/>
+        <ignoreFiles>
+            <directory name="vendor"/>
+        </ignoreFiles>
+    </projectFiles>
+
+    <plugins>
+        <pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
+    </plugins>
+</psalm>
diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php
@@ -18,7 +18,8 @@ public function getDependencies(): array
         return [
             // Legacy Zend Framework aliases
             'aliases'    => [
-                \Zend\Expressive\Flash\FlashMessageMiddleware::class => FlashMessageMiddleware::class,
+                // phpcs:ignore
+                'Zend\Expressive\Flash\FlashMessageMiddleware' => FlashMessageMiddleware::class,
             ],
             'invokables' => [
                 FlashMessageMiddleware::class => FlashMessageMiddleware::class,

diff --git a/src/FlashMessageMiddleware.php b/src/FlashMessageMiddleware.php
@@ -11,9 +11,7 @@
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 
-use function class_exists;
-use function class_implements;
-use function in_array;
+use function is_callable;
 
 class FlashMessageMiddleware implements MiddlewareInterface
 {
@@ -22,7 +20,7 @@ class FlashMessageMiddleware implements MiddlewareInterface
     /** @var string */
     private $attributeKey;
 
-    /** @var callable */
+    /** @psalm-var callable(SessionInterface, string): FlashMessagesInterface */
     private $flashMessageFactory;
 
     /** @var string */
@@ -33,14 +31,12 @@ public function __construct(
         string $sessionKey = FlashMessagesInterface::FLASH_NEXT,
         string $attributeKey = self::FLASH_ATTRIBUTE
     ) {
-        if (
-            ! class_exists($flashMessagesClass)
-            || ! in_array(FlashMessagesInterface::class, class_implements($flashMessagesClass), true)
-        ) {
+        $factory = [$flashMessagesClass, 'createFromSession'];
+        if (! is_callable($factory)) {
             throw Exception\InvalidFlashMessagesImplementationException::forClass($flashMessagesClass);
         }
 
-        $this->flashMessageFactory = [$flashMessagesClass, 'createFromSession'];
+        $this->flashMessageFactory = $factory;
         $this->sessionKey          = $sessionKey;
         $this->attributeKey        = $attributeKey;
     }

diff --git a/src/FlashMessages.php b/src/FlashMessages.php
@@ -6,8 +6,6 @@
 
 use Mezzio\Session\SessionInterface;
 
-use function is_array;
-
 /**
  * Create, retrieve, and manipulate flash messages.
  *
@@ -26,10 +24,12 @@
  *
  * In order to keep messages made available to the current request for another
  * hop, use the `prolongFlash()` method.
+ *
+ * @psalm-type StoredMessages = array<string,array{value:mixed,hops:int}>
  */
 class FlashMessages implements FlashMessagesInterface
 {
-    /** @var array */
+    /** @var array<string,mixed> */
     private $currentMessages = [];
 
     /** @var SessionInterface */
@@ -71,7 +71,7 @@ public function flash(string $key, $value, int $hops = 1): void
             throw Exception\InvalidHopsValueException::valueTooLow($key, $hops);
         }
 
-        $messages       = $this->session->get($this->sessionKey, []);
+        $messages       = $this->getStoredMessages();
         $messages[$key] = [
             'value' => $value,
             'hops'  => $hops,
@@ -145,7 +145,9 @@ public function clearFlash(): void
      */
     public function prolongFlash(): void
     {
-        $messages = $this->session->get($this->sessionKey, []);
+        $messages = $this->getStoredMessages();
+
+        /** @var mixed $value */
         foreach ($this->currentMessages as $key => $value) {
             if (isset($messages[$key])) {
                 continue;
@@ -161,11 +163,17 @@ public function prepareMessages(SessionInterface $session, string $sessionKey):
             return;
         }
 
-        $sessionMessages = $session->get($sessionKey);
-        $sessionMessages = ! is_array($sessionMessages) ? [] : $sessionMessages;
+        $sessionMessages = $this->getStoredMessages($sessionKey);
 
+        /** @var array<string,mixed> $currentMessages */
         $currentMessages = [];
         foreach ($sessionMessages as $key => $data) {
+            /**
+             * The public API of flash() and flashNow() explicitly allow calling
+             * code to pass `$value`s of any type, making this unavoidable.
+             *
+             * @psalm-suppress MixedAssignment
+             */
             $currentMessages[$key] = $data['value'];
 
             if ($data['hops'] === 1) {
@@ -183,4 +191,14 @@ public function prepareMessages(SessionInterface $session, string $sessionKey):
 
         $this->currentMessages = $currentMessages;
     }
+
+    /**
+     * @return StoredMessages
+     */
+    private function getStoredMessages(?string $sessionKey = null): array
+    {
+        /** @var StoredMessages|null $messages */
+        $messages = $this->session->get($sessionKey ?? $this->sessionKey, []);
+        return $messages ?? [];
+    }
 }
diff --git a/test/ConfigProviderTest.php b/test/ConfigProviderTest.php
@@ -17,18 +17,9 @@ public function setUp(): void
         $this->provider = new ConfigProvider();
     }
 
-    public function testInvocationReturnsArray(): array
+    public function testReturnedArrayContainsDependencies(): void
     {
         $config = ($this->provider)();
-        $this->assertIsArray($config);
-        return $config;
-    }
-
-    /**
-     * @depends testInvocationReturnsArray
-     */
-    public function testReturnedArrayContainsDependencies(array $config)
-    {
         $this->assertArrayHasKey('dependencies', $config);
         $this->assertIsArray($config['dependencies']);
     }

diff --git a/test/FlashMessageMiddlewareTest.php b/test/FlashMessageMiddlewareTest.php
@@ -17,21 +17,21 @@
 
 class FlashMessageMiddlewareTest extends TestCase
 {
-    public function testConstructorRaisesExceptionIfFlashMessagesClassIsNotAClass()
+    public function testConstructorRaisesExceptionIfFlashMessagesClassIsNotAClass(): void
     {
         $this->expectException(Exception\InvalidFlashMessagesImplementationException::class);
         $this->expectExceptionMessage('not-a-class');
         new FlashMessageMiddleware('not-a-class');
     }
 
-    public function testConstructorRaisesExceptionIfFlashMessagesClassDoesNotImplementCorrectInterface()
+    public function testConstructorRaisesExceptionIfFlashMessagesClassDoesNotImplementCorrectInterface(): void
     {
         $this->expectException(Exception\InvalidFlashMessagesImplementationException::class);
         $this->expectExceptionMessage('stdClass');
         new FlashMessageMiddleware(stdClass::class);
     }
 
-    public function testProcessRaisesExceptionIfRequestSessionAttributeDoesNotReturnSessionInterface()
+    public function testProcessRaisesExceptionIfRequestSessionAttributeDoesNotReturnSessionInterface(): void
     {
         $request = $this->createMock(ServerRequestInterface::class);
         $request
@@ -61,8 +61,10 @@ public function testProcessRaisesExceptionIfRequestSessionAttributeDoesNotReturn
         $middleware->process($request, $handler);
     }
 
-    public function testProcessUsesConfiguredClassAndSessionKeyAndAttributeKeyToCreateFlashMessagesAndPassToHandler()
+    // @codingStandardsIgnoreStart
+    public function testProcessUsesConfiguredClassAndSessionKeyAndAttributeKeyToCreateFlashMessagesAndPassToHandler(): void
     {
+        // @codingStandardsIgnoreEnd
         $session = $this->createMock(SessionInterface::class);
 
         $request = $this->createMock(ServerRequestInterface::class);

diff --git a/test/FlashMessagesTest.php b/test/FlashMessagesTest.php
@@ -13,15 +13,15 @@
 
 class FlashMessagesTest extends TestCase
 {
-    /** @var SessionInterface|MockObject */
+    /** @var SessionInterface&MockObject */
     private $session;
 
     public function setUp(): void
     {
         $this->session = $this->createMock(SessionInterface::class);
     }
 
-    public function testCreationAggregatesNothingIfNoMessagesExistUnderSpecifiedSessionKey()
+    public function testCreationAggregatesNothingIfNoMessagesExistUnderSpecifiedSessionKey(): void
     {
         $this->session
             ->expects($this->once())
@@ -38,7 +38,7 @@ public function testCreationAggregatesNothingIfNoMessagesExistUnderSpecifiedSess
         $this->assertSame([], $flash->getFlashes());
     }
 
-    public function testCreationAggregatesItemsMarkedNextAndRemovesThemFromSession()
+    public function testCreationAggregatesItemsMarkedNextAndRemovesThemFromSession(): void
     {
         $messages = [
             'test'   => [
@@ -74,7 +74,7 @@ public function testCreationAggregatesItemsMarkedNextAndRemovesThemFromSession()
         $this->assertSame(['test' => 'value1', 'test-2' => 'value2'], $flash->getFlashes());
     }
 
-    public function testCreationAggregatesPersistsItemsWithMultipleHopsInSessionWithDecrementedHops()
+    public function testCreationAggregatesPersistsItemsWithMultipleHopsInSessionWithDecrementedHops(): void
     {
         $messages                           = [
             'test'   => [
@@ -116,7 +116,7 @@ public function testCreationAggregatesPersistsItemsWithMultipleHopsInSessionWith
         $this->assertSame(['test' => 'value1', 'test-2' => 'value2'], $flash->getFlashes());
     }
 
-    public function testFlashingAValueMakesItAvailableInNextSessionButNotFlashMessages()
+    public function testFlashingAValueMakesItAvailableInNextSessionButNotFlashMessages(): void
     {
         $this->session
             ->expects($this->once())
@@ -148,7 +148,7 @@ public function testFlashingAValueMakesItAvailableInNextSessionButNotFlashMessag
         $this->assertSame([], $flash->getFlashes());
     }
 
-    public function testFlashNowMakesValueAvailableBothInNextSessionAndCurrentFlashMessages()
+    public function testFlashNowMakesValueAvailableBothInNextSessionAndCurrentFlashMessages(): void
     {
         $this->session
             ->expects($this->once())
@@ -180,7 +180,7 @@ public function testFlashNowMakesValueAvailableBothInNextSessionAndCurrentFlashM
         $this->assertSame(['test' => 'value'], $flash->getFlashes());
     }
 
-    public function testProlongFlashAddsCurrentMessagesToNextSession()
+    public function testProlongFlashAddsCurrentMessagesToNextSession(): void
     {
         $messages = [
             'test'   => [
@@ -243,7 +243,7 @@ public function testProlongFlashAddsCurrentMessagesToNextSession()
         $flash->prolongFlash();
     }
 
-    public function testProlongFlashDoesNotReFlashMessagesThatAlreadyHaveMoreHops()
+    public function testProlongFlashDoesNotReFlashMessagesThatAlreadyHaveMoreHops(): void
     {
         $messages                           = [
             'test'   => [
@@ -290,7 +290,7 @@ public function testProlongFlashDoesNotReFlashMessagesThatAlreadyHaveMoreHops()
         $flash->prolongFlash();
     }
 
-    public function testClearFlashShouldRemoveAnyUnexpiredMessages()
+    public function testClearFlashShouldRemoveAnyUnexpiredMessages(): void
     {
         $messages                           = [
             'test'   => [
@@ -337,7 +337,7 @@ public function testClearFlashShouldRemoveAnyUnexpiredMessages()
         $flash->clearFlash();
     }
 
-    public function testCreationAggregatesThrowsExceptionIfInvalidNumberOfHops()
+    public function testCreationAggregatesThrowsExceptionIfInvalidNumberOfHops(): void
     {
         $this->expectException(InvalidHopsValueException::class);
 
@@ -363,15 +363,15 @@ public function testCreationAggregatesThrowsExceptionIfInvalidNumberOfHops()
         $flash->flash('test', 'value', 0);
     }
 
-    public function testFlashNowAcceptsZeroHops()
+    public function testFlashNowAcceptsZeroHops(): void
     {
         $flash = FlashMessages::createFromSession($this->session);
         $flash->flashNow('test', 'value', 0);
 
         $this->assertSame('value', $flash->getFlash('test'));
     }
 
-    public function testFlashNowWithZeroHopsShouldNotSetValueToSession()
+    public function testFlashNowWithZeroHopsShouldNotSetValueToSession(): void
     {
         $this->session
             ->expects($this->never())

diff --git a/test/TestAsset/FlashMessages.php b/test/TestAsset/FlashMessages.php
@@ -42,7 +42,7 @@ public function flashNow(string $key, $value, int $hops = 1): void
     }
 
     /**
-     * @param null $default
+     * @param mixed|null $default
      * @return mixed|void
      */
     public function getFlash(string $key, $default = null)