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 #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 #223, #204 and #218
  • Loading branch information
WyriHaximus committed Jun 5, 2024
1 parent c134600 commit 9f16e24
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 8 deletions.
6 changes: 3 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
"php": ">=5.3.0",
"react/cache": "^1.0 || ^0.6 || ^0.5",
"react/event-loop": "^1.2",
"react/promise": "^3.0 || ^2.7 || ^1.2.1"
"react/promise": "^3.2 || ^2.7 || ^1.2.1"
},
"require-dev": {
"phpunit/phpunit": "^9.6 || ^5.7 || ^4.8.36",
"react/async": "^4 || ^3 || ^2",
"react/promise-timer": "^1.9"
"react/async": "^4.3 || ^3 || ^2",
"react/promise-timer": "^1.11"
},
"autoload": {
"psr-4": {
Expand Down
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
11 changes: 10 additions & 1 deletion src/Query/TimeoutExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
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
7 changes: 7 additions & 0 deletions tests/Query/TcpTransportExecutorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
7 changes: 7 additions & 0 deletions tests/Query/TimeoutExecutorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
7 changes: 7 additions & 0 deletions tests/Query/UdpTransportExecutorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
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 9f16e24

Please sign in to comment.