Merge pull request #200 from clue-labs/phpstan

Add PHPStan to test environment

Author: SimonFrings

.gitattributes

@@ -1,10 +1,12 @@
 # exclude dev files from export to reduce archive download size
 /.gitattributes export-ignore
+/.github/ISSUE_TEMPLATE/ export-ignore
 /.github/workflows/ export-ignore
 /.gitignore export-ignore
 /docs/ export-ignore
 /examples/ export-ignore
 /mkdocs.yml export-ignore
+/phpstan.neon.dist export-ignore
 /phpunit.xml.dist export-ignore
 /phpunit.xml.legacy export-ignore
 /tests/ export-ignore

.github/workflows/ci.yml

@@ -34,6 +34,28 @@ jobs:
       - run: vendor/bin/phpunit --coverage-text --stderr -c phpunit.xml.legacy
         if: ${{ matrix.php < 7.3 }}
 
+  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
+
   Built-in-webserver:
     name: Built-in webserver (PHP ${{ matrix.php }})
     runs-on: ubuntu-22.04

composer.json

@@ -18,6 +18,7 @@
         "react/promise": "^3 || ^2.7"
     },
     "require-dev": {
+        "phpstan/phpstan": "1.8.10 || 1.4.10",
         "phpunit/phpunit": "^9.5 || ^7.5",
         "psr/container": "^2 || ^1"
     },

examples/index.php

@@ -118,7 +118,7 @@
    return new React\Http\Message\Response(
         React\Http\Message\Response::STATUS_OK,
         [
-            'Content-Length' => 5,
+            'Content-Length' => '5',
             'Content-Type' => 'text/plain; charset=utf-8',
             'X-Is-Head' => 'true'
         ]
@@ -161,7 +161,7 @@
             React\Http\Message\Response::STATUS_NOT_MODIFIED,
             [
                 'ETag' => $etag,
-                'Content-Length' => strlen($etag) - 1
+                'Content-Length' => (string) (strlen($etag) - 1)
             ]
         );
     }
@@ -193,7 +193,7 @@
 $app->get('/error/yield', function () {
     yield null;
 });
-$app->get('/error/class', 'Acme\Http\UnknownDeleteUserController');
+$app->get('/error/class', 'Acme\Http\UnknownDeleteUserController'); // @phpstan-ignore-line
 
 // OPTIONS *
 $app->options('', function () {

phpstan.neon.dist

@@ -0,0 +1,17 @@
+parameters:
+    level: 5
+
+    paths:
+        - examples/
+        - src/
+        - tests/
+
+    reportUnmatchedIgnoredErrors: false
+    ignoreErrors:
+        # ignore generic usage like `PromiseInterface<ResponseInterface>` until fixed upstream
+        - '/^PHPDoc tag @return contains generic type React\\Promise\\PromiseInterface<Psr\\Http\\Message\\ResponseInterface> but interface React\\Promise\\PromiseInterface is not generic\.$/'
+        # ignore unknown `Fiber` class (PHP 8.1+)
+        - '/^Instantiated class Fiber not found\.$/'
+        - '/^Call to method (start|isTerminated|getReturn)\(\) on an unknown class Fiber\.$/'
+        # ignore incomplete type information for mocks in legacy PHPUnit 7.5
+        - '/^Parameter #\d+ \$.+ of class .+ constructor expects .+, PHPUnit\\Framework\\MockObject\\MockObject given\.$/'

src/App.php

@@ -286,7 +286,7 @@ private function runLoop()
         } while (true);
 
         // remove signal handlers when loop stops (if registered)
-        Loop::removeSignal(\defined('SIGINT') ? \SIGINT : 2, $f1);
+        Loop::removeSignal(\defined('SIGINT') ? \SIGINT : 2, $f1 ?? 'printf');
         Loop::removeSignal(\defined('SIGTERM') ? \SIGTERM : 15, $f2 ?? 'printf');
     }
 
@@ -309,11 +309,12 @@ private function runOnce()
 
     /**
      * @param ServerRequestInterface $request
-     * @return ResponseInterface|PromiseInterface<ResponseInterface,void>
+     * @return ResponseInterface|PromiseInterface<ResponseInterface>
      *     Returns a response or a Promise which eventually fulfills with a
      *     response. This method never throws or resolves a rejected promise.
      *     If the request can not be routed or the handler fails, it will be
      *     turned into a valid error response before returning.
+     * @throws void
      */
     private function handleRequest(ServerRequestInterface $request)
     {

src/Container.php

@@ -13,9 +13,10 @@ class Container
     /** @var array<string,object|callable():(object|scalar|null)|scalar|null>|ContainerInterface */
     private $container;
 
-    /** @var array<string,callable():(object|scalar|null) | object | scalar | null>|ContainerInterface $loader */
+    /** @param array<string,callable():(object|scalar|null) | object | scalar | null>|ContainerInterface $loader */
     public function __construct($loader = [])
     {
+        /** @var mixed $loader explicit type check for mixed if user ignores parameter type */
         if (!\is_array($loader) && !$loader instanceof ContainerInterface) {
             throw new \TypeError(
                 'Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . (\is_object($loader) ? get_class($loader) : gettype($loader)) . ' given'
@@ -233,6 +234,7 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool
         $hasDefault = $parameter->isDefaultValueAvailable() || ((!$type instanceof \ReflectionNamedType || $type->getName() !== 'mixed') && $parameter->allowsNull());
 
         // abort for union types (PHP 8.0+) and intersection types (PHP 8.1+)
+        // @phpstan-ignore-next-line for PHP < 8
         if ($type instanceof \ReflectionUnionType || $type instanceof \ReflectionIntersectionType) { // @codeCoverageIgnoreStart
             if ($hasDefault) {
                 return $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null;

src/ErrorHandler.php

@@ -22,7 +22,7 @@ public function __construct()
     }
 
     /**
-     * @return ResponseInterface|PromiseInterface<ResponseInterface,void>|\Generator
+     * @return ResponseInterface|PromiseInterface<ResponseInterface>|\Generator
      *     Returns a response, a Promise which eventually fulfills with a
      *     response or a Generator which eventually returns a response. This
      *     method never throws or resolves a rejected promise. If the next

src/Io/FiberHandler.php

@@ -21,7 +21,7 @@
 class FiberHandler
 {
     /**
-     * @return ResponseInterface|PromiseInterface<ResponseInterface,void>|\Generator
+     * @return ResponseInterface|PromiseInterface<ResponseInterface>|\Generator
      *     Returns a `ResponseInterface` from the next request handler in the
      *     chain. If the next request handler returns immediately, this method
      *     will return immediately. If the next request handler suspends the
@@ -42,15 +42,20 @@ public function __invoke(ServerRequestInterface $request, callable $next): mixed
             $response = $next($request);
             assert($response instanceof ResponseInterface || $response instanceof PromiseInterface || $response instanceof \Generator);
 
+            // if the next request handler returns immediately, the fiber can terminate immediately without using a Deferred
+            // if the next request handler suspends the fiber, we only reach this point after resuming the fiber, so the code below will have assigned a Deferred
+            /** @var ?Deferred $deferred */
             if ($deferred !== null) {
                 $deferred->resolve($response);
             }
 
             return $response;
         });
 
+        /** @throws void because the next handler will always be an `ErrorHandler` */
         $fiber->start();
         if ($fiber->isTerminated()) {
+            /** @throws void because fiber is known to have terminated successfully */
             return $fiber->getReturn();
         }
 

src/Io/RouteHandler.php

@@ -76,28 +76,37 @@ public function map(array $methods, string $route, $handler, ...$handlers): void
     public function __invoke(ServerRequestInterface $request)
     {
         if ($request->getRequestTarget()[0] !== '/' && $request->getRequestTarget() !== '*') {
-            return $this->errorHandler->requestProxyUnsupported($request);
+            return $this->errorHandler->requestProxyUnsupported();
         }
 
         if ($this->routeDispatcher === null) {
             $this->routeDispatcher = new RouteDispatcher($this->routeCollector->getData());
         }
 
         $routeInfo = $this->routeDispatcher->dispatch($request->getMethod(), $request->getUri()->getPath());
-        switch ($routeInfo[0]) {
-            case \FastRoute\Dispatcher::NOT_FOUND:
-                return $this->errorHandler->requestNotFound($request);
-            case \FastRoute\Dispatcher::METHOD_NOT_ALLOWED:
-                return $this->errorHandler->requestMethodNotAllowed($routeInfo[1]);
-            case \FastRoute\Dispatcher::FOUND:
-                $handler = $routeInfo[1];
-                $vars = $routeInfo[2];
-
-                foreach ($vars as $key => $value) {
-                    $request = $request->withAttribute($key, rawurldecode($value));
-                }
-
-                return $handler($request);
+        assert(\is_array($routeInfo) && isset($routeInfo[0]));
+
+        // happy path: matching route found, assign route attributes and invoke request handler
+        if ($routeInfo[0] === \FastRoute\Dispatcher::FOUND) {
+            $handler = $routeInfo[1];
+            $vars = $routeInfo[2];
+
+            foreach ($vars as $key => $value) {
+                $request = $request->withAttribute($key, rawurldecode($value));
+            }
+
+            return $handler($request);
         }
-    } // @codeCoverageIgnore
+
+        // no matching route found: report error `404 Not Found`
+        if ($routeInfo[0] === \FastRoute\Dispatcher::NOT_FOUND) {
+            return $this->errorHandler->requestNotFound();
+        }
+
+        // unexpected request method for route: report error `405 Method Not Allowed`
+        assert($routeInfo[0] === \FastRoute\Dispatcher::METHOD_NOT_ALLOWED);
+        assert(\is_array($routeInfo[1]) && \count($routeInfo[1]) > 0);
+
+        return $this->errorHandler->requestMethodNotAllowed($routeInfo[1]);
+    }
 }