From 4f9b9756f05444c9168d494147b8cc742af04d9b Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 22 May 2024 08:16:50 +0200 Subject: [PATCH] [1.x] Improve PHP 8.4+ support by avoiding implicitly nullable types This changeset backports #223 from `3.x` to `1.x` to improve PHP 8.4+ support by avoiding implicitly nullable types as discussed in https://github.com/reactphp/promise/pull/260. The same idea applies, but v1 requires manual type checks to support legacy PHP versions as the nullable type syntax requires PHP 7.1+ otherwise. Builds on top of #223, #204 and #218 --- src/Query/TcpTransportExecutor.php | 6 +++++- src/Query/TimeoutExecutor.php | 11 +++++++++- src/Query/UdpTransportExecutor.php | 6 +++++- src/Resolver/Factory.php | 16 +++++++++++++-- tests/Query/TcpTransportExecutorTest.php | 7 +++++++ tests/Query/TimeoutExecutorTest.php | 7 +++++++ tests/Query/UdpTransportExecutorTest.php | 7 +++++++ tests/Resolver/FactoryTest.php | 26 ++++++++++++++++++++++++ 8 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/Query/TcpTransportExecutor.php b/src/Query/TcpTransportExecutor.php index bfaedbae..669fd012 100644 --- a/src/Query/TcpTransportExecutor.php +++ b/src/Query/TcpTransportExecutor.php @@ -134,7 +134,7 @@ class TcpTransportExecutor implements ExecutorInterface * @param string $nameserver * @param ?LoopInterface $loop */ - public function __construct($nameserver, LoopInterface $loop = null) + public function __construct($nameserver, $loop = null) { if (\strpos($nameserver, '[') === false && \substr_count($nameserver, ':') >= 2 && \strpos($nameserver, '://') === false) { // several colons, but not enclosed in square brackets => enclose IPv6 address in square brackets @@ -146,6 +146,10 @@ public function __construct($nameserver, LoopInterface $loop = null) throw new \InvalidArgumentException('Invalid nameserver address given'); } + if ($loop !== null && !$loop instanceof LoopInterface) { // manual type check to support legacy PHP < 7.1 + throw new \InvalidArgumentException('Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + } + $this->nameserver = 'tcp://' . $parts['host'] . ':' . (isset($parts['port']) ? $parts['port'] : 53); $this->loop = $loop ?: Loop::get(); $this->parser = new Parser(); diff --git a/src/Query/TimeoutExecutor.php b/src/Query/TimeoutExecutor.php index 06c51b15..b61fea6d 100644 --- a/src/Query/TimeoutExecutor.php +++ b/src/Query/TimeoutExecutor.php @@ -12,8 +12,17 @@ final class TimeoutExecutor implements ExecutorInterface private $loop; private $timeout; - public function __construct(ExecutorInterface $executor, $timeout, LoopInterface $loop = null) + /** + * @param ExecutorInterface $executor + * @param float $timeout + * @param ?LoopInterface $loop + */ + public function __construct(ExecutorInterface $executor, $timeout, $loop = null) { + if ($loop !== null && !$loop instanceof LoopInterface) { // manual type check to support legacy PHP < 7.1 + throw new \InvalidArgumentException('Argument #3 ($loop) expected null|React\EventLoop\LoopInterface'); + } + $this->executor = $executor; $this->loop = $loop ?: Loop::get(); $this->timeout = $timeout; diff --git a/src/Query/UdpTransportExecutor.php b/src/Query/UdpTransportExecutor.php index 30a3d705..a8cbfafa 100644 --- a/src/Query/UdpTransportExecutor.php +++ b/src/Query/UdpTransportExecutor.php @@ -98,7 +98,7 @@ final class UdpTransportExecutor implements ExecutorInterface * @param string $nameserver * @param ?LoopInterface $loop */ - public function __construct($nameserver, LoopInterface $loop = null) + public function __construct($nameserver, $loop = null) { if (\strpos($nameserver, '[') === false && \substr_count($nameserver, ':') >= 2 && \strpos($nameserver, '://') === false) { // several colons, but not enclosed in square brackets => enclose IPv6 address in square brackets @@ -110,6 +110,10 @@ public function __construct($nameserver, LoopInterface $loop = null) throw new \InvalidArgumentException('Invalid nameserver address given'); } + if ($loop !== null && !$loop instanceof LoopInterface) { // manual type check to support legacy PHP < 7.1 + throw new \InvalidArgumentException('Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + } + $this->nameserver = 'udp://' . $parts['host'] . ':' . (isset($parts['port']) ? $parts['port'] : 53); $this->loop = $loop ?: Loop::get(); $this->parser = new Parser(); diff --git a/src/Resolver/Factory.php b/src/Resolver/Factory.php index 5fe608cb..52658951 100644 --- a/src/Resolver/Factory.php +++ b/src/Resolver/Factory.php @@ -36,8 +36,12 @@ final class Factory * @throws \InvalidArgumentException for invalid DNS server address * @throws \UnderflowException when given DNS Config object has an empty list of nameservers */ - public function create($config, LoopInterface $loop = null) + public function create($config, $loop = null) { + if ($loop !== null && !$loop instanceof LoopInterface) { // manual type check to support legacy PHP < 7.1 + throw new \InvalidArgumentException('Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + } + $executor = $this->decorateHostsFileExecutor($this->createExecutor($config, $loop ?: Loop::get())); return new Resolver($executor); @@ -59,8 +63,16 @@ public function create($config, LoopInterface $loop = null) * @throws \InvalidArgumentException for invalid DNS server address * @throws \UnderflowException when given DNS Config object has an empty list of nameservers */ - public function createCached($config, LoopInterface $loop = null, CacheInterface $cache = null) + public function createCached($config, $loop = null, $cache = null) { + if ($loop !== null && !$loop instanceof LoopInterface) { // manual type check to support legacy PHP < 7.1 + throw new \InvalidArgumentException('Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + } + + if ($cache !== null && !$cache instanceof CacheInterface) { // manual type check to support legacy PHP < 7.1 + throw new \InvalidArgumentException('Argument #3 ($cache) expected null|React\Cache\CacheInterface'); + } + // default to keeping maximum of 256 responses in cache unless explicitly given if (!($cache instanceof CacheInterface)) { $cache = new ArrayCache(256); diff --git a/tests/Query/TcpTransportExecutorTest.php b/tests/Query/TcpTransportExecutorTest.php index 33dddacd..6c25a6db 100644 --- a/tests/Query/TcpTransportExecutorTest.php +++ b/tests/Query/TcpTransportExecutorTest.php @@ -943,4 +943,11 @@ public function testQueryAgainAfterPreviousQueryResolvedWillReuseSocketAndCancel // trigger second query $executor->query($query); } + + /** @test */ + public function constructorThrowsExceptionForInvalidLoop() + { + $this->setExpectedException('InvalidArgumentException', 'Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + new TcpTransportExecutor('tcp://127.0.0.1:53', 'loop'); + } } diff --git a/tests/Query/TimeoutExecutorTest.php b/tests/Query/TimeoutExecutorTest.php index b7857783..e4ed5788 100644 --- a/tests/Query/TimeoutExecutorTest.php +++ b/tests/Query/TimeoutExecutorTest.php @@ -185,4 +185,11 @@ public function testRejectsPromiseAndCancelsPendingQueryWhenTimeoutTriggers() $this->assertInstanceOf('React\Dns\Query\TimeoutException', $exception); $this->assertEquals('DNS query for igor.io (A) timed out' , $exception->getMessage()); } + + /** @test */ + public function constructorThrowsExceptionForInvalidLoop() + { + $this->setExpectedException('InvalidArgumentException', 'Argument #3 ($loop) expected null|React\EventLoop\LoopInterface'); + new TimeoutExecutor($this->executor, 5.0, 'loop'); + } } diff --git a/tests/Query/UdpTransportExecutorTest.php b/tests/Query/UdpTransportExecutorTest.php index 3f04d9ad..eb1afbaf 100644 --- a/tests/Query/UdpTransportExecutorTest.php +++ b/tests/Query/UdpTransportExecutorTest.php @@ -375,4 +375,11 @@ public function testQueryResolvesIfServerSendsValidResponse() $this->assertInstanceOf('React\Dns\Model\Message', $response); } + + /** @test */ + public function constructorThrowsExceptionForInvalidLoop() + { + $this->setExpectedException('InvalidArgumentException', 'Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + new UdpTransportExecutor('udp://127.0.0.1:53', 'loop'); + } } diff --git a/tests/Resolver/FactoryTest.php b/tests/Resolver/FactoryTest.php index af758b51..585ee985 100644 --- a/tests/Resolver/FactoryTest.php +++ b/tests/Resolver/FactoryTest.php @@ -399,6 +399,32 @@ public function createCachedShouldCreateResolverWithCachingExecutorWithCustomCac $this->assertSame($cache, $cacheProperty); } + /** @test */ + public function createThrowsExceptionForInvalidLoop() + { + $this->setExpectedException('InvalidArgumentException', 'Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + $factory = new Factory(); + $factory->create('config', 'loop'); + } + + /** @test */ + public function createCachedThrowsExceptionForInvalidLoop() + { + $this->setExpectedException('InvalidArgumentException', 'Argument #2 ($loop) expected null|React\EventLoop\LoopInterface'); + $factory = new Factory(); + $factory->createCached('config', 'loop'); + } + + /** @test */ + public function createCachedThrowsExceptionForInvalidCache() + { + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + + $this->setExpectedException('InvalidArgumentException', 'Argument #3 ($cache) expected null|React\Cache\CacheInterface'); + $factory = new Factory(); + $factory->createCached('config', $loop, 'cache'); + } + private function getResolverPrivateExecutor($resolver) { $executor = $this->getResolverPrivateMemberValue($resolver, 'executor');