Skip to content

fix: correct phpstan config error #296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
phil-davis merged 1 commit into from
Aug 26, 2026
Merged

fix: correct phpstan config error #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions lib/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,17 @@ public function send(RequestInterface $request): ResponseInterface
// If retry was still set to false, it means no event handler
// dealt with the problem. In this case we just re-throw the
// exception.
// @phpstan-ignore booleanNot.alwaysTrue

@phil-davis phil-davis Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For most of these, phpstan is noticing "defensive" code.
In this case $retry has been passed to emit in a way that is potentially writeable.
So maybe it could have value either true or false here?

@staabm staabm Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Pass-by-ref is hard to properly analyze. Needs a new phpstan issue with a small reproducer.

@phil-davis phil-davis Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpstan/phpstan#15116

if (!$retry) {
throw $e;
}
}

// @phpstan-ignore if.alwaysFalse
if ($retry) {
++$retryCount;
}
// @phpstan-ignore booleanOr.leftAlwaysFalse
} while ($retry || $doRedirect);

$this->emit('afterRequest', [$request, $response]);
Expand Down Expand Up @@ -228,6 +231,7 @@ public function poll(): bool
$e = new ClientException($curlResult['curl_errmsg'], $curlResult['curl_errno']);
$this->emit('exception', [$request, $e, &$retry, $retryCount]);

// @phpstan-ignore if.alwaysFalse
if ($retry) {
++$retryCount;
$this->sendAsyncInternal($request, $successCallback, $errorCallback, $retryCount);
Expand All @@ -236,13 +240,15 @@ public function poll(): bool

$curlResult['request'] = $request;

// @phpstan-ignore function.alreadyNarrowedType
if (is_callable($errorCallback)) {
$errorCallback($curlResult);
}
} elseif (self::STATUS_HTTPERROR === $curlResult['status']) {
$this->emit('error', [$request, $curlResult['response'], &$retry, $retryCount]);
$this->emit('error:'.$curlResult['http_code'], [$request, $curlResult['response'], &$retry, $retryCount]);

// @phpstan-ignore if.alwaysFalse
if ($retry) {
++$retryCount;
$this->sendAsyncInternal($request, $successCallback, $errorCallback, $retryCount);
Expand All @@ -251,12 +257,14 @@ public function poll(): bool

$curlResult['request'] = $request;

// @phpstan-ignore function.alreadyNarrowedType
if (is_callable($errorCallback)) {
$errorCallback($curlResult);
}
} else {
$this->emit('afterRequest', [$request, $curlResult['response']]);

// @phpstan-ignore function.alreadyNarrowedType
if (is_callable($successCallback)) {
$successCallback($curlResult['response']);
}
Expand Down Expand Up @@ -299,7 +307,7 @@ public function setThrowExceptions(bool $throwExceptions): void
*
* These settings will be included in every HTTP request.
*/
public function addCurlSetting(int $name, $value): void
public function addCurlSetting(int $name, mixed $value): void

@phil-davis phil-davis Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpstan complained that the type of $value was not declared.
These days we can directly declare the mixed type. So that is good.

{
$this->curlSettings[$name] = $value;
}
Expand All @@ -311,10 +319,10 @@ protected function doRequest(RequestInterface $request): ResponseInterface
{
$settings = $this->createCurlSettingsArray($request);

if (null === $this->curlHandle) {
$this->curlHandle = curl_init();
} else {
if (isset($this->curlHandle)) {

@phil-davis phil-davis Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curlHandle is either uninitialized or a proper CurlHandle type.
So check for uninitialized by using isset() rather than trying to compare to null

curl_reset($this->curlHandle);
} else {
$this->curlHandle = curl_init();
}

curl_setopt_array($this->curlHandle, $settings);
Expand All @@ -332,17 +340,15 @@ protected function doRequest(RequestInterface $request): ResponseInterface
*
* By keeping this resource around for the lifetime of this object, things
* like persistent connections are possible.
*
* @var resource|null
*/
private $curlHandle;
private \CurlHandle $curlHandle;

/**
* Handler for curl_multi requests.
*
* The first time sendAsync is used, this will be created.
*
* @var resource|null
* @var \CurlMultiHandle|null
*/
private $curlMultiHandle;

Expand Down Expand Up @@ -382,6 +388,7 @@ protected function createCurlSettingsArray(RequestInterface $request): array
// reason.
$settings[CURLOPT_PUT] = true;
$settings[CURLOPT_INFILE] = $body;
// @phpstan-ignore function.alreadyNarrowedType
if (false !== $bodyStat && array_key_exists('size', $bodyStat)) {
$settings[CURLOPT_INFILESIZE] = $bodyStat['size'];
}
Expand Down Expand Up @@ -426,7 +433,7 @@ protected function createCurlSettingsArray(RequestInterface $request): array
public const STATUS_HTTPERROR = 2;

/**
* @param resource $curlHandle
* @param \CurlHandle $curlHandle
*
* @return mixed[]
*/
Expand Down Expand Up @@ -466,7 +473,7 @@ private function parseResponse(string $response, $curlHandle): array
* status is STATUS_SUCCESS, or STATUS_HTTPERROR
*
* @param array<int, string> $headerLines
* @param resource $curlHandle
* @param \CurlHandle $curlHandle
*
* @return array<string, mixed>
*/
Expand Down Expand Up @@ -523,7 +530,7 @@ protected function parseCurlResponse(array $headerLines, string $body, $curlHand
*
* @deprecated Use parseCurlResponse instead
*
* @param resource $curlHandle
* @param \CurlHandle $curlHandle
*
* @return array<string, mixed>
*/
Expand All @@ -548,6 +555,7 @@ protected function parseCurlResult(string $response, $curlHandle): array
// This will cause substr($response, $curlInfo['header_size']) return FALSE instead of NULL
// An exception will be thrown when calling getBodyAsString then
$responseBody = substr($response, $curlInfo['header_size']);
// @phpstan-ignore identical.alwaysFalse
if (false === $responseBody) {
$responseBody = '';
}
Expand Down Expand Up @@ -603,7 +611,7 @@ protected function sendAsyncInternal(RequestInterface $request, callable $succes
*
* This method exists so that it can easily be overridden and mocked.
*
* @param resource $curlHandle
* @param \CurlHandle $curlHandle
*/
protected function curlExec($curlHandle): string
{
Expand All @@ -622,7 +630,7 @@ protected function curlExec($curlHandle): string
*
* This method exists so that it can easily be overridden and mocked.
*
* @param resource $curlHandle
* @param \CurlHandle $curlHandle
*
* @return array<int, mixed>
*/
Expand Down
4 changes: 0 additions & 4 deletions phpstan.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,3 @@ parameters:
message: "#^.* will always evaluate to true\\.$#"
count: 4
path: tests/*
-
message: "#^Left side of || is always false.$#"
count: 23
path: lib/Client.php
4 changes: 2 additions & 2 deletions tests/HTTP/ClientTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ public function doRequest(RequestInterface $request): ResponseInterface
*
* This method exists so that it can easily be overridden and mocked.
*
* @param resource $curlHandle
* @param \CurlHandle $curlHandle
*/
protected function curlStuff($curlHandle): array
{
Expand All @@ -581,7 +581,7 @@ protected function curlStuff($curlHandle): array
*
* This method exists so that it can easily be overridden and mocked.
*
* @param resource $curlHandle
* @param \CurlHandle $curlHandle
*/
protected function curlExec($curlHandle): string
{
Expand Down