Skip to content

Add PHPStan to test environment with max level #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 22 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
4 changes: 3 additions & 1 deletion examples/cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
17 changes: 17 additions & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
@@ -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\.$/'
6 changes: 4 additions & 2 deletions src/Io/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
}
Expand All @@ -91,6 +92,7 @@ public function createClient(string $uri): PromiseInterface
$connecting->then(function (ConnectionInterface $connection) {
$connection->close();
});
assert(\method_exists($connecting, 'cancel'));
$connecting->cancel();
});

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 3 additions & 16 deletions src/Io/StreamingClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,9 +21,6 @@ class StreamingClient extends EventEmitter
/** @var DuplexStreamInterface */
private $stream;

/** @var ParserInterface */
private $parser;

/** @var SerializerInterface */
private $serializer;

Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion src/RedisClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
});
Expand Down
25 changes: 13 additions & 12 deletions tests/FunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -53,7 +53,7 @@ public function testPingLazy()
/**
* @doesNotPerformAssertions
*/
public function testPingLazyWillNotBlockLoop()
public function testPingLazyWillNotBlockLoop(): void
{
$redis = new RedisClient($this->uri, null, $this->loop);

Expand All @@ -65,7 +65,7 @@ public function testPingLazyWillNotBlockLoop()
/**
* @doesNotPerformAssertions
*/
public function testLazyClientWithoutCommandsWillNotBlockLoop()
public function testLazyClientWithoutCommandsWillNotBlockLoop(): void
{
$redis = new RedisClient($this->uri, null, $this->loop);

Expand All @@ -74,7 +74,7 @@ public function testLazyClientWithoutCommandsWillNotBlockLoop()
unset($redis);
}

public function testMgetIsNotInterpretedAsSubMessage()
public function testMgetIsNotInterpretedAsSubMessage(): void
{
$redis = new RedisClient($this->uri, null, $this->loop);

Expand All @@ -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);

Expand All @@ -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'));
Expand All @@ -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);

Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);

Expand Down
Loading