Improve TypeHinting (#3)

Signed-off-by: Lukas Kämmerling <[email protected]>
PromPHP · Oct 26, 2020 · a353ed1 · a353ed1
1 parent 09e7574
commit a353ed1
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 34 deletions.
diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml
@@ -0,0 +1,33 @@
+name: Code Checks
+
+on: [ push, pull_request ]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: true
+
+    name: Code Checks
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v2
+
+      - name: Cache dependencies
+        uses: actions/cache@v2
+        with:
+          path: ~/.composer/cache/files
+          key: dependencies-code-checks
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 7.4
+          coverage: none
+
+      - name: Install dependencies
+        run: composer install --prefer-dist --no-interaction --no-suggest
+      - name: Run PHP Code Sniffer
+        run: ./vendor/bin/phpcs
+      - name: Run PHPStan
+        run: php vendor/bin/phpstan analyse --autoload-file tests/bootstrap.php
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -32,7 +32,5 @@ jobs:
 
       - name: Install dependencies
         run: composer update --${{ matrix.dependency-version }} --prefer-dist --no-interaction --no-suggest
-      - name: Run PHP Code Sniffer
-        run: ./vendor/bin/phpcs
       - name: Execute tests
         run: vendor/bin/phpunit
diff --git a/composer.json b/composer.json
@@ -9,7 +9,11 @@
   },
   "require-dev": {
     "phpunit/phpunit": "^8.4",
-    "squizlabs/php_codesniffer": "^3.5"
+    "squizlabs/php_codesniffer": "^3.5",
+    "phpstan/extension-installer": "^1.0",
+    "phpstan/phpstan": "^0.12.50",
+    "phpstan/phpstan-phpunit": "^0.12.16",
+    "phpstan/phpstan-strict-rules": "^0.12.5"
   },
   "license": "Apache-2.0",
   "authors": [

diff --git a/examples/pushgateway.php b/examples/pushgateway.php
@@ -5,16 +5,17 @@
 use Prometheus\CollectorRegistry;
 use Prometheus\Storage\Redis;
 use PrometheusPushGateway\PushGateway;
-
+use Prometheus\Storage\APC;
+use Prometheus\Storage\InMemory;
 $adapter = $_GET['adapter'];
 
 if ($adapter === 'redis') {
     Redis::setDefaultOptions(['host' => $_SERVER['REDIS_HOST'] ?? '127.0.0.1']);
-    $adapter = new Prometheus\Storage\Redis();
+    $adapter = new Redis();
 } elseif ($adapter === 'apc') {
-    $adapter = new Prometheus\Storage\APC();
+    $adapter = new APC();
 } elseif ($adapter === 'in-memory') {
-    $adapter = new Prometheus\Storage\InMemory();
+    $adapter = new InMemory();
 }
 
 $registry = new CollectorRegistry($adapter);

diff --git a/php-fpm/Dockerfile b/php-fpm/Dockerfile
@@ -1,6 +1,6 @@
-FROM php:7.2-fpm
+FROM php:7.4-fpm
 
-RUN pecl install redis-5.3.1 && docker-php-ext-enable redis
+RUN pecl install redis-5.3.2 && docker-php-ext-enable redis
 RUN pecl install apcu-5.1.19 && docker-php-ext-enable apcu
 
 COPY www.conf /usr/local/etc/php-fpm.d/

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -0,0 +1,10 @@
+parameters:
+    level: 8
+    paths:
+        - src
+        - tests
+    reportUnmatchedIgnoredErrors: false
+    ignoreErrors:
+        -
+            # REDIS stuff only runs with it available
+            '#Constant REDIS_HOST not found\.#'
diff --git a/src/PrometheusPushGateway/PushGateway.php b/src/PrometheusPushGateway/PushGateway.php
@@ -13,6 +13,9 @@
 
 class PushGateway
 {
+    const HTTP_PUT = "PUT";
+    const HTTP_POST = "POST";
+    const HTTP_DELETE = "DELETE";
     /**
      * @var string
      */
@@ -26,9 +29,9 @@ class PushGateway
     /**
      * PushGateway constructor.
      * @param string $address (http|https)://host:port of the push gateway
-     * @param ClientInterface $client
+     * @param ClientInterface|null $client
      */
-    public function __construct($address, ClientInterface $client = null)
+    public function __construct(string $address, ?ClientInterface $client = null)
     {
         $this->address = strpos($address, 'http') === false ? 'http://' . $address : $address;
         $this->client = $client ?? new Client();
@@ -39,50 +42,50 @@ public function __construct($address, ClientInterface $client = null)
      * Uses HTTP PUT.
      * @param CollectorRegistry $collectorRegistry
      * @param string $job
-     * @param array $groupingKey
+     * @param array<string> $groupingKey
      * @throws GuzzleException
      */
     public function push(CollectorRegistry $collectorRegistry, string $job, array $groupingKey = []): void
     {
-        $this->doRequest($collectorRegistry, $job, $groupingKey, 'put');
+        $this->doRequest($collectorRegistry, $job, $groupingKey, self::HTTP_PUT);
     }
 
     /**
      * Pushes all metrics in a Collector, replacing only previously pushed metrics of the same name and job.
      * Uses HTTP POST.
      * @param CollectorRegistry $collectorRegistry
-     * @param $job
-     * @param $groupingKey
+     * @param string $job
+     * @param array<string> $groupingKey
      * @throws GuzzleException
      */
     public function pushAdd(CollectorRegistry $collectorRegistry, string $job, array $groupingKey = []): void
     {
-        $this->doRequest($collectorRegistry, $job, $groupingKey, 'post');
+        $this->doRequest($collectorRegistry, $job, $groupingKey, self::HTTP_POST);
     }
 
     /**
      * Deletes metrics from the Push Gateway.
      * Uses HTTP POST.
      * @param string $job
-     * @param array $groupingKey
+     * @param array<string> $groupingKey
      * @throws GuzzleException
      */
     public function delete(string $job, array $groupingKey = []): void
     {
-        $this->doRequest(null, $job, $groupingKey, 'delete');
+        $this->doRequest(null, $job, $groupingKey, self::HTTP_DELETE);
     }
 
     /**
      * @param CollectorRegistry|null $collectorRegistry
      * @param string $job
-     * @param array $groupingKey
+     * @param array<string> $groupingKey
      * @param string $method
      * @throws GuzzleException
      */
-    private function doRequest(?CollectorRegistry $collectorRegistry, string $job, array $groupingKey, $method): void
+    private function doRequest(?CollectorRegistry $collectorRegistry, string $job, array $groupingKey, string $method): void
     {
         $url = $this->address . "/metrics/job/" . $job;
-        if (!empty($groupingKey)) {
+        if ($groupingKey !== []) {
             foreach ($groupingKey as $label => $value) {
                 $url .= "/" . $label . "/" . $value;
             }
@@ -96,13 +99,13 @@ private function doRequest(?CollectorRegistry $collectorRegistry, string $job, a
             'timeout' => 20,
         ];
 
-        if ($method != 'delete') {
+        if ($method !== self::HTTP_DELETE && $collectorRegistry !== null) {
             $renderer = new RenderTextFormat();
             $requestOptions['body'] = $renderer->render($collectorRegistry->getMetricFamilySamples());
         }
         $response = $this->client->request($method, $url, $requestOptions);
         $statusCode = $response->getStatusCode();
-        if (!in_array($statusCode, [200, 202])) {
+        if (!in_array($statusCode, [200, 202], true)) {
             $msg = "Unexpected status code "
                 . $statusCode
                 . " received from push gateway "

diff --git a/tests/Test/BlackBoxPushGatewayTest.php b/tests/Test/BlackBoxPushGatewayTest.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Test;
 
 use GuzzleHttp\Client;
@@ -13,7 +15,7 @@ class BlackBoxPushGatewayTest extends TestCase
     /**
      * @test
      */
-    public function pushGatewayShouldWork()
+    public function pushGatewayShouldWork(): void
     {
         $adapter = new InMemory();
         $registry = new CollectorRegistry($adapter);
@@ -26,7 +28,7 @@ public function pushGatewayShouldWork()
 
         $httpClient = new Client();
         $metrics = $httpClient->get("http://pushgateway:9091/metrics")->getBody()->getContents();
-        $this->assertStringContainsString(
+        self::assertStringContainsString(
             '# HELP test_some_counter it increases
 # TYPE test_some_counter counter
 test_some_counter{instance="foo",job="my_job",type="blue"} 6',
@@ -38,7 +40,7 @@ public function pushGatewayShouldWork()
 
         $httpClient = new Client();
         $metrics = $httpClient->get("http://pushgateway:9091/metrics")->getBody()->getContents();
-        $this->assertStringNotContainsString(
+        self::assertStringNotContainsString(
             '# HELP test_some_counter it increases
 # TYPE test_some_counter counter
 test_some_counter{instance="foo",job="my_job",type="blue"} 6',

diff --git a/tests/Test/PrometheusPushGateway/PushGatewayTest.php b/tests/Test/PrometheusPushGateway/PushGatewayTest.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Test\PrometheusPushGateway;
 
 use GuzzleHttp\Client;
@@ -43,7 +45,7 @@ public function validResponseShouldNotThrowException(): void
      */
     public function invalidResponseShouldThrowRuntimeException(): void
     {
-        $this->expectException(\RuntimeException::class);
+        self::expectException(\RuntimeException::class);
 
         $mockedCollectorRegistry = $this->createMock(CollectorRegistry::class);
         $mockedCollectorRegistry->method('getMetricFamilySamples')->with()->willReturn([
@@ -81,6 +83,11 @@ public function clientGetsDefinedIfNotSpecified(): void
      * @test
      *
      * @dataProvider validAddressAndRequestsProvider
+     * @param string $address
+     * @param string $scheme
+     * @param string $host
+     * @param int $port
+     * @throws \GuzzleHttp\Exception\GuzzleException
      */
     public function validAddressShouldCreateValidRequests(string $address, string $scheme, string $host, int $port): void
     {
@@ -97,15 +104,21 @@ public function validAddressShouldCreateValidRequests(string $address, string $s
 
         $pushGateway = new PushGateway($address, $client);
         $pushGateway->push($mockedCollectorRegistry, 'foo');
-
-        $uri = $mockHandler->getLastRequest()->getUri();
-        $this->assertEquals($scheme, $uri->getScheme());
-        $this->assertEquals($host, $uri->getHost());
-        $this->assertEquals($port, $uri->getPort());
-        $this->assertEquals('/metrics/job/foo', $uri->getPath());
+        if ($mockHandler->getLastRequest() !== null) {
+            $uri = $mockHandler->getLastRequest()->getUri();
+            self::assertEquals($scheme, $uri->getScheme());
+            self::assertEquals($host, $uri->getHost());
+            self::assertEquals($port, $uri->getPort());
+            self::assertEquals('/metrics/job/foo', $uri->getPath());
+        } else {
+            self::fail("No request performed");
+        }
     }
 
-    public function validAddressAndRequestsProvider()
+    /**
+     * @return array[]
+     */
+    public function validAddressAndRequestsProvider(): array
     {
         return [
             ['foo.bar:123', 'http', 'foo.bar', 123],

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 error_reporting(-1);
 date_default_timezone_set('UTC');
 $autoload = __DIR__ . '/../vendor/autoload.php';