Rework InputOptionsSerializer (#77)

In #28, the feature to forward the command options was introduced. It had to later be followed up by #41, #42 in order to fix the issue where the option value may require quoting. This piece of code is heavily under-tested (and it was really hard to test it as is) and is prone to bugs (I found 2 adding tests).

In this PR, I'm extracting this element into `InputOptionsSerializer` as a closed piece (static & final) as IMO there is no need to make it extensible: if broken it needs to be fixed. This extraction allows much easier testing.

This PR also fixes:

- the case of negatable options
- detection quoting with may fail to detect some special characters

Author: theofidry

src/InputOptionsSerializer.php

@@ -0,0 +1,137 @@
+<?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);
+
+namespace Webmozarts\Console\Parallelization;
+
+use function array_diff_key;
+use function array_fill_keys;
+use function array_keys;
+use function array_map;
+use function implode;
+use function is_string;
+use function preg_match;
+use function sprintf;
+use function str_replace;
+use Symfony\Component\Console\Input\InputDefinition;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+
+final class InputOptionsSerializer
+{
+    private function __construct()
+    {
+    }
+
+    /**
+     * @param string[] $excludedOptionNames
+     *
+     * @return string[]
+     */
+    public static function serialize(
+        InputDefinition $commandDefinition,
+        InputInterface $input,
+        array $excludedOptionNames
+    ): array {
+        $filteredOptions = array_diff_key(
+            $input->getOptions(),
+            array_fill_keys($excludedOptionNames, ''),
+        );
+
+        return array_map(
+            static fn (string $name) => self::serializeOption(
+                $commandDefinition->getOption($name),
+                $name,
+                $filteredOptions[$name],
+            ),
+            array_keys($filteredOptions),
+        );
+    }
+
+    /**
+     * @param string|bool|int|float|null|array<string|bool|int|float|null> $value
+     */
+    private static function serializeOption(
+        InputOption $option,
+        string $name,
+        $value
+    ): string {
+        if ($option->isNegatable()) {
+            return sprintf(
+                '--%s%s',
+                $value ? '' : 'no-',
+                $name,
+            );
+        }
+
+        if (!$option->acceptValue()) {
+            return sprintf(
+                '--%s',
+                $name,
+            );
+        }
+
+        if ($option->isArray()) {
+            return implode(
+                '',
+                array_map(
+                    static fn ($item) => self::serializeOptionWithValue($name, $item),
+                    $value,
+                ),
+            );
+        }
+
+        return self::serializeOptionWithValue($name, $value);
+    }
+
+    /**
+     * @param string|bool|int|float|null|array<string|bool|int|float|null> $value
+     */
+    private static function serializeOptionWithValue(
+        string $name,
+        $value
+    ): string {
+        return sprintf(
+            '--%s=%s',
+            $name,
+            self::quoteOptionValue($value),
+        );
+    }
+
+    /**
+     * Ensure that an option value is quoted correctly before it is passed to a
+     * child process.
+     *
+     * @param string|bool|int|float|null $value
+     *
+     * @return string|bool|int|float|null
+     */
+    private static function quoteOptionValue($value)
+    {
+        if (self::isValueRequiresQuoting($value)) {
+            return sprintf(
+                '"%s"',
+                str_replace('"', '\"', $value),
+            );
+        }
+
+        return $value;
+    }
+
+    /**
+     * Validate whether a command option requires quoting.
+     */
+    private static function isValueRequiresQuoting($value): bool
+    {
+        return is_string($value) && 0 < preg_match('/[\s\W]/x', $value);
+    }
+}

src/Parallelization.php

@@ -13,8 +13,6 @@
 
 namespace Webmozarts\Console\Parallelization;
 
-use function array_diff_key;
-use function array_fill_keys;
 use function array_filter;
 use function array_map;
 use function array_merge;
@@ -336,7 +334,14 @@ protected function executeMasterProcess(
                     ),
                     '--child',
                 ]),
-                $this->serializeInputOptions($input, ['child', 'processes']),
+                // Forward all the options except for "processes" to the children
+                // this way the children can inherit the options such as env
+                // or no-debug.
+                InputOptionsSerializer::serialize(
+                    $this->getDefinition(),
+                    $input,
+                    ['child', 'processes'],
+                ),
             );
 
             $terminalWidth = (new Terminal())->getWidth();
@@ -570,38 +575,4 @@ private function runTolerantSingleCommand(
             }
         }
     }
-
-    /**
-     * @param string[] $blackListParams
-     *
-     * @return string[]
-     */
-    private function serializeInputOptions(InputInterface $input, array $blackListParams): array
-    {
-        $options = array_diff_key(
-            array_filter($input->getOptions()),
-            array_fill_keys($blackListParams, ''),
-        );
-
-        $preparedOptionList = [];
-        foreach ($options as $name => $value) {
-            $definition = $this->getDefinition();
-            $option = $definition->getOption($name);
-
-            $optionString = '';
-            if (!$option->acceptValue()) {
-                $optionString .= '--'.$name;
-            } elseif ($option->isArray()) {
-                foreach ($value as $arrayValue) {
-                    $optionString .= '--'.$name.'='.$this->quoteOptionValue($arrayValue);
-                }
-            } else {
-                $optionString .= '--'.$name.'='.$this->quoteOptionValue($value);
-            }
-
-            $preparedOptionList[] = $optionString;
-        }
-
-        return $preparedOptionList;
-    }
 }