From bcde19048e9ed9559247b5af522c2032facc085d Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Thu, 9 Feb 2023 12:58:34 +0545 Subject: [PATCH 1/4] Add phpstan-phpunit and phpstan-strict-rules --- composer.json | 8 ++++++++ tests/Event/CoroutineTest.php | 4 ++-- tests/Event/EmitterTest.php | 12 ++++++------ tests/Event/Loop/FunctionsTest.php | 4 ++-- tests/Event/Loop/LoopTest.php | 4 ++-- tests/Event/Promise/PromiseTest.php | 5 ++--- tests/Event/WildcardEmitterTest.php | 14 +++++++------- 7 files changed, 29 insertions(+), 22 deletions(-) diff --git a/composer.json b/composer.json index 59937c5..1a9a48f 100644 --- a/composer.json +++ b/composer.json @@ -48,6 +48,9 @@ "require-dev": { "friendsofphp/php-cs-fixer": "^3.14", "phpstan/phpstan": "^1.9", + "phpstan/phpstan-phpunit": "^1.3", + "phpstan/phpstan-strict-rules": "^1.4", + "phpstan/extension-installer": "^1.2", "phpunit/phpunit" : "^9.6" }, "scripts": { @@ -65,5 +68,10 @@ "composer cs-fixer", "composer phpunit" ] + }, + "config": { + "allow-plugins": { + "phpstan/extension-installer": true + } } } diff --git a/tests/Event/CoroutineTest.php b/tests/Event/CoroutineTest.php index c2e7d4c..172b26d 100644 --- a/tests/Event/CoroutineTest.php +++ b/tests/Event/CoroutineTest.php @@ -173,7 +173,7 @@ public function testReturn(): void self::assertEquals(3, $value); $ok = true; })->otherwise(function ($reason) { - $this->fail($reason); + self::fail($reason); }); Loop\run(); @@ -193,7 +193,7 @@ public function testReturnPromise(): void })->then(function ($value) use (&$ok) { $ok = $value; })->otherwise(function ($reason) { - $this->fail($reason); + self::fail($reason); }); $promise->fulfill('omg it worked'); diff --git a/tests/Event/EmitterTest.php b/tests/Event/EmitterTest.php index 93665da..c686c8d 100644 --- a/tests/Event/EmitterTest.php +++ b/tests/Event/EmitterTest.php @@ -9,7 +9,7 @@ class EmitterTest extends \PHPUnit\Framework\TestCase public function testInit(): void { $ee = new Emitter(); - self::assertInstanceOf('Sabre\\Event\\Emitter', $ee); + self::assertInstanceOf('Sabre\\Event\\Emitter', $ee); /* @phpstan-ignore-line */ } public function testListeners(): void @@ -139,7 +139,7 @@ public function testRemoveListener(): void ); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testRemoveUnknownListener(): void @@ -161,7 +161,7 @@ public function testRemoveUnknownListener(): void self::assertFalse($ee->removeListener('bar', $callBack)); $ee->emit('foo'); - self::assertTrue($result); + self::assertTrue($result); /* @phpstan-ignore-line */ } public function testRemoveListenerTwice(): void @@ -188,7 +188,7 @@ public function testRemoveListenerTwice(): void ); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testRemoveAllListeners(): void @@ -208,7 +208,7 @@ public function testRemoveAllListeners(): void $ee->removeAllListeners('foo'); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testRemoveAllListenersNoArg(): void @@ -229,7 +229,7 @@ public function testRemoveAllListenersNoArg(): void $ee->removeAllListeners(); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testOnce(): void diff --git a/tests/Event/Loop/FunctionsTest.php b/tests/Event/Loop/FunctionsTest.php index 396f8d4..fcec877 100644 --- a/tests/Event/Loop/FunctionsTest.php +++ b/tests/Event/Loop/FunctionsTest.php @@ -82,7 +82,7 @@ public function testAddWriteStream(): void { $h = fopen('php://temp', 'r+'); if (false === $h) { - $this->fail('failed to open php://temp'); + self::fail('failed to open php://temp'); } addWriteStream($h, function () use ($h) { fwrite($h, 'hello world'); @@ -97,7 +97,7 @@ public function testAddReadStream(): void { $h = fopen('php://temp', 'r+'); if (false === $h) { - $this->fail('failed to open php://temp'); + self::fail('failed to open php://temp'); } fwrite($h, 'hello world'); rewind($h); diff --git a/tests/Event/Loop/LoopTest.php b/tests/Event/Loop/LoopTest.php index 992d7f2..1152b38 100644 --- a/tests/Event/Loop/LoopTest.php +++ b/tests/Event/Loop/LoopTest.php @@ -74,7 +74,7 @@ public function testAddWriteStream(): void { $h = fopen('php://temp', 'r+'); if (false === $h) { - $this->fail('failed to open php://temp'); + self::fail('failed to open php://temp'); } $loop = new Loop(); $loop->addWriteStream($h, function () use ($h, $loop) { @@ -90,7 +90,7 @@ public function testAddReadStream(): void { $h = fopen('php://temp', 'r+'); if (false === $h) { - $this->fail('failed to open php://temp'); + self::fail('failed to open php://temp'); } fwrite($h, 'hello world'); rewind($h); diff --git a/tests/Event/Promise/PromiseTest.php b/tests/Event/Promise/PromiseTest.php index 080de2a..8dcf20f 100644 --- a/tests/Event/Promise/PromiseTest.php +++ b/tests/Event/Promise/PromiseTest.php @@ -199,7 +199,7 @@ public function testWaitRejectedException(): void }); try { $promise->wait(); - $this->fail('We did not get the expected exception'); + self::fail('We did not get the expected exception'); } catch (\Exception $e) { self::assertInstanceOf('OutOfBoundsException', $e); self::assertEquals('foo', $e->getMessage()); @@ -214,9 +214,8 @@ public function testWaitRejectedScalar(): void }); try { $promise->wait(); - $this->fail('We did not get the expected exception'); + self::fail('We did not get the expected exception'); } catch (\Exception $e) { - self::assertInstanceOf('Exception', $e); self::assertEquals('foo', $e->getMessage()); } } diff --git a/tests/Event/WildcardEmitterTest.php b/tests/Event/WildcardEmitterTest.php index ab0be9c..4726adb 100644 --- a/tests/Event/WildcardEmitterTest.php +++ b/tests/Event/WildcardEmitterTest.php @@ -9,7 +9,7 @@ class WildcardEmitterTest extends \PHPUnit\Framework\TestCase public function testInit(): void { $ee = new WildcardEmitter(); - self::assertInstanceOf('Sabre\\Event\\WildcardEmitter', $ee); + self::assertInstanceOf('Sabre\\Event\\WildcardEmitter', $ee); /* @phpstan-ignore-line */ } public function testListeners(): void @@ -159,7 +159,7 @@ public function testRemoveListener(): void ); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testRemoveUnknownListener(): void @@ -181,7 +181,7 @@ public function testRemoveUnknownListener(): void self::assertFalse($ee->removeListener('bar', $callBack)); $ee->emit('foo'); - self::assertTrue($result); + self::assertTrue($result); /* @phpstan-ignore-line */ } public function testRemoveListenerTwice(): void @@ -208,7 +208,7 @@ public function testRemoveListenerTwice(): void ); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testRemoveAllListeners(): void @@ -228,7 +228,7 @@ public function testRemoveAllListeners(): void $ee->removeAllListeners('foo'); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testRemoveAllListenersNoArg(): void @@ -249,7 +249,7 @@ public function testRemoveAllListenersNoArg(): void $ee->removeAllListeners(); $ee->emit('foo'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testRemoveAllListenersWildcard(): void @@ -269,7 +269,7 @@ public function testRemoveAllListenersWildcard(): void $ee->removeAllListeners('foo:*'); $ee->emit('foo:bar'); - self::assertFalse($result); + self::assertFalse($result); /* @phpstan-ignore-line */ } public function testOnce(): void From b6948d311811d1fd10f9e94323c4340573fb04b4 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Thu, 9 Feb 2023 14:12:30 +0545 Subject: [PATCH 2/4] Add phpstan messages to ignoreErrors --- phpstan.neon | 24 ++++++++++++++++++++++++ tests/Event/EmitterTest.php | 12 ++++++------ tests/Event/WildcardEmitterTest.php | 14 +++++++------- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index ba0869a..19389ea 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -10,3 +10,27 @@ parameters: message: "#^Left side of && is always true.$#" count: 1 path: lib/Loop/Loop.php + - + message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertInstanceOf\\(\\) with .* and .* will always evaluate to true\\.$#" + count: 1 + path: tests/Event/EmitterTest.php + - + message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertFalse\\(\\) with false will always evaluate to true\\.$#" + count: 4 + path: tests/Event/EmitterTest.php + - + message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertTrue\\(\\) with false will always evaluate to false\\.$#" + count: 1 + path: tests/Event/EmitterTest.php + - + message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertInstanceOf\\(\\) with .* and .* will always evaluate to true\\.$#" + count: 1 + path: tests/Event/WildcardEmitterTest.php + - + message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertFalse\\(\\) with false will always evaluate to true\\.$#" + count: 5 + path: tests/Event/WildcardEmitterTest.php + - + message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertTrue\\(\\) with false will always evaluate to false\\.$#" + count: 1 + path: tests/Event/WildcardEmitterTest.php diff --git a/tests/Event/EmitterTest.php b/tests/Event/EmitterTest.php index c686c8d..93665da 100644 --- a/tests/Event/EmitterTest.php +++ b/tests/Event/EmitterTest.php @@ -9,7 +9,7 @@ class EmitterTest extends \PHPUnit\Framework\TestCase public function testInit(): void { $ee = new Emitter(); - self::assertInstanceOf('Sabre\\Event\\Emitter', $ee); /* @phpstan-ignore-line */ + self::assertInstanceOf('Sabre\\Event\\Emitter', $ee); } public function testListeners(): void @@ -139,7 +139,7 @@ public function testRemoveListener(): void ); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testRemoveUnknownListener(): void @@ -161,7 +161,7 @@ public function testRemoveUnknownListener(): void self::assertFalse($ee->removeListener('bar', $callBack)); $ee->emit('foo'); - self::assertTrue($result); /* @phpstan-ignore-line */ + self::assertTrue($result); } public function testRemoveListenerTwice(): void @@ -188,7 +188,7 @@ public function testRemoveListenerTwice(): void ); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testRemoveAllListeners(): void @@ -208,7 +208,7 @@ public function testRemoveAllListeners(): void $ee->removeAllListeners('foo'); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testRemoveAllListenersNoArg(): void @@ -229,7 +229,7 @@ public function testRemoveAllListenersNoArg(): void $ee->removeAllListeners(); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testOnce(): void diff --git a/tests/Event/WildcardEmitterTest.php b/tests/Event/WildcardEmitterTest.php index 4726adb..ab0be9c 100644 --- a/tests/Event/WildcardEmitterTest.php +++ b/tests/Event/WildcardEmitterTest.php @@ -9,7 +9,7 @@ class WildcardEmitterTest extends \PHPUnit\Framework\TestCase public function testInit(): void { $ee = new WildcardEmitter(); - self::assertInstanceOf('Sabre\\Event\\WildcardEmitter', $ee); /* @phpstan-ignore-line */ + self::assertInstanceOf('Sabre\\Event\\WildcardEmitter', $ee); } public function testListeners(): void @@ -159,7 +159,7 @@ public function testRemoveListener(): void ); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testRemoveUnknownListener(): void @@ -181,7 +181,7 @@ public function testRemoveUnknownListener(): void self::assertFalse($ee->removeListener('bar', $callBack)); $ee->emit('foo'); - self::assertTrue($result); /* @phpstan-ignore-line */ + self::assertTrue($result); } public function testRemoveListenerTwice(): void @@ -208,7 +208,7 @@ public function testRemoveListenerTwice(): void ); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testRemoveAllListeners(): void @@ -228,7 +228,7 @@ public function testRemoveAllListeners(): void $ee->removeAllListeners('foo'); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testRemoveAllListenersNoArg(): void @@ -249,7 +249,7 @@ public function testRemoveAllListenersNoArg(): void $ee->removeAllListeners(); $ee->emit('foo'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testRemoveAllListenersWildcard(): void @@ -269,7 +269,7 @@ public function testRemoveAllListenersWildcard(): void $ee->removeAllListeners('foo:*'); $ee->emit('foo:bar'); - self::assertFalse($result); /* @phpstan-ignore-line */ + self::assertFalse($result); } public function testOnce(): void From b421231d4dc6137db68b871a16d83b5f62b6dbfb Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Thu, 9 Feb 2023 15:44:16 +0545 Subject: [PATCH 3/4] Adjust code for things reported by phpstan --- lib/EmitterTrait.php | 2 +- lib/Loop/Loop.php | 14 +++++++------- lib/Loop/functions.php | 2 +- lib/Promise.php | 2 +- lib/Promise/functions.php | 2 +- phpstan.neon | 12 ++++++++++++ 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/EmitterTrait.php b/lib/EmitterTrait.php index 67d732c..c75645d 100644 --- a/lib/EmitterTrait.php +++ b/lib/EmitterTrait.php @@ -172,7 +172,7 @@ public function removeAllListeners(string $eventName = null): void /** * The list of listeners. * - * @var array + * @var array, 2:array}> */ protected array $listeners = []; } diff --git a/lib/Loop/Loop.php b/lib/Loop/Loop.php index e7019ce..2a9f767 100644 --- a/lib/Loop/Loop.php +++ b/lib/Loop/Loop.php @@ -26,7 +26,7 @@ public function setTimeout(callable $cb, float $timeout): void { $triggerTime = microtime(true) + $timeout; - if (!$this->timers) { + if (0 === count($this->timers)) { // Special case when the timers array was empty. $this->timers[] = [$triggerTime, $cb]; @@ -200,7 +200,7 @@ public function tick(bool $block = false): bool if (!$block) { // Don't wait $streamWait = 0; - } elseif ($this->nextTick) { + } elseif (count($this->nextTick) > 0) { // There's a pending 'nextTick'. Don't wait. $streamWait = 0; } elseif (is_numeric($nextTimeout)) { @@ -213,7 +213,7 @@ public function tick(bool $block = false): bool $this->runStreams($streamWait); - return $this->readStreams || $this->writeStreams || $this->nextTick || $this->timers; + return count($this->readStreams) > 0 || count($this->writeStreams) > 0 || count($this->nextTick) > 0 || count($this->timers) > 0; } /** @@ -250,11 +250,11 @@ protected function runNextTicks(): void protected function runTimers(): ?float { $now = microtime(true); - while (($timer = array_pop($this->timers)) && $timer[0] < $now) { + while (($timer = array_pop($this->timers)) !== null && $timer[0] < $now) { $timer[1](); } // Add the last timer back to the array. - if ($timer) { + if (null !== $timer) { $this->timers[] = $timer; return max(0, $timer[0] - microtime(true)); @@ -271,7 +271,7 @@ protected function runTimers(): ?float */ protected function runStreams(?float $timeout): void { - if ($this->readStreams || $this->writeStreams) { + if (count($this->readStreams) > 0 || count($this->writeStreams) > 0) { $read = $this->readStreams; $write = $this->writeStreams; $except = null; @@ -289,7 +289,7 @@ protected function runStreams(?float $timeout): void $writeCb(); } } - } elseif ($this->running && ($this->nextTick || $this->timers)) { + } elseif ($this->running && (count($this->nextTick) > 0 || count($this->timers) > 0)) { usleep(null !== $timeout ? intval($timeout * 1000000) : 200000); } } diff --git a/lib/Loop/functions.php b/lib/Loop/functions.php index 6cc457c..541427d 100644 --- a/lib/Loop/functions.php +++ b/lib/Loop/functions.php @@ -137,7 +137,7 @@ function stop(): void function instance(Loop $newLoop = null): Loop { static $loop; - if ($newLoop) { + if ($newLoop instanceof Loop) { $loop = $newLoop; } elseif (!$loop) { $loop = new Loop(); diff --git a/lib/Promise.php b/lib/Promise.php index 60022ac..bd6cea3 100644 --- a/lib/Promise.php +++ b/lib/Promise.php @@ -55,7 +55,7 @@ class Promise */ public function __construct(callable $executor = null) { - if ($executor) { + if (is_callable($executor)) { $executor( [$this, 'fulfill'], [$this, 'reject'] diff --git a/lib/Promise/functions.php b/lib/Promise/functions.php index 8b9a7cb..d618303 100644 --- a/lib/Promise/functions.php +++ b/lib/Promise/functions.php @@ -34,7 +34,7 @@ function all(array $promises): Promise { return new Promise(function ($success, $fail) use ($promises) { - if (empty($promises)) { + if (0 === count($promises)) { $success([]); return; diff --git a/phpstan.neon b/phpstan.neon index 19389ea..50e29f9 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -10,6 +10,18 @@ parameters: message: "#^Left side of && is always true.$#" count: 1 path: lib/Loop/Loop.php + - + message: "#^Variable \\$timer might not be defined.$#" + count: 1 + path: lib/Loop/Loop.php + - + message: "#^Only booleans are allowed in an if condition, int<0, max>|false given.$#" + count: 1 + path: lib/Loop/Loop.php + - + message: "#^Instanceof between Generator and Generator will always evaluate to true.$#" + count: 1 + path: lib/coroutine.php - message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertInstanceOf\\(\\) with .* and .* will always evaluate to true\\.$#" count: 1 From 496e8839c4288c8f8e00b4487bc254a34adbb436 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Thu, 9 Feb 2023 15:49:15 +0545 Subject: [PATCH 4/4] Reduce detail in phpstan.neon for tests Co-authored-by: Markus Staab --- phpstan.neon | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 50e29f9..fd52a60 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -23,26 +23,8 @@ parameters: count: 1 path: lib/coroutine.php - - message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertInstanceOf\\(\\) with .* and .* will always evaluate to true\\.$#" - count: 1 - path: tests/Event/EmitterTest.php - - - message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertFalse\\(\\) with false will always evaluate to true\\.$#" - count: 4 - path: tests/Event/EmitterTest.php - - - message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertTrue\\(\\) with false will always evaluate to false\\.$#" - count: 1 - path: tests/Event/EmitterTest.php + message: "#^.* will always evaluate to true\\.$#" + path: tests/* - - message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertInstanceOf\\(\\) with .* and .* will always evaluate to true\\.$#" - count: 1 - path: tests/Event/WildcardEmitterTest.php - - - message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertFalse\\(\\) with false will always evaluate to true\\.$#" - count: 5 - path: tests/Event/WildcardEmitterTest.php - - - message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert::assertTrue\\(\\) with false will always evaluate to false\\.$#" - count: 1 - path: tests/Event/WildcardEmitterTest.php + message: "#^.* will always evaluate to false\\.$#" + path: tests/*