From 339eb4d65484aba3c0381f29ef0fe475b0b72ae3 Mon Sep 17 00:00:00 2001 From: Benoit Colin Date: Thu, 11 Jul 2024 20:21:17 +0200 Subject: [PATCH 1/3] feat: add code style --- .github/workflows/ci.yml | 8 +++++- .php-cs-fixer.dist.php | 22 +++++++++++++++ .php_cs.dist.php | 40 ---------------------------- Makefile | 20 ++++++++++++++ composer.json | 3 +++ phpstan.neon.dist | 15 +++++++++++ src/Facades/OpenFoodFacts.php | 8 +++++- src/OpenFoodFacts.php | 29 +++++++++++--------- src/OpenFoodFactsApiWrapper.php | 8 +++--- src/OpenFoodFactsServiceProvider.php | 5 +++- tests/BarcodeFindTest.php | 11 ++++---- tests/Base/FacadeTestCase.php | 4 +-- tests/FacadeBridgeToApiTest.php | 5 ++-- tests/GeographyTest.php | 4 +-- tests/ProductSearchTest.php | 14 +++++----- 15 files changed, 118 insertions(+), 78 deletions(-) create mode 100644 .php-cs-fixer.dist.php delete mode 100644 .php_cs.dist.php create mode 100644 Makefile create mode 100644 phpstan.neon.dist diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 759dcac..eeb3399 100644 --- a/.github/workflows/ci.yml +++ b/.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" \ No newline at end of file + cli-args: "--format=php-clover build/coverage.clover" diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php new file mode 100644 index 0000000..386e405 --- /dev/null +++ b/.php-cs-fixer.dist.php @@ -0,0 +1,22 @@ +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); diff --git a/.php_cs.dist.php b/.php_cs.dist.php deleted file mode 100644 index 6478dd8..0000000 --- a/.php_cs.dist.php +++ /dev/null @@ -1,40 +0,0 @@ -in([ - __DIR__ . '/src', - __DIR__ . '/tests', - ]) - ->name('*.php') - ->notName('*.blade.php') - ->ignoreDotFiles(true) - ->ignoreVCS(true); - -return (new PhpCsFixer\Config()) - ->setRules([ - '@PSR12' => true, - 'array_syntax' => ['syntax' => 'short'], - 'ordered_imports' => ['sort_algorithm' => 'alpha'], - 'no_unused_imports' => true, - 'not_operator_with_successor_space' => true, - 'trailing_comma_in_multiline' => true, - 'phpdoc_scalar' => true, - 'unary_operator_spaces' => true, - 'binary_operator_spaces' => true, - 'blank_line_before_statement' => [ - 'statements' => ['break', 'continue', 'declare', 'return', 'throw', 'try'], - ], - 'phpdoc_single_line_var_spacing' => true, - 'phpdoc_var_without_name' => true, - 'class_attributes_separation' => [ - 'elements' => [ - 'method' => 'one', - ], - ], - 'method_argument_space' => [ - 'on_multiline' => 'ensure_fully_multiline', - 'keep_multiple_spaces_after_comma' => true, - ], - 'single_trait_insert_per_statement' => true, - ]) - ->setFinder($finder); diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..4abcb0b --- /dev/null +++ b/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 diff --git a/composer.json b/composer.json index 49c301f..188d4f4 100644 --- a/composer.json +++ b/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": { diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..e85fe25 --- /dev/null +++ b/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 + checkMissingIterableValueType: false + paths: + - src + - tests + ignoreErrors: + - + message: "#^Cannot access offset '[a-z.]+' on Illuminate\\\\Contracts\\\\Container\\\\Container.$#" + path: src/OpenFoodFacts.php diff --git a/src/Facades/OpenFoodFacts.php b/src/Facades/OpenFoodFacts.php index e6292a8..0c6fc45 100644 --- a/src/Facades/OpenFoodFacts.php +++ b/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'; diff --git a/src/OpenFoodFacts.php b/src/OpenFoodFacts.php index daa0273..740c36e 100644 --- a/src/OpenFoodFacts.php +++ b/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 + * @return Collection */ 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 $products */ $products = Collection::make(); $page = 0; @@ -67,16 +70,18 @@ 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); + return (array) reset($product); }); } - public function __call($method, $parameters) + public function __call(string $method, array $parameters): mixed { return $this->api->$method(...$parameters); } diff --git a/src/OpenFoodFactsApiWrapper.php b/src/OpenFoodFactsApiWrapper.php index ddaaaf1..ca8fea0 100644 --- a/src/OpenFoodFactsApiWrapper.php +++ b/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'], diff --git a/src/OpenFoodFactsServiceProvider.php b/src/OpenFoodFactsServiceProvider.php index a93e2c0..309c79b 100644 --- a/src/OpenFoodFactsServiceProvider.php +++ b/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'); diff --git a/tests/BarcodeFindTest.php b/tests/BarcodeFindTest.php index 0967b49..17e977e 100644 --- a/tests/BarcodeFindTest.php +++ b/tests/BarcodeFindTest.php @@ -7,15 +7,14 @@ 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'); @@ -23,10 +22,10 @@ public function it_returns_an_empty_array_when_product_not_found() } /** @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(''); } diff --git a/tests/Base/FacadeTestCase.php b/tests/Base/FacadeTestCase.php index 27bef2e..798b425 100644 --- a/tests/Base/FacadeTestCase.php +++ b/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]; } diff --git a/tests/FacadeBridgeToApiTest.php b/tests/FacadeBridgeToApiTest.php index 8099375..4c58015 100644 --- a/tests/FacadeBridgeToApiTest.php +++ b/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); } } diff --git a/tests/GeographyTest.php b/tests/GeographyTest.php index 49ed89b..e343f04 100644 --- a/tests/GeographyTest.php +++ b/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'); diff --git a/tests/ProductSearchTest.php b/tests/ProductSearchTest.php index 919149c..b4847f3 100644 --- a/tests/ProductSearchTest.php +++ b/tests/ProductSearchTest.php @@ -7,7 +7,7 @@ class ProductSearchTest extends Base\FacadeTestCase { /** @test */ - public function it_returns_a_laravelcollection_with_arrays() + public function it_returns_a_laravelcollection_with_arrays(): void { $results = OpenFoodFacts::find('Stir-Fry Rice Noodles'); @@ -21,7 +21,7 @@ public function it_returns_a_laravelcollection_with_arrays() } /** @test */ - public function it_returns_an_empty_laravelcollection_when_no_results_found() + public function it_returns_an_empty_laravelcollection_when_no_results_found(): void { $results = OpenFoodFacts::find('no-such-product-exists'); @@ -29,18 +29,18 @@ public function it_returns_an_empty_laravelcollection_when_no_results_found() } /** @test */ - public function it_throws_an_exception_on_too_many_results() + public function it_throws_an_exception_on_too_many_results(): void { - $this->expectException("Exception"); + $this->expectException('Exception'); OpenFoodFacts::find('cola'); } /** @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("Specify a search term to find data for matching products"); + $this->expectException('InvalidArgumentException'); + $this->expectExceptionMessage('Specify a search term to find data for matching products'); OpenFoodFacts::find(''); } From 998e47900a9633725dab35b96239818b7088662d Mon Sep 17 00:00:00 2001 From: Benoit Colin Date: Thu, 11 Jul 2024 20:42:08 +0200 Subject: [PATCH 2/3] feat: phpstan fix --- phpstan.neon.dist | 2 +- src/OpenFoodFacts.php | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index e85fe25..2d272f9 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -5,11 +5,11 @@ includes: parameters: level: 9 - checkMissingIterableValueType: false paths: - src - tests ignoreErrors: + - identifier: missingType.iterableValue - message: "#^Cannot access offset '[a-z.]+' on Illuminate\\\\Contracts\\\\Container\\\\Container.$#" path: src/OpenFoodFacts.php diff --git a/src/OpenFoodFacts.php b/src/OpenFoodFacts.php index 740c36e..a9dad28 100644 --- a/src/OpenFoodFacts.php +++ b/src/OpenFoodFacts.php @@ -77,7 +77,14 @@ public function find(string $searchterm): Collection } while ($page < $pages); return $products->map(function ($product) { - return (array) reset($product); + /** + * @var array $value + * Just to avoid error from phpstan : + * > Method OpenFoodFacts\Laravel\OpenFoodFacts::find() should return Illuminate\Support\Collection but returns Illuminate\Support\Collection. + */ + $value = (array) reset($product); + + return $value; }); } From 5361de8a8542f3429c97b3b27bd68a6f3840fe0d Mon Sep 17 00:00:00 2001 From: Benoit Colin Date: Thu, 26 Sep 2024 09:22:56 +0200 Subject: [PATCH 3/3] feat: phpstan fix --- tests/ProductSearchTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/ProductSearchTest.php b/tests/ProductSearchTest.php index b4847f3..3d72deb 100644 --- a/tests/ProductSearchTest.php +++ b/tests/ProductSearchTest.php @@ -25,7 +25,9 @@ public function it_returns_an_empty_laravelcollection_when_no_results_found(): v { $results = OpenFoodFacts::find('no-such-product-exists'); - $this->assertTrue($results->isEmpty()); + // Call to method PHPUnit\Framework\Assert::assertTrue() with bool will always evaluate to false. + // 💡 Because the type is coming from a PHPDoc + $this->assertEquals(true, $results->isEmpty()); } /** @test */