Improve PHP 8.4+ support by avoiding implicitly nullable types

clue-labs · Dec 27, 2024 · 2b3fef6 · 2b3fef6
1 parent 2283690
commit 2b3fef6
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 8 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -7,10 +7,11 @@ on:
 jobs:
   PHPUnit:
     name: PHPUnit (PHP ${{ matrix.php }})
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
     strategy:
       matrix:
         php:
+          - 8.4
           - 8.3
           - 8.2
           - 8.1
@@ -39,7 +40,7 @@ jobs:
 
   PHPUnit-hhvm:
     name: PHPUnit (HHVM)
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
     continue-on-error: true
     services:
       redis:

diff --git a/composer.json b/composer.json
@@ -12,12 +12,12 @@
     ],
     "require": {
         "php": ">=5.3",
-        "clue/redis-protocol": "0.3.*",
+        "clue/redis-protocol": "^0.3.2",
         "evenement/evenement": "^3.0 || ^2.0 || ^1.0",
         "react/event-loop": "^1.2",
-        "react/promise": "^3 || ^2.0 || ^1.1",
-        "react/promise-timer": "^1.9",
-        "react/socket": "^1.12"
+        "react/promise": "^3.2 || ^2.0 || ^1.1",
+        "react/promise-timer": "^1.11",
+        "react/socket": "^1.16"
     },
     "require-dev": {
         "clue/block-react": "^1.5",

diff --git a/src/Factory.php b/src/Factory.php
@@ -27,8 +27,18 @@ class Factory
      * @param ?ConnectorInterface $connector
      * @param ?ProtocolFactory $protocol
      */
-    public function __construct(LoopInterface $loop = null, ConnectorInterface $connector = null, ProtocolFactory $protocol = null)
+    public function __construct($loop = null, $connector = null, $protocol = null)
     {
+        if ($loop !== null && !$loop instanceof LoopInterface) { // manual type check to support legacy PHP < 7.1
+            throw new \InvalidArgumentException('Argument #1 ($loop) expected null|React\EventLoop\LoopInterface');
+        }
+        if ($connector !== null && !$connector instanceof ConnectorInterface) { // manual type check to support legacy PHP < 7.1
+            throw new \InvalidArgumentException('Argument #2 ($connector) expected null|React\Socket\ConnectorInterface');
+        }
+        if ($protocol !== null && !$protocol instanceof LoopInterface) { // manual type check to support legacy PHP < 7.1
+            throw new \InvalidArgumentException('Argument #3 ($protocol) expected null|Clue\Redis\Protocol\Factory');
+        }
+
         $this->loop = $loop ?: Loop::get();
         $this->connector = $connector ?: new Connector(array(), $this->loop);
         $this->protocol = $protocol ?: new ProtocolFactory();

diff --git a/src/StreamingClient.php b/src/StreamingClient.php
@@ -28,8 +28,17 @@ class StreamingClient extends EventEmitter implements Client
     private $subscribed = 0;
     private $psubscribed = 0;
 
-    public function __construct(DuplexStreamInterface $stream, ParserInterface $parser = null, SerializerInterface $serializer = null)
+    /**
+     * @param DuplexStreamInterface $stream
+     * @param ?ParserInterface $parser
+     * @param ?SerializerInterface $serializer
+     */
+    public function __construct(DuplexStreamInterface $stream, $parser = null, $serializer = null)
     {
+        // manual type checks to support legacy PHP < 7.1
+        assert($parser === null || $parser instanceof ParserInterface);
+        assert($serializer === null || $serializer instanceof SerializerInterface);
+
         if ($parser === null || $serializer === null) {
             $factory = new ProtocolFactory();
             if ($parser === null) {

diff --git a/tests/FactoryLazyClientTest.php b/tests/FactoryLazyClientTest.php
@@ -32,6 +32,24 @@ public function testConstructWithoutLoopAssignsLoopAutomatically()
         $this->assertInstanceOf('React\EventLoop\LoopInterface', $loop);
     }
 
+    public function testContructorThrowsExceptionForInvalidLoop()
+    {
+        $this->setExpectedException('InvalidArgumentException', 'Argument #1 ($loop) expected null|React\EventLoop\LoopInterface');
+        new Factory('loop');
+    }
+
+    public function testContructorThrowsExceptionForInvalidConnector()
+    {
+        $this->setExpectedException('InvalidArgumentException', 'Argument #2 ($connector) expected null|React\Socket\ConnectorInterface');
+        new Factory(null, 'connector');
+    }
+
+    public function testContructorThrowsExceptionForInvalidProtocolFactory()
+    {
+        $this->setExpectedException('InvalidArgumentException', 'Argument #3 ($protocol) expected null|Clue\Redis\Protocol\Factory');
+        new Factory(null, null, 'protocol');
+    }
+
     public function testWillConnectWithDefaultPort()
     {
         $this->connector->expects($this->never())->method('connect');

diff --git a/tests/TestCase.php b/tests/TestCase.php
@@ -69,4 +69,21 @@ protected function expectPromiseReject($promise)
 
         return $promise;
     }
+
+    public function setExpectedException($exception, $exceptionMessage = '', $exceptionCode = null)
+    {
+        if (method_exists($this, 'expectException')) {
+            // PHPUnit 5.2+
+            $this->expectException($exception);
+            if ($exceptionMessage !== '') {
+                $this->expectExceptionMessage($exceptionMessage);
+            }
+            if ($exceptionCode !== null) {
+                $this->expectExceptionCode($exceptionCode);
+            }
+        } else {
+            // legacy PHPUnit
+            parent::setExpectedException($exception, $exceptionMessage, $exceptionCode);
+        }
+    }
 }