Add PHPStan to test environment

reactphp · May 22, 2024 · 9234da0 · 9234da0
1 parent cf107f5
commit 9234da0
Show file tree

Hide file tree

Showing 16 changed files with 112 additions and 27 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -2,6 +2,7 @@
 /.github/ 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
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -47,3 +47,26 @@ jobs:
       - run: composer install
       - run: vendor/bin/phpunit --coverage-text
       - run: time php examples/91-benchmark-throughput.php
+
+  PHPStan:
+    name: PHPStan (PHP ${{ matrix.php }})
+    runs-on: ubuntu-22.04
+    strategy:
+      matrix:
+        php:
+          - 8.3
+          - 8.2
+          - 8.1
+          - 8.0
+          - 7.4
+          - 7.3
+          - 7.2
+          - 7.1
+    steps:
+      - uses: actions/checkout@v4
+      - uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php }}
+          coverage: none
+      - run: composer install
+      - run: vendor/bin/phpstan
diff --git a/README.md b/README.md
@@ -1243,6 +1243,12 @@ If you do not want to run these, they can simply be skipped like this:
 vendor/bin/phpunit --exclude-group internet
 ```
 
+On top of this, we use PHPStan on level 5 to ensure type safety across the project:
+
+```bash
+vendor/bin/phpstan
+```
+
 ## License
 
 MIT, see [LICENSE file](LICENSE).

diff --git a/composer.json b/composer.json
@@ -31,8 +31,9 @@
         "evenement/evenement": "^3.0 || ^2.0 || ^1.0"
     },
     "require-dev": {
-        "phpunit/phpunit": "^9.6 || ^7.5",
-        "clue/stream-filter": "~1.2"
+        "clue/stream-filter": "^1.2",
+        "phpstan/phpstan": "1.11.1 || 1.4.10",
+        "phpunit/phpunit": "^9.6 || ^7.5"
     },
     "autoload": {
         "psr-4": {

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -0,0 +1,7 @@
+parameters:
+    level: 5
+
+    paths:
+        - examples/
+        - src/
+        - tests/
diff --git a/src/DuplexResourceStream.php b/src/DuplexResourceStream.php
@@ -171,14 +171,15 @@ public function pipe(WritableStreamInterface $dest, array $options = []): Writab
     public function handleData($stream)
     {
         $error = null;
-        \set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$error) {
+        \set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$error): bool {
             $error = new \ErrorException(
                 $errstr,
                 0,
                 $errno,
                 $errfile,
                 $errline
             );
+            return true;
         });
 
         $data = \stream_get_contents($stream, $this->bufferSize);

diff --git a/src/ReadableResourceStream.php b/src/ReadableResourceStream.php
@@ -124,14 +124,15 @@ public function close(): void
     public function handleData()
     {
         $error = null;
-        \set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$error) {
+        \set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$error): bool {
             $error = new \ErrorException(
                 $errstr,
                 0,
                 $errno,
                 $errfile,
                 $errline
             );
+            return true;
         });
 
         $data = \stream_get_contents($this->stream, $this->bufferSize);

diff --git a/src/ThroughStream.php b/src/ThroughStream.php
@@ -144,6 +144,7 @@ public function write($data): bool
         }
 
         // continue writing if still writable and not paused (throttled), false otherwise
+        // @phpstan-ignore-next-line (may be false when write() causes stream to close)
         return $this->writable && !$this->paused;
     }
 
@@ -157,6 +158,7 @@ public function end($data = null): void
             $this->write($data);
 
             // return if write() already caused the stream to close
+            // @phpstan-ignore-next-line (may be false when write() causes stream to close)
             if (!$this->writable) {
                 return;
             }

diff --git a/src/WritableResourceStream.php b/src/WritableResourceStream.php
@@ -122,8 +122,9 @@ public function close(): void
     public function handleWrite()
     {
         $error = null;
-        \set_error_handler(function ($_, $errstr) use (&$error) {
+        \set_error_handler(function ($_, $errstr) use (&$error): bool {
             $error = $errstr;
+            return true;
         });
 
         if ($this->writeChunkSize === -1) {

diff --git a/tests/CompositeStreamTest.php b/tests/CompositeStreamTest.php
@@ -23,12 +23,14 @@ public function itShouldCloseReadableIfNotWritable()
         $readable
             ->expects($this->once())
             ->method('close');
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable
             ->expects($this->once())
             ->method('isWritable')
             ->willReturn(false);
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
 
@@ -44,11 +46,13 @@ public function itShouldCloseWritableIfNotReadable()
             ->expects($this->once())
             ->method('isReadable')
             ->willReturn(false);
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable
             ->expects($this->once())
             ->method('close');
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
 
@@ -64,6 +68,7 @@ public function itShouldForwardWritableCallsToWritableStream()
             ->expects($this->once())
             ->method('isReadable')
             ->willReturn(true);
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable
@@ -74,6 +79,7 @@ public function itShouldForwardWritableCallsToWritableStream()
             ->expects($this->exactly(2))
             ->method('isWritable')
             ->willReturn(true);
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
         $composite->write('foo');
@@ -94,12 +100,14 @@ public function itShouldForwardReadableCallsToReadableStream()
         $readable
             ->expects($this->once())
             ->method('resume');
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable
             ->expects($this->any())
             ->method('isWritable')
             ->willReturn(true);
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
         $composite->isReadable();
@@ -118,12 +126,14 @@ public function itShouldNotForwardResumeIfStreamIsNotWritable()
         $readable
             ->expects($this->never())
             ->method('resume');
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable
             ->expects($this->exactly(2))
             ->method('isWritable')
             ->willReturnOnConsecutiveCalls(true, false);
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
         $composite->resume();
@@ -137,6 +147,7 @@ public function endShouldDelegateToWritableWithData()
             ->expects($this->once())
             ->method('isReadable')
             ->willReturn(true);
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable
@@ -147,6 +158,7 @@ public function endShouldDelegateToWritableWithData()
             ->expects($this->once())
             ->method('end')
             ->with('foo');
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
         $composite->end('foo');
@@ -163,6 +175,7 @@ public function closeShouldCloseBothStreams()
         $readable
             ->expects($this->once())
             ->method('close');
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable
@@ -172,6 +185,7 @@ public function closeShouldCloseBothStreams()
         $writable
             ->expects($this->once())
             ->method('close');
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
         $composite->close();
@@ -231,13 +245,15 @@ public function itShouldHandlePipingCorrectly()
             ->expects($this->once())
             ->method('isReadable')
             ->willReturn(true);
+        assert($readable instanceof ReadableStreamInterface);
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable->expects($this->any())->method('isWritable')->willReturn(True);
         $writable
             ->expects($this->once())
             ->method('write')
             ->with('foo');
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
 
@@ -253,6 +269,7 @@ public function itShouldForwardPipeCallsToReadableStream()
 
         $writable = $this->createMock(WritableStreamInterface::class);
         $writable->expects($this->any())->method('isWritable')->willReturn(True);
+        assert($writable instanceof WritableStreamInterface);
 
         $composite = new CompositeStream($readable, $writable);
 
@@ -262,6 +279,7 @@ public function itShouldForwardPipeCallsToReadableStream()
             ->expects($this->once())
             ->method('write')
             ->with('foo');
+        assert($output instanceof WritableStreamInterface);
 
         $composite->pipe($output);
         $readable->emit('data', ['foo']);

diff --git a/tests/DuplexResourceStreamIntegrationTest.php b/tests/DuplexResourceStreamIntegrationTest.php
@@ -37,7 +37,7 @@ function () {
     public function testBufferReadsLargeChunks($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -73,7 +73,7 @@ public function testBufferReadsLargeChunks($condition, $loopFactory)
     public function testWriteLargeChunk($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -113,7 +113,7 @@ public function testWriteLargeChunk($condition, $loopFactory)
     public function testDoesNotEmitDataIfNothingHasBeenWritten($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -141,7 +141,7 @@ public function testDoesNotEmitDataIfNothingHasBeenWritten($condition, $loopFact
     public function testDoesNotWriteDataIfRemoteSideFromPairHasBeenClosed($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -171,7 +171,7 @@ public function testDoesNotWriteDataIfRemoteSideFromPairHasBeenClosed($condition
     public function testDoesNotWriteDataIfServerSideHasBeenClosed($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -204,7 +204,7 @@ public function testDoesNotWriteDataIfServerSideHasBeenClosed($condition, $loopF
     public function testDoesNotWriteDataIfClientSideHasBeenClosed($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -237,7 +237,7 @@ public function testDoesNotWriteDataIfClientSideHasBeenClosed($condition, $loopF
     public function testReadsSingleChunkFromProcessPipe($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -256,7 +256,7 @@ public function testReadsSingleChunkFromProcessPipe($condition, $loopFactory)
     public function testReadsMultipleChunksFromProcessPipe($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -282,7 +282,7 @@ public function testReadsMultipleChunksFromProcessPipe($condition, $loopFactory)
     public function testReadsLongChunksFromProcessPipe($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -308,7 +308,7 @@ public function testReadsLongChunksFromProcessPipe($condition, $loopFactory)
     public function testReadsNothingFromProcessPipeWithNoOutput($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $loop = $loopFactory();
@@ -328,7 +328,7 @@ public function testReadsNothingFromProcessPipeWithNoOutput($condition, $loopFac
     public function testEmptyReadShouldntFcloseStream($condition, $loopFactory)
     {
         if (true !== $condition()) {
-            return $this->markTestSkipped('Loop implementation not available');
+            $this->markTestSkipped('Loop implementation not available');
         }
 
         $server = stream_socket_server('tcp://127.0.0.1:0');