Merge pull request #201 from clue-labs/phpstan-max

Improve type definitions and update to PHPStan level `max`

Author: SimonFrings

examples/index.php

@@ -21,8 +21,11 @@
         return htmlspecialchars_decode(htmlspecialchars($str, ENT_SUBSTITUTE | ENT_DISALLOWED, 'utf-8'));
     };
 
+    $name = $request->getAttribute('name');
+    assert(is_string($name));
+
     return React\Http\Message\Response::plaintext(
-        "Hello " . $escape($request->getAttribute('name')) . "!\n"
+        "Hello " . $escape($name) . "!\n"
     );
 });
 
@@ -63,6 +66,7 @@
     ob_start();
     var_dump($request);
     $info = ob_get_clean();
+    assert(is_string($info));
 
     if (PHP_SAPI !== 'cli' && (!function_exists('xdebug_is_enabled') || !xdebug_is_enabled())) {
         $info = htmlspecialchars($info, 0, 'utf-8');
@@ -201,9 +205,10 @@
 });
 
 $app->get('/location/{status:\d+}', function (Psr\Http\Message\ServerRequestInterface $request) {
-    $statusCode = (int) $request->getAttribute('status');
+    $statusCode = $request->getAttribute('status');
+    assert(is_string($statusCode) && is_numeric($statusCode));
 
-    return new React\Http\Message\Response($statusCode, ['Location' => '/foobar']);
+    return new React\Http\Message\Response((int) $statusCode, ['Location' => '/foobar']);
 });
 
 $app->run();

phpstan.neon.dist

@@ -1,5 +1,5 @@
 parameters:
-    level: 5
+    level: max
 
     paths:
         - examples/
@@ -14,4 +14,4 @@ parameters:
         - '/^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\.$/'
+        - '/^Parameter #\d+ .+ of .+ expects .+, PHPUnit\\Framework\\MockObject\\MockObject given\.$/'

src/AccessLogHandler.php

@@ -94,7 +94,7 @@ private function log(ServerRequestInterface $request, ResponseInterface $respons
 
     private function escape(string $s): string
     {
-        return preg_replace_callback('/[\x00-\x1F\x7F-\xFF"\\\\]+/', function (array $m) {
+        return (string) preg_replace_callback('/[\x00-\x1F\x7F-\xFF"\\\\]+/', function (array $m) {
             return str_replace('%', '\x', rawurlencode($m[0]));
         }, $s);
     }

src/App.php

@@ -220,7 +220,7 @@ public function redirect(string $route, string $target, int $code = Response::ST
         $this->any($route, new RedirectHandler($target, $code));
     }
 
-    public function run()
+    public function run(): void
     {
         if (\PHP_SAPI === 'cli') {
             $this->runLoop();
@@ -229,7 +229,7 @@ public function run()
         }
     }
 
-    private function runLoop()
+    private function runLoop(): void
     {
         $http = new HttpServer(function (ServerRequestInterface $request) {
             return $this->handleRequest($request);
@@ -240,7 +240,7 @@ private function runLoop()
         $socket = new SocketServer($listen);
         $http->listen($socket);
 
-        $this->sapi->log('Listening on ' . \str_replace('tcp:', 'http:', $socket->getAddress()));
+        $this->sapi->log('Listening on ' . \str_replace('tcp:', 'http:', (string) $socket->getAddress()));
 
         $http->on('error', function (\Exception $e) {
             $orig = $e;
@@ -290,7 +290,7 @@ private function runLoop()
         Loop::removeSignal(\defined('SIGTERM') ? \SIGTERM : 15, $f2 ?? 'printf');
     }
 
-    private function runOnce()
+    private function runOnce(): void
     {
         $request = $this->sapi->requestFromGlobals();
 
@@ -319,11 +319,14 @@ private function runOnce()
     private function handleRequest(ServerRequestInterface $request)
     {
         $response = ($this->handler)($request);
+        assert($response instanceof ResponseInterface || $response instanceof PromiseInterface || $response instanceof \Generator);
+
         if ($response instanceof \Generator) {
             if ($response->valid()) {
                 $response = $this->coroutine($response);
             } else {
                 $response = $response->getReturn();
+                assert($response instanceof ResponseInterface);
             }
         }
 
@@ -341,6 +344,8 @@ private function coroutine(\Generator $generator): PromiseInterface
             }
 
             $promise = $generator->current();
+            assert($promise instanceof PromiseInterface);
+
             $promise->then(function ($value) use ($generator, $next) {
                 $generator->send($value);
                 $next();

src/Container.php

@@ -34,6 +34,7 @@ public function __construct($loader = [])
         $this->container = $loader;
     }
 
+    /** @return mixed */
     public function __invoke(ServerRequestInterface $request, callable $next = null)
     {
         if ($next === null) {
@@ -117,6 +118,7 @@ public function getAccessLogHandler(): AccessLogHandler
     {
         if ($this->container instanceof ContainerInterface) {
             if ($this->container->has(AccessLogHandler::class)) {
+                // @phpstan-ignore-next-line method return type will ensure correct type or throw `TypeError`
                 return $this->container->get(AccessLogHandler::class);
             } else {
                 return new AccessLogHandler();
@@ -130,6 +132,7 @@ public function getErrorHandler(): ErrorHandler
     {
         if ($this->container instanceof ContainerInterface) {
             if ($this->container->has(ErrorHandler::class)) {
+                // @phpstan-ignore-next-line method return type will ensure correct type or throw `TypeError`
                 return $this->container->get(ErrorHandler::class);
             } else {
                 return new ErrorHandler();
@@ -139,22 +142,25 @@ public function getErrorHandler(): ErrorHandler
     }
 
     /**
-     * @template T
+     * @template T of object
      * @param class-string<T> $name
      * @return T
      * @throws \BadMethodCallException if object of type $name can not be loaded
      */
     private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) */
     {
+        assert(\is_array($this->container));
+
         if (\array_key_exists($name, $this->container)) {
             if (\is_string($this->container[$name])) {
                 if ($depth < 1) {
                     throw new \BadMethodCallException('Factory for ' . $name . ' is recursive');
                 }
 
+                // @phpstan-ignore-next-line because type of container value is explicitly checked after getting here
                 $value = $this->loadObject($this->container[$name], $depth - 1);
                 if (!$value instanceof $name) {
-                    throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . (is_object($value) ? get_class($value) : gettype($value)));
+                    throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . \get_class($value));
                 }
 
                 $this->container[$name] = $value;
@@ -171,6 +177,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+)
                         throw new \BadMethodCallException('Factory for ' . $name . ' is recursive');
                     }
 
+                    // @phpstan-ignore-next-line because type of container value is explicitly checked after getting here
                     $value = $this->loadObject($value, $depth - 1);
                 }
                 if (!$value instanceof $name) {
@@ -210,10 +217,14 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+)
         $params = $ctor === null ? [] : $this->loadFunctionParams($ctor, $depth, false);
 
         // instantiate with list of parameters
+        // @phpstan-ignore-next-line because `$class->newInstance()` is known to return `T`
         return $this->container[$name] = $params === [] ? new $name() : $class->newInstance(...$params);
     }
 
-    /** @throws \BadMethodCallException if either parameter can not be loaded */
+    /**
+     * @return list<mixed>
+     * @throws \BadMethodCallException if either parameter can not be loaded
+     */
     private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $depth, bool $allowVariables): array
     {
         $params = [];
@@ -230,6 +241,8 @@ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $
      */
     private function loadParameter(\ReflectionParameter $parameter, int $depth, bool $allowVariables) /*: mixed (PHP 8.0+) */
     {
+        assert(\is_array($this->container));
+
         $type = $parameter->getType();
         $hasDefault = $parameter->isDefaultValueAvailable() || ((!$type instanceof \ReflectionNamedType || $type->getName() !== 'mixed') && $parameter->allowsNull());
 
@@ -277,6 +290,7 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool
             throw new \BadMethodCallException(self::parameterError($parameter) . ' is recursive');
         }
 
+        // @phpstan-ignore-next-line because `$type->getName()` is a `class-string` by definition
         return $this->loadObject($type->getName(), $depth - 1);
     }
 
@@ -286,19 +300,21 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool
      */
     private function loadVariable(string $name, string $type, bool $nullable, int $depth) /*: object|string|int|float|bool|null (PHP 8.0+) */
     {
-        assert(\array_key_exists($name, $this->container) || isset($_SERVER[$name]));
+        assert(\is_array($this->container) && (\array_key_exists($name, $this->container) || isset($_SERVER[$name])));
 
         if (($this->container[$name] ?? null) instanceof \Closure) {
             if ($depth < 1) {
                 throw new \BadMethodCallException('Container variable $' . $name . ' is recursive');
             }
 
             // build list of factory parameters based on parameter types
-            $closure = new \ReflectionFunction($this->container[$name]);
+            $factory = $this->container[$name];
+            assert($factory instanceof \Closure);
+            $closure = new \ReflectionFunction($factory);
             $params = $this->loadFunctionParams($closure, $depth - 1, true);
 
             // invoke factory with list of parameters
-            $value = $params === [] ? ($this->container[$name])() : ($this->container[$name])(...$params);
+            $value = $params === [] ? $factory() : $factory(...$params);
 
             if (!\is_object($value) && !\is_scalar($value) && $value !== null) {
                 throw new \BadMethodCallException('Container variable $' . $name . ' expected type object|scalar|null from factory, but got ' . \gettype($value));

src/ErrorHandler.php

@@ -120,7 +120,10 @@ public function requestNotFound(): ResponseInterface
         );
     }
 
-    /** @internal */
+    /**
+     * @internal
+     * @param list<string> $allowedMethods
+     */
     public function requestMethodNotAllowed(array $allowedMethods): ResponseInterface
     {
         $methods = \implode('/', \array_map(function (string $method) { return '<code>' . $method . '</code>'; }, $allowedMethods));
@@ -155,6 +158,7 @@ private function errorInvalidException(\Throwable $e): ResponseInterface
         );
     }
 
+    /** @param mixed $value */
     private function errorInvalidResponse($value): ResponseInterface
     {
         return $this->htmlResponse(
@@ -165,6 +169,7 @@ private function errorInvalidResponse($value): ResponseInterface
         );
     }
 
+    /** @param mixed $value */
     private function errorInvalidCoroutine($value, string $file, int $line): ResponseInterface
     {
         $where = ' near or before '. $this->where($file, $line) . '.';
@@ -192,6 +197,7 @@ private function htmlResponse(int $statusCode, string $title, string ...$info):
         );
     }
 
+    /** @param mixed $value */
     private function describeType($value): string
     {
         if ($value === null) {

src/FilesystemHandler.php

@@ -4,11 +4,13 @@
 
 use FrameworkX\Io\HtmlHandler;
 use FrameworkX\Io\RedirectHandler;
+use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use React\Http\Message\Response;
 
 class FilesystemHandler
 {
+    /** @var string */
     private $root;
 
     /**
@@ -59,9 +61,10 @@ public function __construct(string $root)
         $this->html = new HtmlHandler();
     }
 
-    public function __invoke(ServerRequestInterface $request)
+    public function __invoke(ServerRequestInterface $request): ResponseInterface
     {
         $local = $request->getAttribute('path', '');
+        assert(\is_string($local));
         $path = \rtrim($this->root . '/' . $local, '/');
 
         // local path should not contain "./", "../", "//" or null bytes or start with slash
@@ -80,6 +83,7 @@ public function __invoke(ServerRequestInterface $request)
             }
 
             $files = \scandir($path);
+            // @phpstan-ignore-next-line TODO handle error if directory can not be accessed
             foreach ($files as $file) {
                 if ($file === '.' || $file === '..') {
                     continue;
@@ -117,7 +121,7 @@ public function __invoke(ServerRequestInterface $request)
             return new Response(
                 Response::STATUS_OK,
                 $headers,
-                \file_get_contents($path)
+                \file_get_contents($path) // @phpstan-ignore-line TODO handle error if file can not be accessed
             );
         } else {
             return $this->errorHandler->requestNotFound();

src/Io/FiberHandler.php

@@ -56,6 +56,7 @@ public function __invoke(ServerRequestInterface $request, callable $next): mixed
         $fiber->start();
         if ($fiber->isTerminated()) {
             /** @throws void because fiber is known to have terminated successfully */
+            /** @var ResponseInterface|PromiseInterface|\Generator */
             return $fiber->getReturn();
         }
 

src/Io/HtmlHandler.php

@@ -50,12 +50,12 @@ public function statusResponse(int $statusCode, string $title, string $subtitle,
 
     public function escape(string $s): string
     {
-        return \preg_replace_callback(
+        return (string) \preg_replace_callback(
             '/[\x00-\x1F]+/',
             function (array $match): string {
                 return '<span>' . \addcslashes($match[0], "\x00..\xff") . '</span>';
             },
-            \preg_replace(
+            (string) \preg_replace(
                 '/(^| ) |(?: $)/',
                 '$1&nbsp;',
                 \htmlspecialchars($s, \ENT_NOQUOTES | \ENT_SUBSTITUTE | \ENT_DISALLOWED, 'utf-8')

src/Io/MiddlewareHandler.php

@@ -9,20 +9,24 @@
  */
 class MiddlewareHandler
 {
+    /** @var list<callable> $handlers */
     private $handlers;
 
+    /** @param list<callable> $handlers */
     public function __construct(array $handlers)
     {
         assert(count($handlers) >= 2);
 
         $this->handlers = $handlers;
     }
 
+    /** @return mixed */
     public function __invoke(ServerRequestInterface $request)
     {
         return $this->call($request, 0);
     }
 
+    /** @return mixed */
     private function call(ServerRequestInterface $request, int $position)
     {
         if (!isset($this->handlers[$position + 2])) {