refactor: Introduce Rector and apply it (#218)

Author: theofidry

Makefile

@@ -35,6 +35,9 @@ INFECTION_BIN = vendor/bin/infection
 INFECTION = $(INFECTION_BIN) --skip-initial-tests --coverage=$(COVERAGE_DIR) --only-covered --show-mutations --min-msi=$(TARGET_MSI) --min-covered-msi=$(TARGET_MSI) --ansi --threads=max
 INFECTION_WITH_INITIAL_TESTS = $(INFECTION_BIN) --only-covered --show-mutations --min-msi=$(TARGET_MSI) --min-covered-msi=$(TARGET_MSI) --ansi --threads=max
 
+RECTOR_BIN = vendor-bin/rector/vendor/bin/rector
+RECTOR = $(RECTOR_BIN)
+
 
 #
 # Commands
@@ -141,6 +144,14 @@ clear_coverage:	  ## Clears the coverage reports
 clear_coverage:
 	rm -rf $(COVERAGE_DIR) || true
 
+.PHONY: rector
+rector: $(RECTOR_BIN)
+	$(RECTOR)
+
+.PHONY: rector_lint
+rector_lint: $(RECTOR_BIN) dist
+	$(RECTOR) --dry-run
+
 
 #
 # Rules from files
@@ -176,3 +187,8 @@ $(COVERAGE_JUNIT): $(PHPUNIT_BIN) $(SRC_TESTS_FILES) phpunit.xml.dist
 
 $(INFECTION_BIN): vendor
 	touch -c $@
+
+
+$(RECTOR_BIN): vendor
+	composer bin rector install
+	touch -c $@

phpstan-tests.neon.dist

@@ -32,17 +32,8 @@ parameters:
         - path: tests/Input/ParallelizationInputTest.php
           message: '#assertTrue\(\) with false will always evaluate to false\.#'
 
-        - path: tests/Process/DummyProcessFactory.php
-          message: '#DummyProcess#'
-
-        - path: tests/Process/SymfonyProcessLauncherTest.php
-          message: '#DummyProcess#'
-
-        - path: tests/Input/RawInputTest.php
-          message: '#FakeInput#'
-
         - path: tests/ParallelExecutorFactoryTest.php
-          message: '#ParallelExecutorFactory::create\(\) expects callable#'
+          message: '#ParallelExecutorFactory::create\(\) expects Closure#'
 
         - path: tests/Fixtures/Command/LegacyCommand.php
           message: '#Static method .* is unused#'

rector.php

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of the Webmozarts Console Parallelization package.
+ *
+ * (c) Webmozarts GmbH <office@webmozarts.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Php71\Rector\FuncCall\CountOnNullRector;
+use Rector\Set\ValueObject\LevelSetList;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->paths([
+        __DIR__.'/src',
+        __DIR__.'/tests',
+    ]);
+
+    $rectorConfig->sets([
+        LevelSetList::UP_TO_PHP_81,
+    ]);
+
+    $rectorConfig->skip([
+        CountOnNullRector::class,
+    ]);
+};

src/ChunkedItemsIterator.php

@@ -20,44 +20,39 @@
 use function array_map;
 use function count;
 use function explode;
-use function gettype;
 use function is_array;
 use function is_numeric;
-use function is_object;
 use function iter\chunk;
 use function iter\mapWithKeys;
 use function iter\values;
 use function Safe\stream_get_contents;
 use function sprintf;
+use function str_contains;
 use function str_replace;
 use const PHP_EOL;
 
 final class ChunkedItemsIterator
 {
-    /**
-     * @var list<string>|Iterator<string>
-     */
-    private iterable $items;
-
     /**
      * @var Iterator<list<string>>
      */
-    private Iterator $itemsChunks;
+    private readonly Iterator $itemsChunks;
 
     /**
      * @var 0|positive-int|null
      */
-    private ?int $numberOfItems;
+    private readonly ?int $numberOfItems;
 
     /**
      * @internal Use the static factory methods instead.
      *
      * @param list<string>|Iterator<string> $items
      * @param positive-int                  $batchSize
      */
-    public function __construct(iterable $items, int $batchSize)
-    {
-        $this->items = $items;
+    public function __construct(
+        private readonly iterable $items,
+        int $batchSize,
+    ) {
         $this->itemsChunks = chunk($items, $batchSize);
         $this->numberOfItems = is_array($items) ? count($items) : null;
     }
@@ -138,18 +133,16 @@ private static function normalizeItems(array $items): array
     }
 
     /**
-     * @param mixed $items
-     *
      * @return Iterator<string>
      */
-    private static function normalizeItemStream($items): Iterator
+    private static function normalizeItemStream(mixed $items): Iterator
     {
         Assert::isIterable(
             $items,
             sprintf(
                 'Expected the fetched items to be a list or an iterable of strings. Got "%s".',
                 // TODO: use get_debug_type when dropping PHP 7.4 support
-                is_object($items) ? $items::class : gettype($items),
+                get_debug_type($items),
             ),
         );
 
@@ -161,11 +154,7 @@ private static function normalizeItemStream($items): Iterator
         );
     }
 
-    /**
-     * @param mixed     $item
-     * @param array-key $index
-     */
-    private static function normalizeItem($item, $index): string
+    private static function normalizeItem(mixed $item, int|string $index): string
     {
         if (is_numeric($item)) {
             return (string) $item;
@@ -176,12 +165,12 @@ private static function normalizeItem($item, $index): string
             sprintf(
                 'The items are potentially passed to the child processes via the STDIN. For this reason they are expected to be string values. Got "%s" for the item "%s".',
                 // TODO: use get_debug_type when dropping PHP 7.4 support
-                is_object($item) ? $item::class : gettype($item),
+                get_debug_type($item),
                 $index,
             ),
         );
         Assert::false(
-            '' !== PHP_EOL && false !== mb_strpos($item, PHP_EOL),
+            '' !== PHP_EOL && str_contains($item, PHP_EOL),
             sprintf(
                 'An item cannot contain a line return. Got one for "%s" for the item "%s".',
                 str_replace(PHP_EOL, '<lineReturn>', $item),

src/Configuration.php

@@ -21,26 +21,6 @@
 
 final class Configuration
 {
-    /**
-     * @var positive-int
-     */
-    private int $numberOfProcesses;
-
-    /**
-     * @var positive-int
-     */
-    private int $segmentSize;
-
-    /**
-     * @var positive-int|null
-     */
-    private ?int $numberOfSegments;
-
-    /**
-     * @var positive-int|0|null
-     */
-    private ?int $totalNumberOfBatches;
-
     /**
      * @internal Use the static factory methods instead.
      *
@@ -50,15 +30,11 @@ final class Configuration
      * @param positive-int|0|null $totalNumberOfBatches
      */
     public function __construct(
-        int $numberOfProcesses,
-        int $segmentSize,
-        ?int $numberOfSegments,
-        ?int $totalNumberOfBatches
+        private readonly int $numberOfProcesses,
+        private readonly int $segmentSize,
+        private readonly ?int $numberOfSegments,
+        private readonly ?int $totalNumberOfBatches,
     ) {
-        $this->numberOfProcesses = $numberOfProcesses;
-        $this->segmentSize = $segmentSize;
-        $this->numberOfSegments = $numberOfSegments;
-        $this->totalNumberOfBatches = $totalNumberOfBatches;
     }
 
     /**
@@ -72,7 +48,7 @@ public static function create(
         ?int $numberOfItems,
         int $numberOfProcesses,
         int $segmentSize,
-        int $batchSize
+        int $batchSize,
     ): self {
         // We always check those (and not the calculated ones) since they come from the command
         // configuration so an issue there hints on a misconfiguration which should be fixed.

src/Deprecation.php

@@ -25,7 +25,7 @@ final class Deprecation
      * @param string $message The message of the deprecation
      * @param mixed  ...$args Values to insert in the message using printf() formatting
      */
-    public static function trigger(string $message, ...$args): void
+    public static function trigger(string $message, mixed ...$args): void
     {
         trigger_deprecation(
             'webmozarts/console-parallelization',

src/ErrorHandler/LoggingErrorHandler.php

@@ -18,11 +18,9 @@
 
 final class LoggingErrorHandler implements ErrorHandler
 {
-    private ErrorHandler $decoratedErrorHandler;
-
-    public function __construct(?ErrorHandler $decoratedErrorHandler = null)
-    {
-        $this->decoratedErrorHandler = $decoratedErrorHandler ?? new NullErrorHandler();
+    public function __construct(
+        private readonly ErrorHandler $decoratedErrorHandler = new NullErrorHandler(),
+    ) {
     }
 
     public function handleError(string $item, Throwable $throwable, Logger $logger): int

src/ErrorHandler/ResetServiceErrorHandler.php

@@ -20,15 +20,10 @@
 
 final class ResetServiceErrorHandler implements ErrorHandler
 {
-    private ResetInterface $resettable;
-    private ErrorHandler $decoratedErrorHandler;
-
     public function __construct(
-        ResetInterface $resettable,
-        ?ErrorHandler $decoratedErrorHandler = null
+        private readonly ResetInterface $resettable,
+        private readonly ErrorHandler $decoratedErrorHandler = new NullErrorHandler(),
     ) {
-        $this->resettable = $resettable;
-        $this->decoratedErrorHandler = $decoratedErrorHandler ?? new NullErrorHandler();
     }
 
     public static function forContainer(?ContainerInterface $container): ErrorHandler

src/ErrorHandler/ThrowableCodeErrorHandler.php

@@ -16,14 +16,13 @@
 use Throwable;
 use Webmozart\Assert\Assert;
 use Webmozarts\Console\Parallelization\Logger\Logger;
+use function max;
 
 final class ThrowableCodeErrorHandler implements ErrorHandler
 {
-    private ErrorHandler $decoratedErrorHandler;
-
-    public function __construct(?ErrorHandler $decoratedErrorHandler = null)
-    {
-        $this->decoratedErrorHandler = $decoratedErrorHandler ?? new NullErrorHandler();
+    public function __construct(
+        private readonly ErrorHandler $decoratedErrorHandler = new NullErrorHandler(),
+    ) {
     }
 
     public function handleError(string $item, Throwable $throwable, Logger $logger): int

src/Input/ChildCommandFactory.php

@@ -18,45 +18,30 @@
 use Webmozart\Assert\Assert;
 use function array_filter;
 use function array_map;
-use function array_merge;
 use function explode;
 use function Safe\getcwd;
 use function sprintf;
-use function strval;
 
 /**
  * @internal
  */
 final class ChildCommandFactory
 {
-    private string $phpExecutable;
-    private string $scriptPath;
-    private string $commandName;
-    private InputDefinition $commandDefinition;
-
     public function __construct(
-        string $phpExecutable,
-        string $scriptPath,
-        string $commandName,
-        InputDefinition $commandDefinition
+        private readonly string $phpExecutable,
+        private readonly string $scriptPath,
+        private readonly string $commandName,
+        private readonly InputDefinition $commandDefinition,
     ) {
         self::validateScriptPath($scriptPath);
-
-        $this->phpExecutable = $phpExecutable;
-        $this->scriptPath = $scriptPath;
-        $this->commandName = $commandName;
-        $this->commandDefinition = $commandDefinition;
     }
 
     /**
      * @return list<string>
      */
     public function createChildCommand(InputInterface $input): array
     {
-        return array_merge(
-            $this->createBaseCommand($input),
-            $this->getForwardedOptions($input),
-        );
+        return [...$this->createBaseCommand($input), ...$this->getForwardedOptions($input)];
     }
 
     /**