Skip to content

Commit 88ec911

Browse files
paulbalandanpaulbalandan
authored
refactor: fix phpstan errors in ResponseTrait (codeigniter4#9591)
1 parent 113a0fd commit 88ec911

8 files changed

Lines changed: 65 additions & 380 deletions

‎system/API/ResponseTrait.php‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,21 @@
1313

1414
namespace CodeIgniter\API;
1515

16+
use CodeIgniter\Format\Format;
1617
use CodeIgniter\Format\FormatterInterface;
1718
use CodeIgniter\HTTP\IncomingRequest;
19+
use CodeIgniter\HTTP\RequestInterface;
1820
use CodeIgniter\HTTP\ResponseInterface;
1921

2022
/**
2123
* Provides common, more readable, methods to provide
2224
* consistent HTTP responses under a variety of common
2325
* situations when working as an API.
2426
*
25-
* @property bool $stringAsHtml Whether to treat string data as HTML in JSON response.
26-
* Setting `true` is only for backward compatibility.
27+
* @property RequestInterface $request
28+
* @property ResponseInterface $response
29+
* @property bool $stringAsHtml Whether to treat string data as HTML in JSON response.
30+
* Setting `true` is only for backward compatibility.
2731
*/
2832
trait ResponseTrait
2933
{
@@ -84,7 +88,7 @@ trait ResponseTrait
8488
* Provides a single, simple method to return an API response, formatted
8589
* to match the requested format, with proper content-type and status code.
8690
*
87-
* @param array|string|null $data
91+
* @param array<string, mixed>|string|null $data
8892
*
8993
* @return ResponseInterface
9094
*/
@@ -118,9 +122,9 @@ protected function respond($data = null, ?int $status = null, string $message =
118122
/**
119123
* Used for generic failures that no custom methods exist for.
120124
*
121-
* @param array|string $messages
122-
* @param int $status HTTP status code
123-
* @param string|null $code Custom, API-specific, error code
125+
* @param list<string>|string $messages
126+
* @param int $status HTTP status code
127+
* @param string|null $code Custom, API-specific, error code
124128
*
125129
* @return ResponseInterface
126130
*/
@@ -146,7 +150,7 @@ protected function fail($messages, int $status = 400, ?string $code = null, stri
146150
/**
147151
* Used after successfully creating a new resource.
148152
*
149-
* @param array|string|null $data
153+
* @param array<string, mixed>|string|null $data
150154
*
151155
* @return ResponseInterface
152156
*/
@@ -158,7 +162,7 @@ protected function respondCreated($data = null, string $message = '')
158162
/**
159163
* Used after a resource has been successfully deleted.
160164
*
161-
* @param array|string|null $data
165+
* @param array<string, mixed>|string|null $data
162166
*
163167
* @return ResponseInterface
164168
*/
@@ -170,7 +174,7 @@ protected function respondDeleted($data = null, string $message = '')
170174
/**
171175
* Used after a resource has been successfully updated.
172176
*
173-
* @param array|string|null $data
177+
* @param array<string, mixed>|string|null $data
174178
*
175179
* @return ResponseInterface
176180
*/
@@ -287,15 +291,17 @@ protected function failServerError(string $description = 'Internal Server Error'
287291
* Handles formatting a response. Currently, makes some heavy assumptions
288292
* and needs updating! :)
289293
*
290-
* @param array|string|null $data
294+
* @param array<string, mixed>|string|null $data
291295
*
292296
* @return string|null
293297
*/
294298
protected function format($data = null)
295299
{
300+
/** @var Format $format */
296301
$format = service('format');
297302

298-
$mime = ($this->format === null) ? $format->getConfig()->supportedResponseFormats[0]
303+
$mime = $this->format === null
304+
? $format->getConfig()->supportedResponseFormats[0]
299305
: "application/{$this->format}";
300306

301307
// Determine correct response type through content negotiation if not explicitly declared
@@ -313,14 +319,10 @@ protected function format($data = null)
313319
$this->response->setContentType($mime);
314320

315321
// if we don't have a formatter, make one
316-
if (! isset($this->formatter)) {
317-
// if no formatter, use the default
318-
$this->formatter = $format->getFormatter($mime);
319-
}
322+
$this->formatter ??= $format->getFormatter($mime);
320323

321324
$asHtml = $this->stringAsHtml ?? false;
322325

323-
// Returns as HTML.
324326
if (
325327
($mime === 'application/json' && $asHtml && is_string($data))
326328
|| ($mime !== 'application/json' && is_string($data))
@@ -338,6 +340,7 @@ protected function format($data = null)
338340
if ($mime !== 'application/json') {
339341
// Recursively convert objects into associative arrays
340342
// Conversion not required for JSONFormatter
343+
/** @var array<string, mixed>|string|null $data */
341344
$data = json_decode(json_encode($data), true);
342345
}
343346

@@ -353,7 +356,7 @@ protected function format($data = null)
353356
*/
354357
protected function setResponseFormat(?string $format = null)
355358
{
356-
$this->format = ($format === null) ? null : strtolower($format);
359+
$this->format = $format === null ? null : strtolower($format);
357360

358361
return $this;
359362
}

‎tests/system/API/ResponseTraitTest.php‎

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717
use CodeIgniter\Format\FormatterInterface;
1818
use CodeIgniter\Format\JSONFormatter;
1919
use CodeIgniter\Format\XMLFormatter;
20+
use CodeIgniter\HTTP\RequestInterface;
21+
use CodeIgniter\HTTP\ResponseInterface;
2022
use CodeIgniter\HTTP\SiteURI;
2123
use CodeIgniter\HTTP\UserAgent;
2224
use CodeIgniter\Test\CIUnitTestCase;
2325
use CodeIgniter\Test\Mock\MockIncomingRequest;
2426
use CodeIgniter\Test\Mock\MockResponse;
2527
use Config\App;
2628
use Config\Cookie;
29+
use Config\Services;
2730
use PHPUnit\Framework\Attributes\Group;
2831
use stdClass;
2932

@@ -82,6 +85,12 @@ private function createCookieConfig(): Cookie
8285
return $cookie;
8386
}
8487

88+
/**
89+
* @param array<string, string> $userHeaders
90+
*
91+
* @phpstan-assert RequestInterface $this->request
92+
* @phpstan-assert ResponseInterface $this->response
93+
*/
8594
private function createRequestAndResponse(string $routePath = '', array $userHeaders = []): void
8695
{
8796
$config = $this->createAppConfig();
@@ -97,29 +106,28 @@ private function createRequestAndResponse(string $routePath = '', array $userHea
97106
$this->response = new MockResponse($config);
98107
}
99108

100-
// Insert headers into request.
101-
$headers = [
102-
'Accept' => 'text/html',
103-
];
104-
$headers = array_merge($headers, $userHeaders);
109+
$headers = array_merge(['Accept' => 'text/html'], $userHeaders);
105110

106111
foreach ($headers as $key => $value) {
107112
$this->request->setHeader($key, $value);
108113
}
109114
}
110115

116+
/**
117+
* @param array<string, string> $userHeaders
118+
*/
111119
protected function makeController(string $routePath = '', array $userHeaders = []): object
112120
{
113121
$this->createRequestAndResponse($routePath, $userHeaders);
114122

115-
// Create the controller class finally.
116123
return new class ($this->request, $this->response, $this->formatter) {
117124
use ResponseTrait;
118125

119-
protected $formatter;
120-
121-
public function __construct(protected $request, protected $response, $formatter)
122-
{
126+
public function __construct(
127+
protected RequestInterface $request,
128+
protected ResponseInterface $response,
129+
?FormatterInterface $formatter,
130+
) {
123131
$this->formatter = $formatter;
124132
}
125133

@@ -173,11 +181,13 @@ public function testNoFormatterWithStringAsHtmlTrue(): void
173181
$controller = new class ($this->request, $this->response, $this->formatter) {
174182
use ResponseTrait;
175183

176-
protected $formatter;
177184
protected bool $stringAsHtml = true;
178185

179-
public function __construct(protected $request, protected $response, $formatter)
180-
{
186+
public function __construct(
187+
protected RequestInterface $request,
188+
protected ResponseInterface $response,
189+
?FormatterInterface $formatter,
190+
) {
181191
$this->formatter = $formatter;
182192
}
183193
};
@@ -291,11 +301,13 @@ public function testRespondSetsCorrectBodyAndStatusWithStringAsHtmlTrue(): void
291301
$controller = new class ($this->request, $this->response, $this->formatter) {
292302
use ResponseTrait;
293303

294-
protected $formatter;
295304
protected bool $stringAsHtml = true;
296305

297-
public function __construct(protected $request, protected $response, $formatter)
298-
{
306+
public function __construct(
307+
protected RequestInterface $request,
308+
protected ResponseInterface $response,
309+
?FormatterInterface $formatter,
310+
) {
299311
$this->formatter = $formatter;
300312
}
301313
};
@@ -546,8 +558,8 @@ public function testValidContentTypes(): void
546558

547559
private function tryValidContentType(string $mimeType, string $contentType): void
548560
{
549-
$original = $_SERVER;
550-
$_SERVER['CONTENT_TYPE'] = $mimeType;
561+
$originalContentType = Services::superglobals()->server('CONTENT_TYPE') ?? '';
562+
Services::superglobals()->setServer('CONTENT_TYPE', $mimeType);
551563

552564
$this->makeController('', ['Accept' => $mimeType]);
553565
$this->assertSame(
@@ -563,7 +575,7 @@ private function tryValidContentType(string $mimeType, string $contentType): voi
563575
'Response header pre-response...',
564576
);
565577

566-
$_SERVER = $original;
578+
Services::superglobals()->setServer('CONTENT_TYPE', $originalContentType);
567579
}
568580

569581
public function testValidResponses(): void
@@ -609,9 +621,11 @@ public function testFormatByRequestNegotiateIfFormatIsNotJsonOrXML(): void
609621
$controller = new class ($request, $response) {
610622
use ResponseTrait;
611623

612-
public function __construct(protected $request, protected $response)
613-
{
614-
$this->format = 'txt';
624+
public function __construct(
625+
protected RequestInterface $request,
626+
protected ResponseInterface $response,
627+
) {
628+
$this->format = 'txt'; // @phpstan-ignore assign.propertyType (needed for testing)
615629
}
616630
};
617631

@@ -659,6 +673,9 @@ public function testXMLResponseFormat(): void
659673
$this->assertSame($xmlFormatter->format($data), $this->response->getXML());
660674
}
661675

676+
/**
677+
* @param list<mixed> $args
678+
*/
662679
private function invoke(object $controller, string $method, array $args = []): object
663680
{
664681
$method = self::getPrivateMethodInvoker($controller, $method);

‎utils/phpstan-baseline/assign.propertyType.neon‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# total 29 errors
1+
# total 28 errors
22

33
parameters:
44
ignoreErrors:
@@ -7,11 +7,6 @@ parameters:
77
count: 1
88
path: ../../system/Controller.php
99

10-
-
11-
message: '#^Property class@anonymous/tests/system/API/ResponseTraitTest\.php\:609\:\:\$format \(''html''\|''json''\|''xml''\|null\) does not accept ''txt''\.$#'
12-
count: 1
13-
path: ../../tests/system/API/ResponseTraitTest.php
14-
1510
-
1611
message: '#^Property CodeIgniter\\Commands\\Utilities\\Routes\\FilterFinderTest\:\:\$response \(CodeIgniter\\HTTP\\Response\) does not accept CodeIgniter\\HTTP\\ResponseInterface\.$#'
1712
count: 1

‎utils/phpstan-baseline/codeigniter.superglobalAccessAssign.neon‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# total 582 errors
1+
# total 581 errors
22

33
parameters:
44
ignoreErrors:
@@ -17,11 +17,6 @@ parameters:
1717
count: 3
1818
path: ../../system/HTTP/IncomingRequest.php
1919

20-
-
21-
message: '#^Assigning string directly on offset ''CONTENT_TYPE'' of \$_SERVER is discouraged\.$#'
22-
count: 1
23-
path: ../../tests/system/API/ResponseTraitTest.php
24-
2520
-
2621
message: '#^Assigning 3 directly on offset ''argc'' of \$_SERVER is discouraged\.$#'
2722
count: 1

‎utils/phpstan-baseline/loader.neon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# total 3145 errors
1+
# total 3075 errors
22
includes:
33
- argument.type.neon
44
- assign.propertyType.neon

0 commit comments

Comments
 (0)