#143 - Type the remaining untyped task options

CHANGELOG.md

@@ -15,6 +15,7 @@ Latest
 * [#243](https://github.com/cleverage/process-bundle/issues/243) Fix RecursivePropertySetterTransformer: a `\stdClass` item without the property was replaced by a copy in the output, so the input object was not modified; the property is now added to the item itself. Update documentation, add tests.
 * [#242](https://github.com/cleverage/process-bundle/issues/242) Fix InputFileReaderTask: an input that is not a non-empty string (e.g. `null`) throws an explicit `\UnexpectedValueException` (`No file path given as input`) instead of a PHP warning followed by a `TypeError`. Update documentation, add tests.
 * [#244](https://github.com/cleverage/process-bundle/issues/244) Fix MappingTransformer: a missing target property of a `\stdClass` destination (`initial_value` or `keep_input`) threw `Property '...' is not writable`, it is now added when the target is a simple property name (nested paths still throw). Update documentation, add tests.
+* [#143](https://github.com/cleverage/process-bundle/issues/143) Type the remaining untyped task options: `split_character` of CsvWriterTask and SplitJoinLineTask must be a `string` (a wrong type used to fail later with a `TypeError`), `write_headers` of CsvWriterTask and `log_empty_lines` of CsvReaderTask are cast to `bool` (any value used to be evaluated as a boolean, so it is still accepted). Update documentation, add tests.
 
 v5.1
 -----

docs/reference/tasks/csv_reader_task.md

@@ -35,7 +35,7 @@ Options
 | `escape`          | `string`      |          | `\`     | CSV escape character                                                                                                              |
 | `headers`         | `array\|null` |          | `null`  | Static list of CSV headers. If `null`, headers are read from the first line of the file; otherwise the first line is read as data |
 | `mode`            | `string`      |          | `rb`    | File open mode (see [fopen mode parameter](https://www.php.net/manual/en/function.fopen.php))                                     |
-| `log_empty_lines` | `bool`        |          | `false` | Log a warning when a line cannot be read (empty line)                                                                             |
+| `log_empty_lines` | `bool`        |          | `false` | Log a warning when a line cannot be read (empty line); cast to `bool`                                                             |
 
 Examples
 --------

docs/reference/tasks/csv_writer_task.md

@@ -34,7 +34,7 @@ Options
 | `headers`         | `array\|null` |          | `null`  | Static list of CSV headers. If `null`, the keys of the first input are used                                                                                                                 |
 | `mode`            | `string`      |          | `wb`    | File open mode (see [fopen mode parameter](https://www.php.net/manual/en/function.fopen.php))                                                                                               |
 | `split_character` | `string`      |          | `\|`    | Used to implode array values                                                                                                                                                                |
-| `write_headers`   | `bool`        |          | `true`  | Write the headers as first line, only if the file is empty (useful with an append `mode`)                                                                                                   |
+| `write_headers`   | `bool`        |          | `true`  | Write the headers as first line, only if the file is empty (useful with an append `mode`); cast to `bool`                                                                                   |
 
 Examples
 --------

src/Task/File/Csv/CsvReaderTask.php

@@ -17,6 +17,7 @@
 use CleverAge\ProcessBundle\Model\IterableTaskInterface;
 use CleverAge\ProcessBundle\Model\ProcessState;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\OptionsResolver\Options;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
 /**
@@ -105,5 +106,7 @@ protected function configureOptions(OptionsResolver $resolver): void
         $resolver->setDefaults([
             'log_empty_lines' => false,
         ]);
+        // Any value used to be evaluated as a boolean: cast it instead of rejecting it
+        $resolver->setNormalizer('log_empty_lines', static fn (Options $options, mixed $value): bool => (bool) $value);
     }
 }

src/Task/File/Csv/CsvWriterTask.php

@@ -52,6 +52,9 @@ protected function configureOptions(OptionsResolver $resolver): void
             'split_character' => '|',
             'write_headers' => true,
         ]);
+        $resolver->setAllowedTypes('split_character', ['string']);
+        // Any value used to be evaluated as a boolean: cast it instead of rejecting it
+        $resolver->setNormalizer('write_headers', static fn (Options $options, mixed $value): bool => (bool) $value);
 
         $resolver->setNormalizer(
             'file_path',

src/Task/SplitJoinLineTask.php

@@ -40,6 +40,7 @@ protected function configureOptions(OptionsResolver $resolver): void
         $resolver->setDefaults([
             'split_character' => ',',
         ]);
+        $resolver->setAllowedTypes('split_character', ['string']);
     }
 
     protected function initializeIterator(ProcessState $state): \Iterator

tests/OptionAllowedTypesTest.php

@@ -19,17 +19,21 @@
 use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
 use CleverAge\ProcessBundle\Model\ProcessHistory;
 use CleverAge\ProcessBundle\Model\ProcessState;
+use CleverAge\ProcessBundle\Task\File\Csv\CsvReaderTask;
+use CleverAge\ProcessBundle\Task\File\Csv\CsvWriterTask;
 use CleverAge\ProcessBundle\Task\ObjectUpdaterTask;
 use CleverAge\ProcessBundle\Task\Serialization\DeserializerTask;
 use CleverAge\ProcessBundle\Task\Serialization\NormalizerTask;
 use CleverAge\ProcessBundle\Task\Serialization\SerializerTask;
 use CleverAge\ProcessBundle\Task\SimpleBatchTask;
+use CleverAge\ProcessBundle\Task\SplitJoinLineTask;
 use CleverAge\ProcessBundle\Transformer\Array\ArrayFilterTransformer;
 use CleverAge\ProcessBundle\Transformer\ConditionTrait;
 use CleverAge\ProcessBundle\Transformer\ConfigurableTransformerInterface;
 use CleverAge\ProcessBundle\Transformer\String\HashTransformer;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
 use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 use Symfony\Component\PropertyAccess\PropertyAccess;
@@ -47,6 +51,9 @@
 #[\PHPUnit\Framework\Attributes\CoversClass(SerializerTask::class)]
 #[\PHPUnit\Framework\Attributes\CoversClass(DeserializerTask::class)]
 #[\PHPUnit\Framework\Attributes\CoversClass(ObjectUpdaterTask::class)]
+#[\PHPUnit\Framework\Attributes\CoversClass(CsvReaderTask::class)]
+#[\PHPUnit\Framework\Attributes\CoversClass(CsvWriterTask::class)]
+#[\PHPUnit\Framework\Attributes\CoversClass(SplitJoinLineTask::class)]
 #[\PHPUnit\Framework\Attributes\UsesClass(AbstractConfigurableTask::class)]
 #[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)]
 #[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]
@@ -107,6 +114,8 @@ public static function invalidTaskOptionsProvider(): iterable
         yield 'serializer context' => [SerializerTask::class, ['format' => 'json', 'context' => 'groups']];
         yield 'deserializer context' => [DeserializerTask::class, ['type' => 'array', 'format' => 'json', 'context' => 'groups']];
         yield 'object updater property_path' => [ObjectUpdaterTask::class, ['property_path' => ['name']]];
+        yield 'csv writer split_character' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'split_character' => 1]];
+        yield 'split join line split_character' => [SplitJoinLineTask::class, ['split_columns' => [], 'join_column' => 'value', 'split_character' => [',']]];
     }
 
     /**
@@ -133,6 +142,35 @@ public static function validTaskOptionsProvider(): iterable
         yield 'deserializer context' => [DeserializerTask::class, ['type' => 'array', 'format' => 'json', 'context' => []]];
         yield 'object updater string property_path' => [ObjectUpdaterTask::class, ['property_path' => 'name']];
         yield 'object updater PropertyPath property_path' => [ObjectUpdaterTask::class, ['property_path' => new PropertyPath('name')]];
+        yield 'csv writer split_character' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'split_character' => ';']];
+        yield 'split join line split_character' => [SplitJoinLineTask::class, ['split_columns' => [], 'join_column' => 'value', 'split_character' => ';']];
+    }
+
+    /**
+     * @return iterable<string, array{class-string<AbstractConfigurableTask>, array<string, mixed>, string, bool}>
+     */
+    public static function booleanTaskOptionsProvider(): iterable
+    {
+        yield 'csv reader log_empty_lines true' => [CsvReaderTask::class, ['file_path' => 'file.csv', 'log_empty_lines' => true], 'log_empty_lines', true];
+        yield 'csv reader log_empty_lines 1' => [CsvReaderTask::class, ['file_path' => 'file.csv', 'log_empty_lines' => 1], 'log_empty_lines', true];
+        yield 'csv reader log_empty_lines empty string' => [CsvReaderTask::class, ['file_path' => 'file.csv', 'log_empty_lines' => ''], 'log_empty_lines', false];
+        yield 'csv writer write_headers false' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'write_headers' => false], 'write_headers', false];
+        yield 'csv writer write_headers 0' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'write_headers' => 0], 'write_headers', false];
+        yield 'csv writer write_headers yes' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'write_headers' => 'yes'], 'write_headers', true];
+    }
+
+    /**
+     * Boolean options used to accept any value evaluated as a boolean: it is cast instead of being rejected.
+     *
+     * @param class-string<AbstractConfigurableTask> $class
+     * @param array<string, mixed>                   $options
+     */
+    #[DataProvider('booleanTaskOptionsProvider')]
+    public function testBooleanTaskOptionIsCast(string $class, array $options, string $option, bool $expected): void
+    {
+        [$task, $state] = $this->initializeTask($class, $options);
+
+        self::assertSame($expected, (new \ReflectionMethod($task, 'getOption'))->invoke($task, $state, $option));
     }
 
     /**
@@ -167,12 +205,15 @@ private function resolveTransformerOptions(string $class, array $options): array
     /**
      * @param class-string<AbstractConfigurableTask> $class
      * @param array<string, mixed>                   $options
+     *
+     * @return array{AbstractConfigurableTask, ProcessState}
      */
-    private function initializeTask(string $class, array $options): void
+    private function initializeTask(string $class, array $options): array
     {
         $task = match ($class) {
             NormalizerTask::class, SerializerTask::class, DeserializerTask::class => new $class(new Serializer()),
             ObjectUpdaterTask::class => new ObjectUpdaterTask(PropertyAccess::createPropertyAccessor()),
+            CsvReaderTask::class => new CsvReaderTask(new NullLogger()),
             default => new $class(),
         };
 
@@ -182,5 +223,7 @@ private function initializeTask(string $class, array $options): void
         $state->setContext([]);
         $state->setTaskConfiguration(new TaskConfiguration('task', $class, $options));
         $task->initialize($state);
+
+        return [$task, $state];
     }
 }