refactor: fix phpstan errors in ResponseTrait (#9591)

Author: paulbalandan

system/API/ResponseTrait.php

@@ -13,17 +13,21 @@
 
 namespace CodeIgniter\API;
 
+use CodeIgniter\Format\Format;
 use CodeIgniter\Format\FormatterInterface;
 use CodeIgniter\HTTP\IncomingRequest;
+use CodeIgniter\HTTP\RequestInterface;
 use CodeIgniter\HTTP\ResponseInterface;
 
 /**
  * Provides common, more readable, methods to provide
  * consistent HTTP responses under a variety of common
  * situations when working as an API.
  *
- * @property bool $stringAsHtml Whether to treat string data as HTML in JSON response.
- *                              Setting `true` is only for backward compatibility.
+ * @property RequestInterface  $request
+ * @property ResponseInterface $response
+ * @property bool              $stringAsHtml Whether to treat string data as HTML in JSON response.
+ *                                           Setting `true` is only for backward compatibility.
  */
 trait ResponseTrait
 {
@@ -84,7 +88,7 @@ trait ResponseTrait
      * Provides a single, simple method to return an API response, formatted
      * to match the requested format, with proper content-type and status code.
      *
-     * @param array|string|null $data
+     * @param array<string, mixed>|string|null $data
      *
      * @return ResponseInterface
      */
@@ -118,9 +122,9 @@ protected function respond($data = null, ?int $status = null, string $message =
     /**
      * Used for generic failures that no custom methods exist for.
      *
-     * @param array|string $messages
-     * @param int          $status   HTTP status code
-     * @param string|null  $code     Custom, API-specific, error code
+     * @param list<string>|string $messages
+     * @param int                 $status   HTTP status code
+     * @param string|null         $code     Custom, API-specific, error code
      *
      * @return ResponseInterface
      */
@@ -146,7 +150,7 @@ protected function fail($messages, int $status = 400, ?string $code = null, stri
     /**
      * Used after successfully creating a new resource.
      *
-     * @param array|string|null $data
+     * @param array<string, mixed>|string|null $data
      *
      * @return ResponseInterface
      */
@@ -158,7 +162,7 @@ protected function respondCreated($data = null, string $message = '')
     /**
      * Used after a resource has been successfully deleted.
      *
-     * @param array|string|null $data
+     * @param array<string, mixed>|string|null $data
      *
      * @return ResponseInterface
      */
@@ -170,7 +174,7 @@ protected function respondDeleted($data = null, string $message = '')
     /**
      * Used after a resource has been successfully updated.
      *
-     * @param array|string|null $data
+     * @param array<string, mixed>|string|null $data
      *
      * @return ResponseInterface
      */
@@ -287,15 +291,17 @@ protected function failServerError(string $description = 'Internal Server Error'
      * Handles formatting a response. Currently, makes some heavy assumptions
      * and needs updating! :)
      *
-     * @param array|string|null $data
+     * @param array<string, mixed>|string|null $data
      *
      * @return string|null
      */
     protected function format($data = null)
     {
+        /** @var Format $format */
         $format = service('format');
 
-        $mime = ($this->format === null) ? $format->getConfig()->supportedResponseFormats[0]
+        $mime = $this->format === null
+            ? $format->getConfig()->supportedResponseFormats[0]
             : "application/{$this->format}";
 
         // Determine correct response type through content negotiation if not explicitly declared
@@ -313,14 +319,10 @@ protected function format($data = null)
         $this->response->setContentType($mime);
 
         // if we don't have a formatter, make one
-        if (! isset($this->formatter)) {
-            // if no formatter, use the default
-            $this->formatter = $format->getFormatter($mime);
-        }
+        $this->formatter ??= $format->getFormatter($mime);
 
         $asHtml = $this->stringAsHtml ?? false;
 
-        // Returns as HTML.
         if (
             ($mime === 'application/json' && $asHtml && is_string($data))
             || ($mime !== 'application/json' && is_string($data))
@@ -338,6 +340,7 @@ protected function format($data = null)
         if ($mime !== 'application/json') {
             // Recursively convert objects into associative arrays
             // Conversion not required for JSONFormatter
+            /** @var array<string, mixed>|string|null $data */
             $data = json_decode(json_encode($data), true);
         }
 
@@ -353,7 +356,7 @@ protected function format($data = null)
      */
     protected function setResponseFormat(?string $format = null)
     {
-        $this->format = ($format === null) ? null : strtolower($format);
+        $this->format = $format === null ? null : strtolower($format);
 
         return $this;
     }

tests/system/API/ResponseTraitTest.php

@@ -17,13 +17,16 @@
 use CodeIgniter\Format\FormatterInterface;
 use CodeIgniter\Format\JSONFormatter;
 use CodeIgniter\Format\XMLFormatter;
+use CodeIgniter\HTTP\RequestInterface;
+use CodeIgniter\HTTP\ResponseInterface;
 use CodeIgniter\HTTP\SiteURI;
 use CodeIgniter\HTTP\UserAgent;
 use CodeIgniter\Test\CIUnitTestCase;
 use CodeIgniter\Test\Mock\MockIncomingRequest;
 use CodeIgniter\Test\Mock\MockResponse;
 use Config\App;
 use Config\Cookie;
+use Config\Services;
 use PHPUnit\Framework\Attributes\Group;
 use stdClass;
 
@@ -82,6 +85,12 @@ private function createCookieConfig(): Cookie
         return $cookie;
     }
 
+    /**
+     * @param array<string, string> $userHeaders
+     *
+     * @phpstan-assert RequestInterface  $this->request
+     * @phpstan-assert ResponseInterface $this->response
+     */
     private function createRequestAndResponse(string $routePath = '', array $userHeaders = []): void
     {
         $config = $this->createAppConfig();
@@ -97,29 +106,28 @@ private function createRequestAndResponse(string $routePath = '', array $userHea
             $this->response = new MockResponse($config);
         }
 
-        // Insert headers into request.
-        $headers = [
-            'Accept' => 'text/html',
-        ];
-        $headers = array_merge($headers, $userHeaders);
+        $headers = array_merge(['Accept' => 'text/html'], $userHeaders);
 
         foreach ($headers as $key => $value) {
             $this->request->setHeader($key, $value);
         }
     }
 
+    /**
+     * @param array<string, string> $userHeaders
+     */
     protected function makeController(string $routePath = '', array $userHeaders = []): object
     {
         $this->createRequestAndResponse($routePath, $userHeaders);
 
-        // Create the controller class finally.
         return new class ($this->request, $this->response, $this->formatter) {
             use ResponseTrait;
 
-            protected $formatter;
-
-            public function __construct(protected $request, protected $response, $formatter)
-            {
+            public function __construct(
+                protected RequestInterface $request,
+                protected ResponseInterface $response,
+                ?FormatterInterface $formatter,
+            ) {
                 $this->formatter = $formatter;
             }
 
@@ -173,11 +181,13 @@ public function testNoFormatterWithStringAsHtmlTrue(): void
         $controller = new class ($this->request, $this->response, $this->formatter) {
             use ResponseTrait;
 
-            protected $formatter;
             protected bool $stringAsHtml = true;
 
-            public function __construct(protected $request, protected $response, $formatter)
-            {
+            public function __construct(
+                protected RequestInterface $request,
+                protected ResponseInterface $response,
+                ?FormatterInterface $formatter,
+            ) {
                 $this->formatter = $formatter;
             }
         };
@@ -291,11 +301,13 @@ public function testRespondSetsCorrectBodyAndStatusWithStringAsHtmlTrue(): void
         $controller = new class ($this->request, $this->response, $this->formatter) {
             use ResponseTrait;
 
-            protected $formatter;
             protected bool $stringAsHtml = true;
 
-            public function __construct(protected $request, protected $response, $formatter)
-            {
+            public function __construct(
+                protected RequestInterface $request,
+                protected ResponseInterface $response,
+                ?FormatterInterface $formatter,
+            ) {
                 $this->formatter = $formatter;
             }
         };
@@ -546,8 +558,8 @@ public function testValidContentTypes(): void
 
     private function tryValidContentType(string $mimeType, string $contentType): void
     {
-        $original                = $_SERVER;
-        $_SERVER['CONTENT_TYPE'] = $mimeType;
+        $originalContentType = Services::superglobals()->server('CONTENT_TYPE') ?? '';
+        Services::superglobals()->setServer('CONTENT_TYPE', $mimeType);
 
         $this->makeController('', ['Accept' => $mimeType]);
         $this->assertSame(
@@ -563,7 +575,7 @@ private function tryValidContentType(string $mimeType, string $contentType): voi
             'Response header pre-response...',
         );
 
-        $_SERVER = $original;
+        Services::superglobals()->setServer('CONTENT_TYPE', $originalContentType);
     }
 
     public function testValidResponses(): void
@@ -609,9 +621,11 @@ public function testFormatByRequestNegotiateIfFormatIsNotJsonOrXML(): void
         $controller = new class ($request, $response) {
             use ResponseTrait;
 
-            public function __construct(protected $request, protected $response)
-            {
-                $this->format = 'txt';
+            public function __construct(
+                protected RequestInterface $request,
+                protected ResponseInterface $response,
+            ) {
+                $this->format = 'txt'; // @phpstan-ignore assign.propertyType (needed for testing)
             }
         };
 
@@ -659,6 +673,9 @@ public function testXMLResponseFormat(): void
         $this->assertSame($xmlFormatter->format($data), $this->response->getXML());
     }
 
+    /**
+     * @param list<mixed> $args
+     */
     private function invoke(object $controller, string $method, array $args = []): object
     {
         $method = self::getPrivateMethodInvoker($controller, $method);

utils/phpstan-baseline/assign.propertyType.neon

@@ -1,4 +1,4 @@
-# total 29 errors
+# total 28 errors
 
 parameters:
     ignoreErrors:
@@ -7,11 +7,6 @@ parameters:
             count: 1
             path: ../../system/Controller.php
 
-        -
-            message: '#^Property class@anonymous/tests/system/API/ResponseTraitTest\.php\:609\:\:\$format \(''html''\|''json''\|''xml''\|null\) does not accept ''txt''\.$#'
-            count: 1
-            path: ../../tests/system/API/ResponseTraitTest.php
-
         -
             message: '#^Property CodeIgniter\\Commands\\Utilities\\Routes\\FilterFinderTest\:\:\$response \(CodeIgniter\\HTTP\\Response\) does not accept CodeIgniter\\HTTP\\ResponseInterface\.$#'
             count: 1

utils/phpstan-baseline/codeigniter.superglobalAccessAssign.neon

@@ -1,4 +1,4 @@
-# total 582 errors
+# total 581 errors
 
 parameters:
     ignoreErrors:
@@ -17,11 +17,6 @@ parameters:
             count: 3
             path: ../../system/HTTP/IncomingRequest.php
 
-        -
-            message: '#^Assigning string directly on offset ''CONTENT_TYPE'' of \$_SERVER is discouraged\.$#'
-            count: 1
-            path: ../../tests/system/API/ResponseTraitTest.php
-
         -
             message: '#^Assigning 3 directly on offset ''argc'' of \$_SERVER is discouraged\.$#'
             count: 1

utils/phpstan-baseline/loader.neon

@@ -1,4 +1,4 @@
-# total 3145 errors
+# total 3075 errors
 includes:
     - argument.type.neon
     - assign.propertyType.neon