Skip to content

Commit

Permalink
[1.x] Improve PHP 8.4+ support by avoiding implicitly nullable types
Browse files Browse the repository at this point in the history
This changeset backports reactphp#223 from `3.x` to `1.x` to improve PHP 8.4+ support by avoiding implicitly nullable types as discussed in reactphp/promise#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 reactphp#223, reactphp#204 and reactphp#218
  • Loading branch information
WyriHaximus committed May 22, 2024
1 parent c134600 commit c9bad9b
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 5 deletions.
6 changes: 5 additions & 1 deletion src/Query/TcpTransportExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down
9 changes: 8 additions & 1 deletion src/Query/TimeoutExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@ final class TimeoutExecutor implements ExecutorInterface
private $loop;
private $timeout;

public function __construct(ExecutorInterface $executor, $timeout, LoopInterface $loop = null)
/**
* @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;
Expand Down
6 changes: 5 additions & 1 deletion src/Query/UdpTransportExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down
16 changes: 14 additions & 2 deletions src/Resolver/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions tests/Query/TcpTransportExecutorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use React\Dns\Protocol\Parser;
use React\Dns\Query\Query;
use React\Dns\Query\TcpTransportExecutor;
use React\Dns\Query\UdpTransportExecutor;
use React\EventLoop\Loop;
use React\Tests\Dns\TestCase;

Expand Down Expand Up @@ -943,4 +944,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('127.0.0.1:0', 'loop');
}
}
8 changes: 8 additions & 0 deletions tests/Query/TimeoutExecutorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use React\Dns\Model\Message;
use React\Dns\Query\CancellationException;
use React\Dns\Query\Query;
use React\Dns\Query\TcpTransportExecutor;
use React\Dns\Query\TimeoutException;
use React\Dns\Query\TimeoutExecutor;
use React\Promise;
Expand Down Expand Up @@ -185,4 +186,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');
}
}
8 changes: 8 additions & 0 deletions tests/Query/UdpTransportExecutorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use React\Dns\Protocol\Parser;
use React\Dns\Query\Query;
use React\Dns\Query\UdpTransportExecutor;
use React\Dns\Resolver\Factory;
use React\EventLoop\Loop;
use React\Tests\Dns\TestCase;

Expand Down Expand Up @@ -375,4 +376,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('127.0.0.1:0', 'loop');
}
}
26 changes: 26 additions & 0 deletions tests/Resolver/FactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit c9bad9b

Please sign in to comment.