Skip to content

Static analysis fixes #203

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.

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
merged 13 commits into from
Jul 20, 2020
Merged
29 changes: 15 additions & 14 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
.editorconfig export-ignore
.gitattributes export-ignore
/.github/ export-ignore
.gitignore export-ignore
/.php_cs export-ignore
/.scrutinizer.yml export-ignore
/.styleci.yml export-ignore
/behat.yml.dist export-ignore
/features/ export-ignore
/phpspec.ci.yml export-ignore
/phpspec.yml.dist export-ignore
/phpunit.xml.dist export-ignore
/spec/ export-ignore
/tests/ export-ignore
.editorconfig export-ignore
.gitattributes export-ignore
/.github/ export-ignore
.gitignore export-ignore
/.php_cs export-ignore
/.scrutinizer.yml export-ignore
/.styleci.yml export-ignore
/behat.yml.dist export-ignore
/features/ export-ignore
/phpspec.ci.yml export-ignore
/phpspec.yml.dist export-ignore
/phpunit.xml.dist export-ignore
/phpstan.neon.dist export-ignore
/spec/ export-ignore
/tests/ export-ignore
21 changes: 21 additions & 0 deletions .github/workflows/static.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: static

on:
push:
pull_request:

jobs:
phpstan:
name: PHPStan
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v2

- name: PHPStan
uses: OskarStark/[email protected]
env:
REQUIRE_DEV: false
with:
args: analyze --no-progress
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
/composer.lock
/phpspec.yml
/phpunit.xml
/phpstan.neon
/vendor/
2 changes: 1 addition & 1 deletion .php_cs.dist
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ return PhpCsFixer\Config::create()
'syntax' => 'short',
],
'no_empty_phpdoc' => true,
'no_superfluous_phpdoc_tags' => true,
'phpdoc_to_comment' => false,
'single_line_throw' => false,
])
->setFinder($finder);
31 changes: 31 additions & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
parameters:
level: max
checkMissingIterableValueType: false
treatPhpDocTypesAsCertain: false
paths:
- src
ignoreErrors:
-
message: "#^Strict comparison using \\!\\=\\= between null and null will always evaluate to false\\.$#"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a false positive, and should be reported to phpstan as a bug.

count: 1
path: src/HttpMethodsClient.php

-
message: "#^Cannot call method createStream\\(\\) on Psr\\\\Http\\\\Message\\\\StreamFactoryInterface\\|null\\.$#"
count: 1
path: src/HttpMethodsClient.php

-
message: "#^Method Http\\\\Client\\\\Common\\\\Plugin\\\\Journal\\:\\:addSuccess\\(\\) has no return typehint specified\\.$#"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a no-fix. Changing it would break BC. Maybe we should consider adding void return to the interface in the next major version.

count: 1
path: src/Plugin/Journal.php

-
message: "#^Method Http\\\\Client\\\\Common\\\\Plugin\\\\Journal\\:\\:addFailure\\(\\) has no return typehint specified\\.$#"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a no-fix. Changing it would break BC. Maybe we should consider adding void return to the interface in the next major version.

count: 1
path: src/Plugin/Journal.php

-
message: "#^Call to an undefined method Http\\\\Client\\\\HttpAsyncClient\\:\\:sendRequest\\(\\)\\.$#"
count: 1
path: src/PluginClient.php
1 change: 1 addition & 0 deletions src/Deferred.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ public function wait($unwrap = true)
return $this->value;
}

/** @var ClientExceptionInterface */
throw $this->failure;
}
}
3 changes: 3 additions & 0 deletions src/Exception/HttpClientNoMatchException.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
*/
final class HttpClientNoMatchException extends TransferException
{
/**
* @var RequestInterface
*/
private $request;

public function __construct(string $message, RequestInterface $request, \Exception $previous = null)
Expand Down
12 changes: 2 additions & 10 deletions src/FlexibleHttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,7 @@ public function __construct($client)
);
}

$this->httpClient = $client;
$this->httpAsyncClient = $client;

if (!($this->httpClient instanceof ClientInterface)) {
$this->httpClient = new EmulatedHttpClient($this->httpClient);
}

if (!($this->httpAsyncClient instanceof HttpAsyncClient)) {
$this->httpAsyncClient = new EmulatedHttpAsyncClient($this->httpAsyncClient);
}
$this->httpClient = $client instanceof ClientInterface ? $client : new EmulatedHttpClient($client);
$this->httpAsyncClient = $client instanceof HttpAsyncClient ? $client : new EmulatedHttpAsyncClient($client);
}
}
7 changes: 4 additions & 3 deletions src/HttpClientPool/HttpClientPool.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ abstract class HttpClientPool implements HttpClientPoolInterface
/**
* Add a client to the pool.
*
* @param ClientInterface|HttpAsyncClient|HttpClientPoolItem $client
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a BC break. A HttpClientPoolItem is a ClientInterface. It's incorrect to specify it again.

* @param ClientInterface|HttpAsyncClient $client
*/
public function addHttpClient($client): void
{
if (!$client instanceof ClientInterface && !$client instanceof HttpAsyncClient && !$client instanceof HttpClientPoolItem) {
// no need to check for HttpClientPoolItem here, since it extends the other interfaces
if (!$client instanceof ClientInterface && !$client instanceof HttpAsyncClient) {
throw new \TypeError(
sprintf('%s::addHttpClient(): Argument #1 ($client) must be of type %s|%s|%s, %s given', self::class, ClientInterface::class, HttpAsyncClient::class, HttpClientPoolItem::class, get_debug_type($client))
sprintf('%s::addHttpClient(): Argument #1 ($client) must be of type %s|%s, %s given', self::class, ClientInterface::class, HttpAsyncClient::class, get_debug_type($client))
);
}

Expand Down
6 changes: 2 additions & 4 deletions src/HttpClientRouter.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
final class HttpClientRouter implements HttpClientRouterInterface
{
/**
* @var array
* @var (array{matcher: RequestMatcher, client: FlexibleHttpClient})[]
*/
private $clients = [];

Expand Down Expand Up @@ -60,10 +60,8 @@ public function addClient($client, RequestMatcher $requestMatcher): void

/**
* Choose an HTTP client given a specific request.
*
* @return ClientInterface|HttpAsyncClient
*/
private function chooseHttpClient(RequestInterface $request)
private function chooseHttpClient(RequestInterface $request): FlexibleHttpClient
{
foreach ($this->clients as $client) {
if ($client['matcher']->matches($request)) {
Expand Down
2 changes: 1 addition & 1 deletion src/HttpMethodsClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ final class HttpMethodsClient implements HttpMethodsClientInterface
private $streamFactory;

/**
* @param RequestFactory|RequestFactoryInterface
* @param RequestFactory|RequestFactoryInterface $requestFactory
*/
public function __construct(ClientInterface $httpClient, $requestFactory, StreamFactoryInterface $streamFactory = null)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Plugin/CookiePlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private function createCookie(RequestInterface $request, string $setCookieHeader
switch (strtolower($key)) {
case 'expires':
try {
$expires = CookieUtil::parseDate($value);
$expires = CookieUtil::parseDate((string) $value);
} catch (UnexpectedValueException $e) {
throw new TransferException(
sprintf(
Expand Down Expand Up @@ -167,13 +167,13 @@ private function createCookie(RequestInterface $request, string $setCookieHeader
*
* @param string $part A single cookie value in format key=value
*
* @return string[]
* @return array{0:string, 1:?string}
*/
private function createValueKey(string $part): array
{
$parts = explode('=', $part, 2);
$key = trim($parts[0]);
$value = isset($parts[1]) ? trim($parts[1]) : true;
$value = isset($parts[1]) ? trim($parts[1]) : null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a BC break: the Cookie class only accepts string or null as a value. This is a bug fix.


return [$key, $value];
}
Expand Down
6 changes: 6 additions & 0 deletions src/Plugin/SeekableBodyPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@
*/
abstract class SeekableBodyPlugin implements Plugin
{
/**
* @var bool
*/
protected $useFileBuffer;

/**
* @var int
*/
protected $memoryBufferSize;

/**
Expand Down
8 changes: 4 additions & 4 deletions src/PluginChain.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ final class PluginChain
/** @var Plugin[] */
private $plugins;

/** @var callable */
/** @var callable(RequestInterface): Promise */
private $clientCallable;

/** @var int */
Expand All @@ -24,9 +24,9 @@ final class PluginChain
private $restarts = 0;

/**
* @param Plugin[] $plugins A plugin chain
* @param callable $clientCallable Callable making the HTTP call
* @param array $options {
* @param Plugin[] $plugins A plugin chain
* @param callable(RequestInterface): Promise $clientCallable Callable making the HTTP call
* @param array $options {
*
* @var int $max_restarts
* }
Expand Down
14 changes: 9 additions & 5 deletions src/PluginClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Http\Client\HttpClient;
use Http\Client\Promise\HttpFulfilledPromise;
use Http\Client\Promise\HttpRejectedPromise;
use Http\Promise\Promise;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
Expand All @@ -24,7 +25,7 @@ final class PluginClient implements HttpClient, HttpAsyncClient
/**
* An HTTP async client.
*
* @var HttpAsyncClient|HttpClient
* @var HttpAsyncClient
*/
private $client;

Expand Down Expand Up @@ -71,13 +72,13 @@ public function __construct($client, array $plugins = [], array $options = [])
*/
public function sendRequest(RequestInterface $request): ResponseInterface
{
// If we don't have an http client, use the async call
if (!($this->client instanceof ClientInterface)) {
// If the client doesn't support sync calls, call async
if (!$this->client instanceof ClientInterface) {
return $this->sendAsyncRequest($request)->wait();
}

// Else we want to use the synchronous call of the underlying client, and not the async one in the case
// we have both an async and sync call
// Else we want to use the synchronous call of the underlying client,
// and not the async one in the case we have both an async and sync call
$pluginChain = $this->createPluginChain($this->plugins, function (RequestInterface $request) {
try {
return new HttpFulfilledPromise($this->client->sendRequest($request));
Expand Down Expand Up @@ -121,9 +122,12 @@ private function configure(array $options = []): array
*
* @param Plugin[] $plugins A plugin chain
* @param callable $clientCallable Callable making the HTTP call
*
* @return callable(RequestInterface): Promise
*/
private function createPluginChain(array $plugins, callable $clientCallable): callable
{
/** @var callable(RequestInterface): Promise */
return new PluginChain($plugins, $clientCallable, $this->options);
}
}
3 changes: 3 additions & 0 deletions src/PluginClientBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ public function addPlugin(Plugin $plugin, int $priority = 0): self
return $this;
}

/**
* @param mixed $value
*/
public function setOption(string $name, $value): self
{
$this->options[$name] = $value;
Expand Down
6 changes: 4 additions & 2 deletions src/PluginClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
final class PluginClientFactory
{
/**
* @var callable|null
* @var (callable(ClientInterface|HttpAsyncClient, Plugin[], array): PluginClient)|null
*/
private static $factory;

Expand All @@ -28,8 +28,10 @@ final class PluginClientFactory
* application execution.
*
* @internal
*
* @param callable(ClientInterface|HttpAsyncClient, Plugin[], array): PluginClient $factory
*/
public static function setFactory(callable $factory)
public static function setFactory(callable $factory): void
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a BC break: the class is final.

{
static::$factory = $factory;
}
Expand Down