Skip to content

Commit 30d9eed

Browse files
fix(task) #143 Type the remaining untyped task options: split_character of CsvWriterTask and SplitJoinLineTask must be a string, write_headers of CsvWriterTask and log_empty_lines of CsvReaderTask are cast to bool
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 6ce6bc4 commit 30d9eed

7 files changed

Lines changed: 54 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Latest
1515
* [#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.
1616
* [#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.
1717
* [#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.
18+
* [#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.
1819

1920
v5.1
2021
-----

‎docs/reference/tasks/csv_reader_task.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Options
3535
| `escape` | `string` | | `\` | CSV escape character |
3636
| `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 |
3737
| `mode` | `string` | | `rb` | File open mode (see [fopen mode parameter](https://www.php.net/manual/en/function.fopen.php)) |
38-
| `log_empty_lines` | `bool` | | `false` | Log a warning when a line cannot be read (empty line) |
38+
| `log_empty_lines` | `bool` | | `false` | Log a warning when a line cannot be read (empty line); cast to `bool` |
3939

4040
Examples
4141
--------

‎docs/reference/tasks/csv_writer_task.md‎

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

3939
Examples
4040
--------

‎src/Task/File/Csv/CsvReaderTask.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use CleverAge\ProcessBundle\Model\IterableTaskInterface;
1818
use CleverAge\ProcessBundle\Model\ProcessState;
1919
use Psr\Log\LoggerInterface;
20+
use Symfony\Component\OptionsResolver\Options;
2021
use Symfony\Component\OptionsResolver\OptionsResolver;
2122

2223
/**
@@ -105,5 +106,7 @@ protected function configureOptions(OptionsResolver $resolver): void
105106
$resolver->setDefaults([
106107
'log_empty_lines' => false,
107108
]);
109+
// Any value used to be evaluated as a boolean: cast it instead of rejecting it
110+
$resolver->setNormalizer('log_empty_lines', static fn (Options $options, mixed $value): bool => (bool) $value);
108111
}
109112
}

‎src/Task/File/Csv/CsvWriterTask.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ protected function configureOptions(OptionsResolver $resolver): void
5252
'split_character' => '|',
5353
'write_headers' => true,
5454
]);
55+
$resolver->setAllowedTypes('split_character', ['string']);
56+
// Any value used to be evaluated as a boolean: cast it instead of rejecting it
57+
$resolver->setNormalizer('write_headers', static fn (Options $options, mixed $value): bool => (bool) $value);
5558

5659
$resolver->setNormalizer(
5760
'file_path',

‎src/Task/SplitJoinLineTask.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ protected function configureOptions(OptionsResolver $resolver): void
4040
$resolver->setDefaults([
4141
'split_character' => ',',
4242
]);
43+
$resolver->setAllowedTypes('split_character', ['string']);
4344
}
4445

4546
protected function initializeIterator(ProcessState $state): \Iterator

‎tests/OptionAllowedTypesTest.php‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,21 @@
1919
use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
2020
use CleverAge\ProcessBundle\Model\ProcessHistory;
2121
use CleverAge\ProcessBundle\Model\ProcessState;
22+
use CleverAge\ProcessBundle\Task\File\Csv\CsvReaderTask;
23+
use CleverAge\ProcessBundle\Task\File\Csv\CsvWriterTask;
2224
use CleverAge\ProcessBundle\Task\ObjectUpdaterTask;
2325
use CleverAge\ProcessBundle\Task\Serialization\DeserializerTask;
2426
use CleverAge\ProcessBundle\Task\Serialization\NormalizerTask;
2527
use CleverAge\ProcessBundle\Task\Serialization\SerializerTask;
2628
use CleverAge\ProcessBundle\Task\SimpleBatchTask;
29+
use CleverAge\ProcessBundle\Task\SplitJoinLineTask;
2730
use CleverAge\ProcessBundle\Transformer\Array\ArrayFilterTransformer;
2831
use CleverAge\ProcessBundle\Transformer\ConditionTrait;
2932
use CleverAge\ProcessBundle\Transformer\ConfigurableTransformerInterface;
3033
use CleverAge\ProcessBundle\Transformer\String\HashTransformer;
3134
use PHPUnit\Framework\Attributes\DataProvider;
3235
use PHPUnit\Framework\TestCase;
36+
use Psr\Log\NullLogger;
3337
use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
3438
use Symfony\Component\OptionsResolver\OptionsResolver;
3539
use Symfony\Component\PropertyAccess\PropertyAccess;
@@ -47,6 +51,9 @@
4751
#[\PHPUnit\Framework\Attributes\CoversClass(SerializerTask::class)]
4852
#[\PHPUnit\Framework\Attributes\CoversClass(DeserializerTask::class)]
4953
#[\PHPUnit\Framework\Attributes\CoversClass(ObjectUpdaterTask::class)]
54+
#[\PHPUnit\Framework\Attributes\CoversClass(CsvReaderTask::class)]
55+
#[\PHPUnit\Framework\Attributes\CoversClass(CsvWriterTask::class)]
56+
#[\PHPUnit\Framework\Attributes\CoversClass(SplitJoinLineTask::class)]
5057
#[\PHPUnit\Framework\Attributes\UsesClass(AbstractConfigurableTask::class)]
5158
#[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)]
5259
#[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]
@@ -107,6 +114,8 @@ public static function invalidTaskOptionsProvider(): iterable
107114
yield 'serializer context' => [SerializerTask::class, ['format' => 'json', 'context' => 'groups']];
108115
yield 'deserializer context' => [DeserializerTask::class, ['type' => 'array', 'format' => 'json', 'context' => 'groups']];
109116
yield 'object updater property_path' => [ObjectUpdaterTask::class, ['property_path' => ['name']]];
117+
yield 'csv writer split_character' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'split_character' => 1]];
118+
yield 'split join line split_character' => [SplitJoinLineTask::class, ['split_columns' => [], 'join_column' => 'value', 'split_character' => [',']]];
110119
}
111120

112121
/**
@@ -133,6 +142,35 @@ public static function validTaskOptionsProvider(): iterable
133142
yield 'deserializer context' => [DeserializerTask::class, ['type' => 'array', 'format' => 'json', 'context' => []]];
134143
yield 'object updater string property_path' => [ObjectUpdaterTask::class, ['property_path' => 'name']];
135144
yield 'object updater PropertyPath property_path' => [ObjectUpdaterTask::class, ['property_path' => new PropertyPath('name')]];
145+
yield 'csv writer split_character' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'split_character' => ';']];
146+
yield 'split join line split_character' => [SplitJoinLineTask::class, ['split_columns' => [], 'join_column' => 'value', 'split_character' => ';']];
147+
}
148+
149+
/**
150+
* @return iterable<string, array{class-string<AbstractConfigurableTask>, array<string, mixed>, string, bool}>
151+
*/
152+
public static function booleanTaskOptionsProvider(): iterable
153+
{
154+
yield 'csv reader log_empty_lines true' => [CsvReaderTask::class, ['file_path' => 'file.csv', 'log_empty_lines' => true], 'log_empty_lines', true];
155+
yield 'csv reader log_empty_lines 1' => [CsvReaderTask::class, ['file_path' => 'file.csv', 'log_empty_lines' => 1], 'log_empty_lines', true];
156+
yield 'csv reader log_empty_lines empty string' => [CsvReaderTask::class, ['file_path' => 'file.csv', 'log_empty_lines' => ''], 'log_empty_lines', false];
157+
yield 'csv writer write_headers false' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'write_headers' => false], 'write_headers', false];
158+
yield 'csv writer write_headers 0' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'write_headers' => 0], 'write_headers', false];
159+
yield 'csv writer write_headers yes' => [CsvWriterTask::class, ['file_path' => 'file.csv', 'write_headers' => 'yes'], 'write_headers', true];
160+
}
161+
162+
/**
163+
* Boolean options used to accept any value evaluated as a boolean: it is cast instead of being rejected.
164+
*
165+
* @param class-string<AbstractConfigurableTask> $class
166+
* @param array<string, mixed> $options
167+
*/
168+
#[DataProvider('booleanTaskOptionsProvider')]
169+
public function testBooleanTaskOptionIsCast(string $class, array $options, string $option, bool $expected): void
170+
{
171+
[$task, $state] = $this->initializeTask($class, $options);
172+
173+
self::assertSame($expected, (new \ReflectionMethod($task, 'getOption'))->invoke($task, $state, $option));
136174
}
137175

138176
/**
@@ -167,12 +205,15 @@ private function resolveTransformerOptions(string $class, array $options): array
167205
/**
168206
* @param class-string<AbstractConfigurableTask> $class
169207
* @param array<string, mixed> $options
208+
*
209+
* @return array{AbstractConfigurableTask, ProcessState}
170210
*/
171-
private function initializeTask(string $class, array $options): void
211+
private function initializeTask(string $class, array $options): array
172212
{
173213
$task = match ($class) {
174214
NormalizerTask::class, SerializerTask::class, DeserializerTask::class => new $class(new Serializer()),
175215
ObjectUpdaterTask::class => new ObjectUpdaterTask(PropertyAccess::createPropertyAccessor()),
216+
CsvReaderTask::class => new CsvReaderTask(new NullLogger()),
176217
default => new $class(),
177218
};
178219

@@ -182,5 +223,7 @@ private function initializeTask(string $class, array $options): void
182223
$state->setContext([]);
183224
$state->setTaskConfiguration(new TaskConfiguration('task', $class, $options));
184225
$task->initialize($state);
226+
227+
return [$task, $state];
185228
}
186229
}

0 commit comments

Comments
 (0)