Skip to content
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

Add phpstan-phpunit and phpstan-strict-rules #105

Merged
merged 4 commits into from
Apr 12, 2023
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
8 changes: 8 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -65,5 +68,10 @@
"composer cs-fixer",
"composer phpunit"
]
},
"config": {
"allow-plugins": {
"phpstan/extension-installer": true
}
}
}
2 changes: 1 addition & 1 deletion lib/EmitterTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public function removeAllListeners(string $eventName = null): void
/**
* The list of listeners.
*
* @var array<string, mixed>
* @var array<string, array{0:boolean, 1:array<int, mixed>, 2:array<int, mixed>}>
*/
protected array $listeners = [];
}
14 changes: 7 additions & 7 deletions lib/Loop/Loop.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function setTimeout(callable $cb, float $timeout): void
{
$triggerTime = microtime(true) + $timeout;

if (!$this->timers) {
if (0 === count($this->timers)) {
phil-davis marked this conversation as resolved.
Show resolved Hide resolved
// Special case when the timers array was empty.
$this->timers[] = [$triggerTime, $cb];

Expand Down Expand Up @@ -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)) {
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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));
Expand All @@ -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;
Expand All @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Loop/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lib/Promise.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class Promise
*/
public function __construct(callable $executor = null)
{
if ($executor) {
if (is_callable($executor)) {
$executor(
[$this, 'fulfill'],
[$this, 'reject']
Expand Down
2 changes: 1 addition & 1 deletion lib/Promise/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 18 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,21 @@ 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<mixed, mixed, mixed, TReturn> and Generator will always evaluate to true.$#"
Copy link
Member

@staabm staabm Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might make sense to configure phpstan with treatPhpDocTypesAsCertain: false since we have defensive code which makes double sure that types in phpdocs are really passed as expected

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that locally, and it does not make any difference for this message.
Somehow PHPstan "knows" about Promises and Generator and still thinks that the check can never fail.

For now, IMO we can just keep this in ignoreErrors

count: 1
path: lib/coroutine.php
-
message: "#^.* will always evaluate to true\\.$#"
path: tests/*
-
message: "#^.* will always evaluate to false\\.$#"
path: tests/*
4 changes: 2 additions & 2 deletions tests/Event/CoroutineTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions tests/Event/Loop/FunctionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions tests/Event/Loop/LoopTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions tests/Event/Promise/PromiseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
}
}
Expand Down