Add PHPStan in CI (#19)

* Add PHPStan

* Apply PHPStan fixes

* Update php.yml

* Update README.md

Author: jbelien

.github/workflows/php.yml

@@ -7,29 +7,37 @@ on:
     branches: [ master ]
 
 jobs:
-  build:
-
+  test:
     runs-on: ubuntu-latest
-
     strategy:
       fail-fast: false
       matrix:
         php-version: ['7.4', '8.0', '8.1', '8.2']
-
     steps:
     - uses: actions/checkout@v2
-
     - name: Use PHP ${{ matrix.php-version }}
       uses: shivammathur/setup-php@v2
       with:
         php-version: ${{ matrix.php-version }}
         extensions: curl
-
     - name: Validate composer.json and composer.lock
       run: composer validate
-
     - name: Install dependencies
       run: composer update --prefer-dist --no-progress
-
     - name: Run test suite
       run: composer run-script test
+
+  phpstan:
+    runs-on: ubuntu-latest
+    name: PHPStan
+    steps:
+    - uses: actions/checkout@v3
+    - name: Use PHP 8.2
+      uses: shivammathur/setup-php@v2
+      with:
+        php-version: 8.2
+        extensions: curl
+    - name: Install dependencies
+      run: composer install --no-progress
+    - name: Run PHPStan
+      run: composer run-script analyse

README.md

@@ -5,7 +5,7 @@
 [![Monthly Downloads](https://poser.pugx.org/geocoder-php/provider-integration-tests/d/monthly.png)](https://packagist.org/packages/geocoder-php/provider-integration-tests)
 [![Software License](https://img.shields.io/badge/license-MIT-brightgreen.svg?style=flat-square)](LICENSE)
 
-This repository contains integration tests to make sure your implementation of a Geocoder Provider is correct. 
+This repository contains integration tests to make sure your implementation of a Geocoder Provider is correct.
 
 ### Install
 
@@ -15,7 +15,7 @@ composer require --dev geocoder-php/provider-integration-tests:dev-master
 
 ### Use
 
-Create a test that looks like this: 
+Create a test that looks like this:
 
 ```php
 use Geocoder\IntegrationTest\ProviderIntegrationTest;
@@ -28,16 +28,15 @@ class IntegrationTest extends ProviderIntegrationTest
     {
         return new GoogleMaps($httpClient);
     }
-    
-    protected function getCacheDir()
+
+    protected function getCacheDir(): string;
     {
         return dirname(__DIR__).'/.cached_responses';
     }
 
-    protected function getApiKey()
+    protected function getApiKey(): string;
     {
         return env('GOOLE_API_KEY');
     }
 }
 ```
-

composer.json

@@ -24,7 +24,8 @@
     },
     "require-dev": {
         "geocoder-php/nominatim-provider": "^5.3",
-        "php-http/curl-client": "^2.2"
+        "php-http/curl-client": "^2.2",
+        "phpstan/phpstan": "^1.10"
     },
     "autoload": {
         "psr-4": {
@@ -45,6 +46,7 @@
         "sort-packages": true
     },
     "scripts": {
+        "analyse": "vendor/bin/phpstan analyse",
         "test": "vendor/bin/phpunit"
     }
 }

phpstan.neon

@@ -0,0 +1,5 @@
+parameters:
+    level: 8
+    paths:
+        - src
+        - tests

src/BaseTestCase.php

@@ -30,10 +30,8 @@ abstract protected function getCacheDir();
 
     /**
      * Get a real HTTP client. If a cache dir is set to a path it will use cached responses.
-     *
-     * @return ClientInterface
      */
-    protected function getHttpClient($apiKey = null)
+    protected function getHttpClient(string $apiKey = null): ClientInterface
     {
         if (null !== $cacheDir = $this->getCacheDir()) {
             return new CachedResponseClient(new HttplugClient(), $cacheDir, $apiKey);
@@ -44,13 +42,8 @@ protected function getHttpClient($apiKey = null)
 
     /**
      * Get a mocked HTTP client that never do calls over the internet. Use this is you want to control the response data.
-     *
-     * @param string|null $body
-     * @param int         $statusCode
-     *
-     * @return ClientInterface
      */
-    protected function getMockedHttpClient($body = null, $statusCode = 200)
+    protected function getMockedHttpClient(string $body = null, int $statusCode = 200): ClientInterface
     {
         $client = new MockedHttpClient();
         $client->addResponse(new Response($statusCode, [], $body));
@@ -60,13 +53,8 @@ protected function getMockedHttpClient($body = null, $statusCode = 200)
 
     /**
      * Get a mocked HTTP client where you may do tests on the request.
-     *
-     * @param string|null $body
-     * @param int         $statusCode
-     *
-     * @return ClientInterface
      */
-    protected function getMockedHttpClientCallback(callable $requestCallback)
+    protected function getMockedHttpClientCallback(callable $requestCallback): ClientInterface
     {
         $client = $this->getMockBuilder(ClientInterface::class)->getMock();
 

src/CachedResponseClient.php

@@ -75,8 +75,8 @@ public function sendRequest(RequestInterface $request): ResponseInterface
         }
 
         $file = sprintf('%s/%s_%s', $this->cacheDir, $host, sha1($cacheKey));
-        if (is_file($file) && is_readable($file)) {
-            return new Response(200, [], (new HttplugFactory())->createStream(unserialize(file_get_contents($file))));
+        if (is_file($file) && is_readable($file) && ($content = file_get_contents($file)) !== false) {
+            return new Response(200, [], (new HttplugFactory())->createStream(unserialize($content)));
         }
 
         $response = $this->delegate->sendRequest($request);

src/ProviderIntegrationTest.php

@@ -10,6 +10,7 @@
 
 namespace Geocoder\IntegrationTest;
 
+use Exception;
 use Geocoder\Collection;
 use Geocoder\Exception\InvalidCredentials;
 use Geocoder\Exception\InvalidServerResponse;
@@ -24,6 +25,7 @@
 use Geocoder\Query\ReverseQuery;
 use Http\Discovery\Psr18ClientDiscovery;
 use Nyholm\Psr7\Response;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Client\ClientInterface;
 use Psr\Http\Message\ResponseInterface;
@@ -34,15 +36,15 @@
 abstract class ProviderIntegrationTest extends TestCase
 {
     /**
-     * @var array with functionName => reason
+     * @var array<string,string> with functionName => reason
      */
-    protected $skippedTests = [];
+    protected array $skippedTests = [];
 
-    protected $testAddress = true;
-    protected $testReverse = true;
-    protected $testIpv4 = true;
-    protected $testIpv6 = true;
-    protected $testHttpProvider = true;
+    protected bool $testAddress = true;
+    protected bool $testReverse = true;
+    protected bool $testIpv4 = true;
+    protected bool $testIpv6 = true;
+    protected bool $testHttpProvider = true;
 
     /**
      * @return Provider that is used in the tests.
@@ -52,17 +54,17 @@ abstract protected function createProvider(ClientInterface $httpClient);
     /**
      * @return string the directory where cached responses are stored
      */
-    abstract protected function getCacheDir();
+    abstract protected function getCacheDir(): string;
 
     /**
      * @return string the API key or substring to be removed from cache.
      */
-    abstract protected function getApiKey();
+    abstract protected function getApiKey(): string;
 
     /**
      * @param ResponseInterface $response
      *
-     * @return \PHPUnit_Framework_MockObject_MockObject|ClientInterface
+     * @return ClientInterface&MockObject
      */
     private function getHttpClient(ResponseInterface $response)
     {
@@ -78,10 +80,8 @@ private function getHttpClient(ResponseInterface $response)
 
     /**
      * This client will make real request if cache was not found.
-     *
-     * @return CachedResponseClient
      */
-    private function getCachedHttpClient()
+    private function getCachedHttpClient(): CachedResponseClient
     {
         try {
             $client = Psr18ClientDiscovery::find();
@@ -97,7 +97,7 @@ private function getCachedHttpClient()
         return new CachedResponseClient($client, $this->getCacheDir(), $this->getApiKey());
     }
 
-    public function testGeocodeQuery()
+    public function testGeocodeQuery(): void
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
             $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
@@ -111,18 +111,20 @@ public function testGeocodeQuery()
         $result = $provider->geocodeQuery($query);
         $this->assertWellFormattedResult($result);
 
-        // Check Downing Street
+        /** @var Location $location */
         $location = $result->first();
-        $this->assertEqualsWithDelta(51.5033, $location->getCoordinates()->getLatitude(), 0.1, 'Latitude should be in London');
-        $this->assertEqualsWithDelta(-0.1276, $location->getCoordinates()->getLongitude(), 0.1, 'Longitude should be in London');
+        /** @var Coordinates|null $coordinates */
+        $coordinates = $location->getCoordinates();
+        $this->assertNotNull($coordinates, 'Coordinates should not be null');
+        $this->assertEqualsWithDelta(51.5033, $coordinates->getLatitude(), 0.1, 'Latitude should be in London');
+        $this->assertEqualsWithDelta(-0.1276, $coordinates->getLongitude(), 0.1, 'Longitude should be in London');
+        $this->assertNotNull($location->getStreetName(), 'Street name should not be null');
         $this->assertStringContainsString('Downing', $location->getStreetName(), 'Street name should contain "Downing St"');
-
-        if (null !== $streetNumber = $location->getStreetNumber()) {
-            $this->assertStringContainsString('10', $streetNumber, 'Street number should contain "10"');
-        }
+        $this->assertNotNull($location->getStreetNumber(), 'Street number should not be null');
+        $this->assertStringContainsString('10', (string) $location->getStreetNumber(), 'Street number should contain "10"');
     }
 
-    public function testGeocodeQueryWithNoResults()
+    public function testGeocodeQueryWithNoResults(): void
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
             $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
@@ -138,7 +140,7 @@ public function testGeocodeQueryWithNoResults()
         $this->assertEquals(0, $result->count());
     }
 
-    public function testReverseQuery()
+    public function testReverseQuery(): void
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
             $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
@@ -154,7 +156,7 @@ public function testReverseQuery()
         $this->assertWellFormattedResult($result);
     }
 
-    public function testReverseQueryWithNoResults()
+    public function testReverseQueryWithNoResults(): void
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
             $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
@@ -170,7 +172,7 @@ public function testReverseQueryWithNoResults()
         $this->assertEquals(1, $result->count());
     }
 
-    public function testGeocodeIpv4()
+    public function testGeocodeIpv4(): void
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
             $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
@@ -185,7 +187,7 @@ public function testGeocodeIpv4()
         $this->assertWellFormattedResult($result);
     }
 
-    public function testGeocodeIpv6()
+    public function testGeocodeIpv6(): void
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
             $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
@@ -204,11 +206,11 @@ public function testGeocodeIpv6()
      * @dataProvider exceptionDataProvider
      *
      * @param GeocodeQuery|ReverseQuery $query
-     * @param string                    $exceptionClass
+     * @param class-string<Exception>   $exceptionClass
      * @param ResponseInterface|null    $response
      * @param string                    $message
      */
-    public function testExceptions($query, string $exceptionClass, ResponseInterface $response = null, string $message = '')
+    public function testExceptions($query, string $exceptionClass, ResponseInterface $response = null, string $message = ''): void
     {
         if (isset($this->skippedTests[__FUNCTION__])) {
             $this->markTestSkipped($this->skippedTests[__FUNCTION__]);
@@ -228,7 +230,10 @@ public function testExceptions($query, string $exceptionClass, ResponseInterface
         }
     }
 
-    public function exceptionDataProvider()
+    /**
+     * @return array<array{GeocodeQuery|ReverseQuery, class-string<Exception>, Response, string}>
+     */
+    public function exceptionDataProvider(): array
     {
         $testData = [];
 
@@ -281,7 +286,7 @@ public function exceptionDataProvider()
      *
      * @param $result
      */
-    private function assertWellFormattedResult(Collection $result)
+    private function assertWellFormattedResult(Collection $result): void
     {
         $this->assertInstanceOf(
             Collection::class,
@@ -335,7 +340,7 @@ private function assertWellFormattedResult(Collection $result)
             }
 
             // Check country
-            if (null !== $country = $location->getCountry()) {
+            if (null !== ($country = $location->getCountry())) {
                 $this->assertInstanceOf(
                     Country::class,
                     $country,
@@ -345,14 +350,14 @@ private function assertWellFormattedResult(Collection $result)
 
                 if (null !== $country->getCode()) {
                     $this->assertNotEmpty(
-                        $location->getCountry()->getCode(),
+                        $country->getCode(),
                         'The Country should not have an empty code.'
                     );
                 }
 
                 if (null !== $country->getName()) {
                     $this->assertNotEmpty(
-                        $location->getCountry()->getName(),
+                        $country->getName(),
                         'The Country should not have an empty name.'
                     );
                 }

tests/NominatimTest.php

@@ -19,20 +19,20 @@
  */
 class NominatimTest extends ProviderIntegrationTest
 {
-    protected $testIpv4 = false;
-    protected $testIpv6 = false;
+    protected bool $testIpv4 = false;
+    protected bool $testIpv6 = false;
 
     protected function createProvider(ClientInterface $httpClient)
     {
         return Nominatim::withOpenStreetMapServer($httpClient, 'Geocoder PHP/Nominatim Provider/Nominatim Test');
     }
 
-    protected function getCacheDir()
+    protected function getCacheDir(): string
     {
         return dirname(__DIR__).'/.cached_responses';
     }
 
-    protected function getApiKey()
+    protected function getApiKey(): string
     {
         return '';
     }