Static analysis fixes (#203)

* Fixed bug in the cookie plugin
* Fixed typo in phpdoc
* Clarified plugin client factory type information
* Added extra type information to help phpstan
* Don't temprarily violate property types in the flexible http client
* Removed redundent information
* Cast null to string for date parsing
* Added additional type to help phpstan
* Run phpstan on actions

Author: GrahamCampbell

.gitattributes

@@ -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

.github/workflows/static.yml

@@ -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/phpstan-ga@0.12.32
+        env:
+          REQUIRE_DEV: false
+        with:
+          args: analyze --no-progress

.gitignore

@@ -5,4 +5,5 @@
 /composer.lock
 /phpspec.yml
 /phpunit.xml
+/phpstan.neon
 /vendor/

.php_cs.dist

@@ -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);

phpstan.neon.dist

@@ -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\\.$#"
+            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\\.$#"
+            count: 1
+            path: src/Plugin/Journal.php
+
+        -
+            message: "#^Method Http\\\\Client\\\\Common\\\\Plugin\\\\Journal\\:\\:addFailure\\(\\) has no return typehint specified\\.$#"
+            count: 1
+            path: src/Plugin/Journal.php
+
+        -
+            message: "#^Call to an undefined method Http\\\\Client\\\\HttpAsyncClient\\:\\:sendRequest\\(\\)\\.$#"
+            count: 1
+            path: src/PluginClient.php

src/Deferred.php

@@ -146,6 +146,7 @@ public function wait($unwrap = true)
             return $this->value;
         }
 
+        /** @var ClientExceptionInterface */
         throw $this->failure;
     }
 }

src/Exception/HttpClientNoMatchException.php

@@ -14,6 +14,9 @@
  */
 final class HttpClientNoMatchException extends TransferException
 {
+    /**
+     * @var RequestInterface
+     */
     private $request;
 
     public function __construct(string $message, RequestInterface $request, \Exception $previous = null)

src/FlexibleHttpClient.php

@@ -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);
     }
 }

src/HttpClientPool/HttpClientPool.php

@@ -25,13 +25,14 @@ abstract class HttpClientPool implements HttpClientPoolInterface
     /**
      * Add a client to the pool.
      *
-     * @param ClientInterface|HttpAsyncClient|HttpClientPoolItem $client
+     * @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))
             );
         }
 

src/HttpClientRouter.php

@@ -19,7 +19,7 @@
 final class HttpClientRouter implements HttpClientRouterInterface
 {
     /**
-     * @var array
+     * @var (array{matcher: RequestMatcher, client: FlexibleHttpClient})[]
      */
     private $clients = [];
 
@@ -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)) {

src/HttpMethodsClient.php

@@ -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)
     {

src/Plugin/CookiePlugin.php

@@ -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(
@@ -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;
 
         return [$key, $value];
     }

src/Plugin/SeekableBodyPlugin.php

@@ -12,8 +12,14 @@
  */
 abstract class SeekableBodyPlugin implements Plugin
 {
+    /**
+     * @var bool
+     */
     protected $useFileBuffer;
 
+    /**
+     * @var int
+     */
     protected $memoryBufferSize;
 
     /**

src/PluginChain.php

@@ -14,7 +14,7 @@ final class PluginChain
     /** @var Plugin[] */
     private $plugins;
 
-    /** @var callable */
+    /** @var callable(RequestInterface): Promise */
     private $clientCallable;
 
     /** @var int */
@@ -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
      * }

src/PluginClient.php

@@ -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;
@@ -24,7 +25,7 @@ final class PluginClient implements HttpClient, HttpAsyncClient
     /**
      * An HTTP async client.
      *
-     * @var HttpAsyncClient|HttpClient
+     * @var HttpAsyncClient
      */
     private $client;
 
@@ -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));
@@ -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);
     }
 }

src/PluginClientBuilder.php

@@ -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;

src/PluginClientFactory.php

@@ -16,7 +16,7 @@
 final class PluginClientFactory
 {
     /**
-     * @var callable|null
+     * @var (callable(ClientInterface|HttpAsyncClient, Plugin[], array): PluginClient)|null
      */
     private static $factory;
 
@@ -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
     {
         static::$factory = $factory;
     }