Merge pull request #54 from openfoodfacts/improve-php-code

feat: improve ci

.github/workflows/ci.yml

@@ -48,9 +48,15 @@ jobs:
         run: |
           composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update
           composer update --${{ matrix.dependency-version }} --prefer-dist --no-interaction
+      - name: Check quality code
+        env:
+            PHP_CS_FIXER_IGNORE_ENV: 1
+        run: vendor/bin/php-cs-fixer fix --ansi --dry-run --using-cache=no --verbose
+      - name: Execute phpstan
+        run: vendor/bin/phpstan
       - name: Execute tests
         run: vendor/bin/phpunit --coverage-clover ./build/coverage.clover
       - name: Uploading code coverage to scrutinize
         uses: sudo-bot/action-scrutinizer@latest
         with:
-            cli-args: "--format=php-clover build/coverage.clover"
+            cli-args: "--format=php-clover build/coverage.clover"

.php-cs-fixer.dist.php

@@ -0,0 +1,22 @@
+<?php
+
+$config = new PhpCsFixer\Config();
+$finder = $config->getFinder()
+    ->in(['src','tests',])
+    ->name('*.php')
+    ->notName('*.blade.php')
+    ->ignoreDotFiles(true)
+    ->ignoreVCS(true);
+
+return $config
+    ->setRules([
+        '@PSR12' => true,
+        'array_indentation' => true,
+        'array_syntax' => ['syntax' => 'short'],
+        'single_quote' => true,
+        'blank_line_before_statement' => true,
+        'no_spaces_around_offset' => true,
+        'no_unused_imports' => true,
+        'ternary_operator_spaces' => true,
+    ])
+    ->setUsingCache(false);

Makefile

@@ -0,0 +1,20 @@
+
+
+.PHONY: ci
+ci: cs-check phpstan test
+
+.PHONY: cs-check
+cs-check:
+	php ./vendor/bin/php-cs-fixer check
+
+.PHONY: cs-fix
+cs-fix:
+	php ./vendor/bin/php-cs-fixer fix
+
+.PHONY: test
+test:
+	php ./vendor/bin/phpunit
+
+.PHONY: phpstan
+phpstan:
+	php ./vendor/bin/phpstan

composer.json

@@ -23,7 +23,10 @@
     },
     "require-dev": {
         "friendsofphp/php-cs-fixer": "^3.9.5",
+        "larastan/larastan": "^2.9",
         "orchestra/testbench": "^7.0|^8.0|^9.0",
+        "phpstan/phpstan": "^1.10",
+        "phpstan/phpstan-phpunit": "^1.3",
         "phpunit/phpunit": "^9.5.21|^10.5"
     },
     "autoload": {

phpstan.neon.dist

@@ -0,0 +1,15 @@
+includes:
+  - vendor/phpstan/phpstan-phpunit/extension.neon
+  - vendor/phpstan/phpstan-phpunit/rules.neon
+  - vendor/larastan/larastan/extension.neon
+
+parameters:
+  level: 9
+  paths:
+    - src
+    - tests
+  ignoreErrors:
+    - identifier: missingType.iterableValue
+    -
+      message: "#^Cannot access offset '[a-z.]+' on Illuminate\\\\Contracts\\\\Container\\\\Container.$#"
+      path: src/OpenFoodFacts.php

src/Facades/OpenFoodFacts.php

@@ -3,15 +3,21 @@
 namespace OpenFoodFacts\Laravel\Facades;
 
 use Illuminate\Support\Facades\Facade;
+use Illuminate\Support\Collection;
+use OpenFoodFacts\Document;
 
 /**
  * @method static array barcode(string $value)
- * @method static \Illuminate\Support\Collection find(string $searchterm)
+ * @method static Collection find(string $searchTerm)
+ * @method static Document getProduct(string $barcode)
  *
  * @see \OpenFoodFacts\Laravel\OpenFoodFacts
  */
 class OpenFoodFacts extends Facade
 {
+    /**
+     * @inheritDoc
+     */
     protected static function getFacadeAccessor()
     {
         return 'openfoodfacts';

src/OpenFoodFacts.php

@@ -5,18 +5,20 @@
 use Illuminate\Contracts\Container\Container;
 use Illuminate\Support\Collection;
 use InvalidArgumentException;
+use OpenFoodFacts\Api;
+use OpenFoodFacts\Document;
 use OpenFoodFacts\Exception\ProductNotFoundException;
 
-/** @mixin \OpenFoodFacts\Api */
+/** @mixin Api */
 class OpenFoodFacts extends OpenFoodFactsApiWrapper
 {
-    protected $max_results;
+    protected int $max_results;
 
     public function __construct(Container $app, string $geography = null)
     {
         parent::__construct([
-            'geography' =>  $geography ?? $app['config']->get('openfoodfacts.geography'),
-            'app' =>  $app['config']->get('app.name'),
+            'geography' => $geography ?? $app['config']->get('openfoodfacts.geography'),
+            'app' => $app['config']->get('app.name'),
         ], $app['cache.store']);
 
         $this->max_results = $app['config']->get('openfoodfacts.max_results', 1000);
@@ -31,14 +33,14 @@ public function __construct(Container $app, string $geography = null)
     public function barcode(string $value): array
     {
         if (empty($value)) {
-            throw new InvalidArgumentException("Argument must represent a barcode");
+            throw new InvalidArgumentException('Argument must represent a barcode');
         }
 
         try {
             $doc = $this->api->getProduct($value);
 
-            return empty($doc->code) ? [] : reset($doc);
-        } catch (ProductNotFoundException $notFoundException) {
+            return empty($doc->code) ? [] : (array) reset($doc);
+        } catch (ProductNotFoundException) {
             return [];
         }
     }
@@ -47,14 +49,15 @@ public function barcode(string $value): array
      * Search products by term
      *
      * @param string $searchterm
-     * @return Collection<array>
+     * @return Collection<int, array>
      */
     public function find(string $searchterm): Collection
     {
         if (empty($searchterm)) {
-            throw new InvalidArgumentException("Specify a search term to find data for matching products");
+            throw new InvalidArgumentException('Specify a search term to find data for matching products');
         }
 
+        /** @var Collection<int, Document> $products */
         $products = Collection::make();
         $page = 0;
 
@@ -67,16 +70,25 @@ public function find(string $searchterm): Collection
             }
 
             $pages = (int)ceil($totalMatches / $pageResults->getPageSize());
+            /** @var Document[] $array */
+            $array = iterator_to_array($pageResults);
 
-            $products = $products->concat(iterator_to_array($pageResults));
+            $products = $products->concat($array);
         } while ($page < $pages);
 
         return $products->map(function ($product) {
-            return reset($product);
+            /**
+             * @var array $value
+             * Just to avoid error from phpstan :
+             * > Method OpenFoodFacts\Laravel\OpenFoodFacts::find() should return Illuminate\Support\Collection<int, array> but returns Illuminate\Support\Collection<int, array{bool}>.
+             */
+            $value = (array) reset($product);
+
+            return $value;
         });
     }
 
-    public function __call($method, $parameters)
+    public function __call(string $method, array $parameters): mixed
     {
         return $this->api->$method(...$parameters);
     }

src/OpenFoodFactsApiWrapper.php

@@ -8,11 +8,11 @@
 
 class OpenFoodFactsApiWrapper
 {
-    public $api;
+    public readonly Api $api;
 
     public function __construct(
-        protected array $parameters,
-        protected ?CacheInterface $cache = null,
+        public readonly array $parameters,
+        protected readonly ?CacheInterface $cache = null,
         string $environment = null
     ) {
         $this->api = $this->setupApi($environment);
@@ -29,7 +29,7 @@ protected function setupApi(string $environment = null): Api
         );
     }
 
-    protected function httpClient()
+    protected function httpClient(): Client
     {
         return new Client([
             'headers' => ['User-Agent' => $this->parameters['app'] ?? 'Laravel Open Food Facts - https://github.com/openfoodfacts/openfoodfacts-laravel'],

src/OpenFoodFactsServiceProvider.php

@@ -7,7 +7,7 @@
 
 class OpenFoodFactsServiceProvider extends ServiceProvider
 {
-    public function boot()
+    public function boot(): void
     {
         if ($this->app->runningInConsole()) {
             $this->publishes([
@@ -16,6 +16,9 @@ public function boot()
         }
     }
 
+    /**
+     * @inheritDoc
+     */
     public function register()
     {
         $this->mergeConfigFrom(__DIR__.'/../config/openfoodfacts.php', 'openfoodfacts');

tests/BarcodeFindTest.php

@@ -7,26 +7,25 @@
 class BarcodeFindTest extends Base\FacadeTestCase
 {
     /** @test */
-    public function it_returns_an_array_with_data_when_product_found()
+    public function it_returns_an_array_with_data_when_product_found(): void
     {
         $arr = OpenFoodFacts::barcode('0737628064502');
-
         $this->assertArrayHasKey('code', $arr);
     }
 
     /** @test */
-    public function it_returns_an_empty_array_when_product_not_found()
+    public function it_returns_an_empty_array_when_product_not_found(): void
     {
         $arr = OpenFoodFacts::barcode('this-barcode-does-not-exist');
 
         $this->assertEquals([], $arr);
     }
 
     /** @test */
-    public function it_throws_an_exception_when_argument_empty()
+    public function it_throws_an_exception_when_argument_empty(): void
     {
-        $this->expectException("InvalidArgumentException");
-        $this->expectExceptionMessage("Argument must represent a barcode");
+        $this->expectException('InvalidArgumentException');
+        $this->expectExceptionMessage('Argument must represent a barcode');
 
         OpenFoodFacts::barcode('');
     }

tests/Base/FacadeTestCase.php

@@ -2,12 +2,12 @@
 
 namespace OpenFoodFacts\Laravel\Tests\Base;
 
-use Orchestra\Testbench\TestCase;
 use OpenFoodFacts\Laravel\OpenFoodFactsServiceProvider;
+use Orchestra\Testbench\TestCase;
 
 abstract class FacadeTestCase extends TestCase
 {
-    protected function getPackageProviders($app)
+    protected function getPackageProviders($app): array
     {
         return [OpenFoodFactsServiceProvider::class];
     }

tests/FacadeBridgeToApiTest.php

@@ -2,15 +2,16 @@
 
 namespace OpenFoodFacts\Laravel\Tests;
 
+use OpenFoodFacts\Document;
 use OpenFoodFacts\Laravel\Facades\OpenFoodFacts;
 
 class FacadeBridgeToApiTest extends Base\FacadeTestCase
 {
     /** @test */
-    public function it_calls_method_on_vendor_apiclass()
+    public function it_calls_method_on_vendor_apiclass(): void
     {
         $doc = OpenFoodFacts::getProduct('0737628064502');
 
-        $this->assertInstanceOf(\OpenFoodFacts\Document::class, $doc);
+        $this->assertInstanceOf(Document::class, $doc);
     }
 }

tests/GeographyTest.php

@@ -9,7 +9,7 @@
 class GeographyTest extends Base\FacadeTestCase
 {
     /** @test */
-    public function it_returns_different_content_based_on_geography_parameter()
+    public function it_returns_different_content_based_on_geography_parameter(): void
     {
         $barcode = '8714200216964';
 
@@ -24,7 +24,7 @@ public function it_returns_different_content_based_on_geography_parameter()
     }
 
     /** @test */
-    public function it_returns_geo_specific_content_through_config_setting()
+    public function it_returns_geo_specific_content_through_config_setting(): void
     {
         Config::set('openfoodfacts.geography', 'nl');
         $product_dutch_content = OpenFoodFactsFacade::barcode('8714200216964');