Merge #76

76: General refactoring r=curquiza a=codedge

This PR improves the code in various aspects

## PHP version

[Supported PHP versions](https://www.php.net/supported-versions.php) are  >=7.4 are >=8.0. Both, Symfony 4.4 and 5.0, can run using these versions.

## Strict types

Using strict types is good. You know 😉 

## Type hints

By using PHP 7.4 as minimum version type hints can be set class properties. There are still places left in the code where multiple types are used or general `object` types. These place can (should?) be refactored at a later stage.

## Comments

A lot of comments just describe the passed/parameter values and return values. Due to type hints they are obsolte.

## Composer packages

The `MeiliSearchImportCommand` needs the `Doctrine\Persistence\ManagerRegistry` injected so Doctrine cannot only be a dev requirement. 

I added the `doctrine/orm` package to the `require` section instead of `require-dev`.

This is also needed for the [Symfony recipe](symfony/recipes-contrib#1128) PR I opened.


## Tests

Some testing configuration seem to be unnecessary. I removed some configuration. Still, the tests run fine.


Co-authored-by: Holger Lösken <[email protected]>
Co-authored-by: Holger Lösken <[email protected]>

Author: 3 people

.github/workflows/tests.yml

@@ -14,7 +14,7 @@ jobs:
         runs-on: ubuntu-latest
         strategy:
             matrix:
-                php-versions: ['7.2', '7.3', '7.4', '8.0']
+                php-versions: ['7.4', '8.0']
         name: integration-tests (PHP ${{ matrix.php-versions }})
         steps:
             -   uses: actions/checkout@v2

.php_cs.dist

@@ -1,33 +1,28 @@
 <?php
 
+declare(strict_types=1);
+
 if (!file_exists(__DIR__.'/src')) {
     exit(0);
 }
 
-return PhpCsFixer\Config::create()
-    ->setRules(
-        [
+$finder = PhpCsFixer\Finder::create()
+    ->in(__DIR__.'/src')
+    ->in(__DIR__.'/tests')
+    ->append([__FILE__]);
+$config = new PhpCsFixer\Config();
+$config->setRules([
             '@Symfony' => true,
-            '@Symfony:risky' => true,
-            '@PHPUnit75Migration:risky' => true,
-            'php_unit_dedicate_assert' => ['target' => 'newest'],
-            'array_syntax' => ['syntax' => 'short'],
-            'fopen_flags' => false,
-            'protected_to_private' => false,
-            'combine_nested_dirname' => true,
+            'declare_strict_types' => true,
             'global_namespace_import' => [
-                'import_classes' => true,
-                'import_constants' => true,
-                'import_functions' => true,
+                'import_classes' => false,
+                'import_functions' => false,
+                'import_constants' => false,
             ],
-            'phpdoc_no_package' => false,
-            'no_superfluous_phpdoc_tags' => false,
-            'ordered_imports' => ['sortAlgorithm' => 'none'],
         ]
     )
     ->setRiskyAllowed(true)
-    ->setFinder(
-        PhpCsFixer\Finder::create()
-            ->in(__DIR__.'/src')
-            ->append([__FILE__])
-    );
+    ->setFinder($finder)
+;
+
+return $config;

Dockerfile

@@ -1,4 +1,4 @@
-FROM php:7.2.0-fpm-alpine as php
+FROM php:7.4-fpm-alpine as php
 
 # persistent / runtime deps
 RUN apk add --no-cache \

bors.toml

@@ -1,6 +1,4 @@
 status = [
-    'integration-tests (PHP 7.2)',
-    'integration-tests (PHP 7.3)',
     'integration-tests (PHP 7.4)',
     'integration-tests (PHP 8.0)',
     'linter-check'

composer.json

@@ -18,17 +18,17 @@
         }
     ],
     "require": {
-        "php": ">=7.2",
+        "php": "^7.4|^8.0",
         "ext-json": "*",
+        "doctrine/orm": "^2.8",
         "meilisearch/meilisearch-php": "^0.17",
         "symfony/filesystem": "^4.0 || ^5.0",
         "symfony/property-access": "^4.0 || ^5.0",
         "symfony/serializer": "^4.0 || ^5.0"
     },
     "require-dev": {
-        "doctrine/doctrine-bundle": "^2.0",
-        "doctrine/orm": "^2.5",
-        "friendsofphp/php-cs-fixer": "^2.16",
+        "doctrine/doctrine-bundle": "^2.3",
+        "friendsofphp/php-cs-fixer": "^2.18",
         "jms/serializer-bundle": "^3.0",
         "nyholm/psr7": "^1.3",
         "phpunit/phpunit": "^8.5",

docker-entrypoint.sh

@@ -12,7 +12,7 @@ if [ "$1" = 'php-fpm' ] || [ "$1" = 'php' ] || [ "$1" = 'bin/console' ]; then
   setfacl -dR -m u:www-data:rwX -m u:"$(whoami)":rwX var
 
   if [ "$APP_ENV" != 'prod' ]; then
-    composer install --prefer-dist --no-progress --no-suggest --no-interaction
+    composer install --prefer-dist --no-progress --no-interaction
   fi
 fi
 

src/Command/IndexCommand.php

@@ -1,32 +1,22 @@
 <?php
 
+declare(strict_types=1);
+
 namespace MeiliSearch\Bundle\Command;
 
 use MeiliSearch\Bundle\SearchService;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
-use function array_keys;
-use function count;
-use function explode;
 
 /**
  * Class IndexCommand.
- *
- * @package MeiliSearch\Bundle\Command
  */
 abstract class IndexCommand extends Command
 {
-    /** @var SearchService */
-    protected $searchService;
+    protected SearchService $searchService;
 
-    /**
-     * IndexCommand constructor.
-     *
-     * @param SearchService $searchService
-     * @param string|null   $name
-     */
-    public function __construct(SearchService $searchService, string $name = null)
+    public function __construct(SearchService $searchService, ?string $name = null)
     {
         $this->searchService = $searchService;
         parent::__construct($name);
@@ -38,7 +28,7 @@ protected function getEntitiesFromArgs(InputInterface $input, OutputInterface $o
         $indexNames = [];
 
         if ($indexList = $input->getOption('indices')) {
-            $indexNames = explode(',', $indexList);
+            $indexNames = \explode(',', $indexList);
         }
 
         $config = $this->searchService->getConfiguration();

src/Command/MeiliSearchClearCommand.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace MeiliSearch\Bundle\Command;
 
 use Symfony\Component\Console\Input\InputInterface;
@@ -8,17 +10,11 @@
 
 /**
  * Class MeiliSearchClearCommand.
- *
- * @package MeiliSearch\Bundle\Command
  */
 final class MeiliSearchClearCommand extends IndexCommand
 {
-    /** @var string */
     protected static $defaultName = 'meili:clear';
 
-    /**
-     * {@inheritdoc}
-     */
     protected function configure()
     {
         $this

src/Command/MeiliSearchImportCommand.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace MeiliSearch\Bundle\Command;
 
 use Doctrine\Persistence\ManagerRegistry;
@@ -8,57 +10,26 @@
 use MeiliSearch\Bundle\Model\Aggregator;
 use MeiliSearch\Bundle\SearchService;
 use MeiliSearch\Client;
-use MeiliSearch\Exceptions\ApiException;
-use MeiliSearch\Exceptions\TimeOutException;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
-use function array_key_exists;
-use function array_map;
-use function array_merge;
-use function array_unique;
-use function count;
-use function is_subclass_of;
-use function method_exists;
-use function sprintf;
-use function ucfirst;
-use const SORT_REGULAR;
 
 /**
  * Class MeiliSearchImportCommand.
- *
- * @package MeiliSearch\Bundle\Command
  */
 final class MeiliSearchImportCommand extends IndexCommand
 {
-    /**
-     * @var string
-     */
     protected static $defaultName = 'meili:import';
+    protected Client $searchClient;
+    protected ManagerRegistry $managerRegistry;
 
-    /** @var Client */
-    protected $searchClient;
-
-    /** @var ManagerRegistry */
-    protected $managerRegistry;
-
-    /**
-     * MeiliSearchImportCommand constructor.
-     *
-     * @param SearchService   $searchService
-     * @param ManagerRegistry $managerRegistry
-     * @param Client          $searchClient
-     */
     public function __construct(SearchService $searchService, ManagerRegistry $managerRegistry, Client $searchClient)
     {
         parent::__construct($searchService);
         $this->managerRegistry = $managerRegistry;
         $this->searchClient = $searchClient;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     protected function configure()
     {
         $this
@@ -72,12 +43,6 @@ protected function configure()
             );
     }
 
-    /**
-     * {@inheritdoc}
-     *
-     * @throws TimeOutException
-     * @throws ApiException
-     */
     protected function execute(InputInterface $input, OutputInterface $output): int
     {
         $entitiesToIndex = $this->getEntitiesFromArgs($input, $output);
@@ -164,11 +129,7 @@ function ($entity) {
         return 0;
     }
 
-    /**
-     * @param array $batch
-     *
-     * @return array
-     *
+    /*
      * @throws TimeOutException
      */
     private function formatIndexingResponse(array $batch): array

src/DependencyInjection/Configuration.php

@@ -1,21 +1,18 @@
 <?php
 
+declare(strict_types=1);
+
 namespace MeiliSearch\Bundle\DependencyInjection;
 
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\ConfigurationInterface;
 
 /**
  * Class Configuration.
- *
- * @package MeiliSearch\Bundle\DependencyInjection
  */
 final class Configuration implements ConfigurationInterface
 {
-    /**
-     * {@inheritdoc}
-     */
-    public function getConfigTreeBuilder()
+    public function getConfigTreeBuilder(): TreeBuilder
     {
         $treeBuilder = new TreeBuilder('meili_search');
         $rootNode = $treeBuilder->getRootNode();

src/DependencyInjection/MeiliSearchExtension.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace MeiliSearch\Bundle\DependencyInjection;
 
 use MeiliSearch\Bundle\Engine;
@@ -10,22 +12,18 @@
 use Symfony\Component\DependencyInjection\Loader;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
-use function count;
-use function dirname;
 
 /**
  * Class MeiliSearchExtension.
- *
- * @package MeiliSearch\Bundle\DependencyInjection
  */
 final class MeiliSearchExtension extends Extension
 {
     /**
      * {@inheritdoc}
      */
-    public function load(array $configs, ContainerBuilder $container)
+    public function load(array $configs, ContainerBuilder $container): void
     {
-        $loader = new Loader\XmlFileLoader($container, new FileLocator(dirname(__DIR__).'/Resources/config'));
+        $loader = new Loader\XmlFileLoader($container, new FileLocator(\dirname(__DIR__).'/Resources/config'));
         $loader->load('services.xml');
 
         $configuration = new Configuration();
@@ -35,7 +33,7 @@ public function load(array $configs, ContainerBuilder $container)
             $config['prefix'] = $container->getParameter('kernel.environment').'_';
         }
 
-        if (count($doctrineSubscribedEvents = $config['doctrineSubscribedEvents']) > 0) {
+        if (\count($doctrineSubscribedEvents = $config['doctrineSubscribedEvents']) > 0) {
             $container->getDefinition('search.search_indexer_subscriber')->setArgument(1, $doctrineSubscribedEvents);
         } else {
             $container->removeDefinition('search.search_indexer_subscriber');

src/Document/Aggregator.php

@@ -1,13 +1,13 @@
 <?php
 
+declare(strict_types=1);
+
 namespace MeiliSearch\Bundle\Document;
 
 use MeiliSearch\Bundle\Model\Aggregator as BaseAggregator;
 
 /**
  * Class Aggregator.
- *
- * @package MeiliSearch\Bundle\Document
  */
 abstract class Aggregator extends BaseAggregator
 {