Merge pull request #140 from clue-labs/phpstan

Add PHPStan to test environment with `max` level

Author: SimonFrings

.gitattributes

@@ -2,6 +2,7 @@
 /.github/workflows/ export-ignore
 /.gitignore export-ignore
 /examples/ export-ignore
+/phpstan.neon.dist export-ignore
 /phpunit.xml.dist export-ignore
 /phpunit.xml.legacy export-ignore
 /tests/ export-ignore

.github/workflows/ci.yml

@@ -37,3 +37,25 @@ jobs:
           <?php
           $metrics = simplexml_load_file('clover.xml')->project->metrics;
           exit((int) $metrics['statements'] === (int) $metrics['coveredstatements'] ? 0 : 1);
+
+  PHPStan:
+    name: PHPStan (PHP ${{ matrix.php }})
+    runs-on: ubuntu-22.04
+    strategy:
+      matrix:
+        php:
+          - 8.2
+          - 8.1
+          - 8.0
+          - 7.4
+          - 7.3
+          - 7.2
+          - 7.1
+    steps:
+      - uses: actions/checkout@v3
+      - uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php }}
+          coverage: none
+      - run: composer install
+      - run: vendor/bin/phpstan

composer.json

@@ -21,6 +21,7 @@
     },
     "require-dev": {
         "clue/block-react": "^1.5",
+        "phpstan/phpstan": "1.8.11 || 1.4.10",
         "phpunit/phpunit": "^9.5 || ^7.5"
     },
     "autoload": {

examples/cli.php

@@ -25,7 +25,9 @@
 
     $params = explode(' ', $line);
     $method = array_shift($params);
-    $promise = call_user_func_array([$redis, $method], $params);
+
+    assert(is_callable([$redis, $method]));
+    $promise = $redis->$method(...$params);
 
     // special method such as end() / close() called
     if (!$promise instanceof React\Promise\PromiseInterface) {

phpstan.neon.dist

@@ -0,0 +1,17 @@
+parameters:
+    level: max
+
+    paths:
+        - examples/
+        - src/
+        - tests/
+
+    reportUnmatchedIgnoredErrors: false
+    ignoreErrors:
+        # ignore generic usage like `PromiseInterface<T>` until fixed upstream
+        - '/^PHPDoc tag @return contains generic type React\\Promise\\PromiseInterface<.+> but interface React\\Promise\\PromiseInterface is not generic\.$/'
+        # ignore undefined methods due to magic `__call()` method
+        - '/^Call to an undefined method Clue\\React\\Redis\\RedisClient::.+\(\)\.$/'
+        - '/^Call to an undefined method Clue\\React\\Redis\\Io\\StreamingClient::.+\(\)\.$/'
+        # ignore incomplete type information for mocks in legacy PHPUnit 7.5
+        - '/^Parameter #\d+ .+ of .+ expects .+, PHPUnit\\Framework\\MockObject\\MockObject given\.$/'

src/Io/Factory.php

@@ -75,6 +75,7 @@ public function createClient(string $uri): PromiseInterface
         if ($parts['scheme'] === 'rediss') {
             $authority = 'tls://' . $authority;
         } elseif ($parts['scheme'] === 'redis+unix') {
+            assert(isset($parts['path']));
             $authority = 'unix://' . substr($parts['path'], 1);
             unset($parts['path']);
         }
@@ -91,6 +92,7 @@ public function createClient(string $uri): PromiseInterface
             $connecting->then(function (ConnectionInterface $connection) {
                 $connection->close();
             });
+            assert(\method_exists($connecting, 'cancel'));
             $connecting->cancel();
         });
 
@@ -106,7 +108,7 @@ public function createClient(string $uri): PromiseInterface
 
         // use `?password=secret` query or `user:secret@host` password form URL
         if (isset($args['password']) || isset($parts['pass'])) {
-            $pass = $args['password'] ?? rawurldecode($parts['pass']);
+            $pass = $args['password'] ?? rawurldecode($parts['pass']); // @phpstan-ignore-line
             $promise = $promise->then(function (StreamingClient $redis) use ($pass, $uri) {
                 return $redis->auth($pass)->then(
                     function () use ($redis) {
@@ -134,7 +136,7 @@ function (\Exception $e) use ($redis, $uri) {
 
         // use `?db=1` query or `/1` path (skip first slash)
         if (isset($args['db']) || (isset($parts['path']) && $parts['path'] !== '/')) {
-            $db = $args['db'] ?? substr($parts['path'], 1);
+            $db = $args['db'] ?? substr($parts['path'], 1); // @phpstan-ignore-line
             $promise = $promise->then(function (StreamingClient $redis) use ($db, $uri) {
                 return $redis->select($db)->then(
                     function () use ($redis) {

src/Io/StreamingClient.php

@@ -2,7 +2,6 @@
 
 namespace Clue\React\Redis\Io;
 
-use Clue\Redis\Protocol\Factory as ProtocolFactory;
 use Clue\Redis\Protocol\Model\ErrorReply;
 use Clue\Redis\Protocol\Model\ModelInterface;
 use Clue\Redis\Protocol\Model\MultiBulkReply;
@@ -22,9 +21,6 @@ class StreamingClient extends EventEmitter
     /** @var DuplexStreamInterface */
     private $stream;
 
-    /** @var ParserInterface */
-    private $parser;
-
     /** @var SerializerInterface */
     private $serializer;
 
@@ -43,18 +39,8 @@ class StreamingClient extends EventEmitter
     /** @var int */
     private $psubscribed = 0;
 
-    public function __construct(DuplexStreamInterface $stream, ParserInterface $parser = null, SerializerInterface $serializer = null)
+    public function __construct(DuplexStreamInterface $stream, ParserInterface $parser, SerializerInterface $serializer)
     {
-        if ($parser === null || $serializer === null) {
-            $factory = new ProtocolFactory();
-            if ($parser === null) {
-                $parser = $factory->createResponseParser();
-            }
-            if ($serializer === null) {
-                $serializer = $factory->createSerializer();
-            }
-        }
-
         $stream->on('data', function (string $chunk) use ($parser) {
             try {
                 $models = $parser->pushIncoming($chunk);
@@ -82,10 +68,10 @@ public function __construct(DuplexStreamInterface $stream, ParserInterface $pars
         $stream->on('close', [$this, 'close']);
 
         $this->stream = $stream;
-        $this->parser = $parser;
         $this->serializer = $serializer;
     }
 
+    /** @param string[] $args */
     public function __call(string $name, array $args): PromiseInterface
     {
         $request = new Deferred();
@@ -139,6 +125,7 @@ public function handleMessage(ModelInterface $message): void
     {
         if (($this->subscribed !== 0 || $this->psubscribed !== 0) && $message instanceof MultiBulkReply) {
             $array = $message->getValueNative();
+            assert(\is_array($array));
             $first = array_shift($array);
 
             // pub/sub messages are to be forwarded and should not be processed as request responses

src/RedisClient.php

@@ -46,7 +46,7 @@ class RedisClient extends EventEmitter
     /** @var float */
     private $idlePeriod = 0.001;
 
-    /** @var ?TimerInterface */
+    /** @var ?\React\EventLoop\TimerInterface */
     private $idleTimer = null;
 
     /** @var int */
@@ -161,6 +161,7 @@ public function __call(string $name, array $args): PromiseInterface
 
         return $this->client()->then(function (StreamingClient $redis) use ($name, $args) {
             $this->awake();
+            assert(\is_callable([$redis, $name])); // @phpstan-ignore-next-line
             return \call_user_func_array([$redis, $name], $args)->then(
                 function ($result) {
                     $this->idle();
@@ -221,6 +222,7 @@ public function close(): void
                 $redis->close();
             });
             if ($this->promise !== null) {
+                assert(\method_exists($this->promise, 'cancel'));
                 $this->promise->cancel();
                 $this->promise = null;
             }
@@ -251,6 +253,7 @@ private function idle(): void
 
         if ($this->pending < 1 && $this->idlePeriod >= 0 && !$this->subscribed && !$this->psubscribed && $this->promise !== null) {
             $this->idleTimer = $this->loop->addTimer($this->idlePeriod, function () {
+                assert($this->promise instanceof PromiseInterface);
                 $this->promise->then(function (StreamingClient $redis) {
                     $redis->close();
                 });

tests/FunctionalTest.php

@@ -26,7 +26,7 @@ public function setUp(): void
         $this->loop = new StreamSelectLoop();
     }
 
-    public function testPing()
+    public function testPing(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -38,7 +38,7 @@ public function testPing()
         $this->assertEquals('PONG', $ret);
     }
 
-    public function testPingLazy()
+    public function testPingLazy(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -53,7 +53,7 @@ public function testPingLazy()
     /**
      * @doesNotPerformAssertions
      */
-    public function testPingLazyWillNotBlockLoop()
+    public function testPingLazyWillNotBlockLoop(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -65,7 +65,7 @@ public function testPingLazyWillNotBlockLoop()
     /**
      * @doesNotPerformAssertions
      */
-    public function testLazyClientWithoutCommandsWillNotBlockLoop()
+    public function testLazyClientWithoutCommandsWillNotBlockLoop(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -74,7 +74,7 @@ public function testLazyClientWithoutCommandsWillNotBlockLoop()
         unset($redis);
     }
 
-    public function testMgetIsNotInterpretedAsSubMessage()
+    public function testMgetIsNotInterpretedAsSubMessage(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -86,7 +86,7 @@ public function testMgetIsNotInterpretedAsSubMessage()
         await($promise, $this->loop);
     }
 
-    public function testPipeline()
+    public function testPipeline(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -98,20 +98,21 @@ public function testPipeline()
         await($promise, $this->loop);
     }
 
-    public function testInvalidCommand()
+    public function testInvalidCommand(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
         $promise = $redis->doesnotexist(1, 2, 3);
 
         if (method_exists($this, 'expectException')) {
             $this->expectException('Exception');
         } else {
+            assert(method_exists($this, 'setExpectedException'));
             $this->setExpectedException('Exception');
         }
         await($promise, $this->loop);
     }
 
-    public function testMultiExecEmpty()
+    public function testMultiExecEmpty(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
         $redis->multi()->then($this->expectCallableOnceWith('OK'));
@@ -120,7 +121,7 @@ public function testMultiExecEmpty()
         await($promise, $this->loop);
     }
 
-    public function testMultiExecQueuedExecHasValues()
+    public function testMultiExecQueuedExecHasValues(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -134,7 +135,7 @@ public function testMultiExecQueuedExecHasValues()
         await($promise, $this->loop);
     }
 
-    public function testPubSub()
+    public function testPubSub(): void
     {
         $consumer = new RedisClient($this->uri, null, $this->loop);
         $producer = new RedisClient($this->uri, null, $this->loop);
@@ -155,7 +156,7 @@ public function testPubSub()
         await($deferred->promise(), $this->loop, 0.1);
     }
 
-    public function testClose()
+    public function testClose(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);
 
@@ -166,7 +167,7 @@ public function testClose()
         $redis->get('willBeRejectedRightAway')->then(null, $this->expectCallableOnce());
     }
 
-    public function testCloseLazy()
+    public function testCloseLazy(): void
     {
         $redis = new RedisClient($this->uri, null, $this->loop);