From e88f0a7cd7de95d6d924ffa8bdb7424de0dccced Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Fri, 23 Oct 2026 11:21:18 +0200 Subject: [PATCH 1/2] #365 - array_element / array_last return null outside the array without warning; documentation gaps (advanced_stat_counter rate, CSV headers: [], ConditionTrait accessor, rules ParsedExpression) Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + .../tasks/advanced_stat_counter_task.md | 2 ++ docs/reference/tasks/csv_reader_task.md | 2 +- docs/reference/tasks/input_csv_reader_task.md | 2 +- docs/reference/traits/condition_trait.md | 3 ++- .../transformers/array_element_transformer.md | 11 ++++++---- .../transformers/array_last_transformer.md | 7 +------ .../transformers/rules_transformer.md | 2 +- .../Array/ArrayElementTransformer.php | 9 +++++++- .../Array/ArrayLastTransformer.php | 3 ++- .../Array/ArrayElementTransformerTest.php | 21 +++++++++++++++++++ .../Array/ArrayLastTransformerTest.php | 6 ++++++ 12 files changed, 53 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62454c6e..9ab638b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ## Changes +* [#365](https://github.com/cleverage/process-bundle/issues/365) `ArrayElementTransformer` (`array_element`) and `ArrayLastTransformer` (`array_last`) return `null` when the position is outside the array (or the array is empty), without the "Undefined array key" PHP warning (it was an exception in debug mode, `null` in production); a negative `index` before the start of the array no longer returns its first element. Documentation: rate `n/a` of `advanced_stat_counter`, `headers: []` of `csv_reader` / `input_csv_reader`, optional `$accessor` of `ConditionTrait`, `ParsedExpression` accepted by the `condition` of `rules`. Add tests. * [#337](https://github.com/cleverage/process-bundle/issues/337) `StopTask` (`stop`): a process stopped on purpose is identified as stopped, not as failed. Its history is flagged as stopped (new `ProcessHistory::STATE_STOPPED`, `setStopped()`, `isStopped()`) instead of failed, the `Process stopped` log is written at the `success_level` (instead of `Process failed` at the `failed_level`), and the `cleverage_process.end` event tells it with the new `ProcessEvent::isProcessStopped()`. The process still ends normally (tasks finalized, exit code 0, parent process going on). Update documentation, add tests. * [#339](https://github.com/cleverage/process-bundle/issues/339) A shared service referenced by several tasks of the same process is rejected when the container is compiled (`InvalidArgumentException` naming the service, the tasks and the process): all these tasks ran with the same instance, initialized with the options of each task in turn, without error. Declare the task services with `shared: false` (applied by the autoconfiguration of `TaskInterface` since 5.4). Update documentation, add tests. * [#351](https://github.com/cleverage/process-bundle/issues/351) `ProcessManager` and `ProcessHelpCommand` are typed on `TaskRegistryInterface` instead of the final `TaskRegistry` class: the task registry can be decorated or replaced. Add tests. diff --git a/docs/reference/tasks/advanced_stat_counter_task.md b/docs/reference/tasks/advanced_stat_counter_task.md index 8425722d..076c7bea 100644 --- a/docs/reference/tasks/advanced_stat_counter_task.md +++ b/docs/reference/tasks/advanced_stat_counter_task.md @@ -27,6 +27,8 @@ The logged message has the following format: Last iteration 00:00:12 ago - 1,50 items/s - 500 items processed in 00:05:33 ``` +The rate is `n/a items/s` when less than one second has elapsed since the first counted execution. + Options ------- diff --git a/docs/reference/tasks/csv_reader_task.md b/docs/reference/tasks/csv_reader_task.md index dc25c615..699628f2 100644 --- a/docs/reference/tasks/csv_reader_task.md +++ b/docs/reference/tasks/csv_reader_task.md @@ -34,7 +34,7 @@ Options | `delimiter` | `string` | | `;` | CSV delimiter | | `enclosure` | `string` | | `"` | CSV enclosure character | | `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 | +| `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. An empty list throws an `UnexpectedValueException` | | `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 an empty read ends the file (e.g. the line break at the end of the last line). A blank line in the middle of the file is not skipped: it fails with "Number of columns not matching" (unless the file has a single column); cast to `bool` | diff --git a/docs/reference/tasks/input_csv_reader_task.md b/docs/reference/tasks/input_csv_reader_task.md index ba5c3750..b35a08fd 100644 --- a/docs/reference/tasks/input_csv_reader_task.md +++ b/docs/reference/tasks/input_csv_reader_task.md @@ -36,7 +36,7 @@ Options | `delimiter` | `string` | | `;` | CSV delimiter | | `enclosure` | `string` | | `"` | CSV enclosure character | | `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 | +| `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. An empty list throws an `UnexpectedValueException` | | `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 an empty read ends the file (e.g. the line break at the end of the last line). A blank line in the middle of the file is not skipped: it fails with "Number of columns not matching" (unless the file has a single column) | diff --git a/docs/reference/traits/condition_trait.md b/docs/reference/traits/condition_trait.md index 90cfe915..c680b2d9 100644 --- a/docs/reference/traits/condition_trait.md +++ b/docs/reference/traits/condition_trait.md @@ -35,7 +35,8 @@ of `preg_match()` is converted into an exception by the Symfony error handler: t * Call `ConditionTrait::configureConditionOptions` to add the condition options at the root of your `OptionsResolver`, or `ConditionTrait::configureWrappedConditionOptions` to add them under a single option (e.g. `condition`) -* Set the `$accessor` property with a `PropertyAccessorInterface` (e.g. in the constructor) +* Optionally set the `$accessor` property with a `PropertyAccessorInterface` (e.g. in the constructor): otherwise a + default property accessor is created on first use * Call `ConditionTrait::checkCondition` with the input and the resolved conditions; it returns `true` if all conditions match diff --git a/docs/reference/transformers/array_element_transformer.md b/docs/reference/transformers/array_element_transformer.md index f9950b1f..4c4b69fc 100644 --- a/docs/reference/transformers/array_element_transformer.md +++ b/docs/reference/transformers/array_element_transformer.md @@ -18,7 +18,7 @@ Accepted inputs Possible outputs ---------------- -`any`: the element found at the given position +`any`: the element found at the given position, `null` when the position is outside the array (at either end) Options ------- @@ -46,7 +46,10 @@ array_element: index: -2 ``` -Notes ------ +* An index outside the array: `['foo']` becomes `null` -There is no bound check: an index outside the array triggers an "Undefined array key" warning. +```yaml +# Transformer options level +array_element: + index: 3 +``` diff --git a/docs/reference/transformers/array_last_transformer.md b/docs/reference/transformers/array_last_transformer.md index af7cc71d..90f5f563 100644 --- a/docs/reference/transformers/array_last_transformer.md +++ b/docs/reference/transformers/array_last_transformer.md @@ -17,7 +17,7 @@ Accepted inputs Possible outputs ---------------- -`any`: the last element of the array +`any`: the last element of the array, `null` for an empty array Options ------- @@ -33,8 +33,3 @@ Examples # Transformer options level array_last: ~ ``` - -Notes ------ - -An empty array triggers an "Undefined array key" warning. diff --git a/docs/reference/transformers/rules_transformer.md b/docs/reference/transformers/rules_transformer.md index 45ceb5bd..04589890 100644 --- a/docs/reference/transformers/rules_transformer.md +++ b/docs/reference/transformers/rules_transformer.md @@ -39,7 +39,7 @@ Each rule of `rules_set` has the following options: | Code | Type | Required | Default | Description | |----------------|----------------|:--------:|---------|---------------------------------------------------------------------------------------------------------------------------------------| -| `condition` | `string\|null` | | `null` | Expression; the rule matches if it is truthy | +| `condition` | `string\|ParsedExpression\|null` | | `null` | Expression (or an already parsed `Symfony\Component\ExpressionLanguage\ParsedExpression`); the rule matches if it is truthy | | `default` | `bool` | | `false` | Mark the rule as the default one (always matches). It cannot have a `condition`, no conditional rule may follow it, only one allowed | | `set_null` | `bool` | | `false` | If `true`, return `null` (takes precedence on `constant` and `transformers`) | | `constant` | `any` | | `null` | If not `null`, return this value (takes precedence on `transformers`) | diff --git a/src/Transformer/Array/ArrayElementTransformer.php b/src/Transformer/Array/ArrayElementTransformer.php index b0ea76b3..8838ef4e 100644 --- a/src/Transformer/Array/ArrayElementTransformer.php +++ b/src/Transformer/Array/ArrayElementTransformer.php @@ -28,7 +28,14 @@ class ArrayElementTransformer implements ConfigurableTransformerInterface */ public function transform(mixed $value, array $options = []): mixed { - return array_values(\array_slice($value, $options['index'], 1))[0]; + $index = $options['index']; + // null when the index is outside the array, at either end: no "Undefined array key" warning after the end, and + // not the first element (as array_slice() would give) before the start + if ($index >= \count($value) || $index < -\count($value)) { + return null; + } + + return array_values(\array_slice($value, $index, 1))[0]; } public function getCode(): string diff --git a/src/Transformer/Array/ArrayLastTransformer.php b/src/Transformer/Array/ArrayLastTransformer.php index a67ce257..b2855808 100644 --- a/src/Transformer/Array/ArrayLastTransformer.php +++ b/src/Transformer/Array/ArrayLastTransformer.php @@ -27,7 +27,8 @@ class ArrayLastTransformer implements TransformerInterface */ public function transform(mixed $value, array $options = []): mixed { - return array_values(\array_slice($value, -1))[0]; + // null for an empty array (no "Undefined array key" warning) + return array_values(\array_slice($value, -1))[0] ?? null; } public function getCode(): string diff --git a/tests/Transformer/Array/ArrayElementTransformerTest.php b/tests/Transformer/Array/ArrayElementTransformerTest.php index 0d0cd18c..b63a9735 100644 --- a/tests/Transformer/Array/ArrayElementTransformerTest.php +++ b/tests/Transformer/Array/ArrayElementTransformerTest.php @@ -14,6 +14,7 @@ namespace CleverAge\ProcessBundle\Tests\Transformer\Array; use CleverAge\ProcessBundle\Transformer\Array\ArrayElementTransformer; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -34,6 +35,26 @@ public function testTransformReturnsNthElementFromArray(): void $this->assertEquals('bar', $result); } + /** + * @return iterable, int}> + */ + public static function outsideIndexProvider(): iterable + { + yield 'index after the end' => [['foo', 'bar'], 2]; + yield 'negative index before the start' => [['foo', 'bar'], -3]; + yield 'empty array' => [[], 0]; + } + + /** + * @param array $value + */ + #[DataProvider('outsideIndexProvider')] + public function testIndexOutsideTheArrayReturnsNullWithoutWarning(array $value, int $index): void + { + // A PHP warning would fail the test (failOnWarning) + $this->assertNull((new ArrayElementTransformer())->transform($value, ['index' => $index])); + } + public function testConfigureOptionsSetsRequiredOptions(): void { $resolver = new OptionsResolver(); diff --git a/tests/Transformer/Array/ArrayLastTransformerTest.php b/tests/Transformer/Array/ArrayLastTransformerTest.php index 68207e00..7acb4fe0 100644 --- a/tests/Transformer/Array/ArrayLastTransformerTest.php +++ b/tests/Transformer/Array/ArrayLastTransformerTest.php @@ -47,6 +47,12 @@ public function testTransform(array $value, mixed $expected): void self::assertSame($expected, (new ArrayLastTransformer())->transform($value)); } + public function testEmptyArrayReturnsNullWithoutWarning(): void + { + // A PHP warning would fail the test (failOnWarning) + self::assertNull((new ArrayLastTransformer())->transform([])); + } + public function testNonArrayInputThrows(): void { $this->expectException(\TypeError::class); From e1fd3b135845fa225cbef60397266a6b39a9f11e Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Fri, 23 Oct 2026 11:24:31 +0200 Subject: [PATCH 2/2] #365 - input_csv_reader: explicit exception for an input that is not a path (null), instead of a PHP warning and a TypeError Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 +- docs/reference/tasks/input_csv_reader_task.md | 3 ++- src/Task/File/Csv/InputCsvReaderTask.php | 13 +++++++++++++ tests/Task/File/Csv/CsvReaderTaskTest.php | 14 ++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ab638b3..bba828cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Latest ------ ## Changes -* [#365](https://github.com/cleverage/process-bundle/issues/365) `ArrayElementTransformer` (`array_element`) and `ArrayLastTransformer` (`array_last`) return `null` when the position is outside the array (or the array is empty), without the "Undefined array key" PHP warning (it was an exception in debug mode, `null` in production); a negative `index` before the start of the array no longer returns its first element. Documentation: rate `n/a` of `advanced_stat_counter`, `headers: []` of `csv_reader` / `input_csv_reader`, optional `$accessor` of `ConditionTrait`, `ParsedExpression` accepted by the `condition` of `rules`. Add tests. +* [#365](https://github.com/cleverage/process-bundle/issues/365) `ArrayElementTransformer` (`array_element`) and `ArrayLastTransformer` (`array_last`) return `null` when the position is outside the array (or the array is empty), without the "Undefined array key" PHP warning (it was an exception in debug mode, `null` in production); a negative `index` before the start of the array no longer returns its first element. `InputCsvReaderTask` (`input_csv_reader`): an input that is not a path (e.g. `null`) throws an explicit `UnexpectedValueException` instead of an "Undefined array key" PHP warning followed by a `TypeError`. Documentation: rate `n/a` of `advanced_stat_counter`, `headers: []` of `csv_reader` / `input_csv_reader`, optional `$accessor` of `ConditionTrait`, `ParsedExpression` accepted by the `condition` of `rules`. Add tests. * [#337](https://github.com/cleverage/process-bundle/issues/337) `StopTask` (`stop`): a process stopped on purpose is identified as stopped, not as failed. Its history is flagged as stopped (new `ProcessHistory::STATE_STOPPED`, `setStopped()`, `isStopped()`) instead of failed, the `Process stopped` log is written at the `success_level` (instead of `Process failed` at the `failed_level`), and the `cleverage_process.end` event tells it with the new `ProcessEvent::isProcessStopped()`. The process still ends normally (tasks finalized, exit code 0, parent process going on). Update documentation, add tests. * [#339](https://github.com/cleverage/process-bundle/issues/339) A shared service referenced by several tasks of the same process is rejected when the container is compiled (`InvalidArgumentException` naming the service, the tasks and the process): all these tasks ran with the same instance, initialized with the options of each task in turn, without error. Declare the task services with `shared: false` (applied by the autoconfiguration of `TaskInterface` since 5.4). Update documentation, add tests. * [#351](https://github.com/cleverage/process-bundle/issues/351) `ProcessManager` and `ProcessHelpCommand` are typed on `TaskRegistryInterface` instead of the final `TaskRegistry` class: the task registry can be decorated or replaced. Add tests. diff --git a/docs/reference/tasks/input_csv_reader_task.md b/docs/reference/tasks/input_csv_reader_task.md index b35a08fd..6902d6f1 100644 --- a/docs/reference/tasks/input_csv_reader_task.md +++ b/docs/reference/tasks/input_csv_reader_task.md @@ -14,7 +14,8 @@ Task reference Accepted inputs --------------- -`string`: path of the file to read, prefixed by the `base_path` option if set. +`string`: path of the file to read, prefixed by the `base_path` option if set. Any other input (e.g. `null`) throws an +`UnexpectedValueException`. The file is closed once fully read, so each input (the same path again or a different one) is read from its beginning. diff --git a/src/Task/File/Csv/InputCsvReaderTask.php b/src/Task/File/Csv/InputCsvReaderTask.php index 2781c000..dd2cdfaa 100644 --- a/src/Task/File/Csv/InputCsvReaderTask.php +++ b/src/Task/File/Csv/InputCsvReaderTask.php @@ -13,6 +13,7 @@ namespace CleverAge\ProcessBundle\Task\File\Csv; +use CleverAge\ProcessBundle\Exception\UnexpectedValueException; use CleverAge\ProcessBundle\Model\ProcessState; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -21,6 +22,18 @@ */ class InputCsvReaderTask extends CsvReaderTask { + #[\Override] + public function execute(ProcessState $state): void + { + // Without a path, the file_path option (removed) would be read: "Undefined array key" warning, then TypeError + $input = $state->getInput(); + if (!\is_string($input)) { + throw new UnexpectedValueException(\sprintf('The input of the input_csv_reader task must be the path of the file to read, "%s" given.', get_debug_type($input))); + } + + parent::execute($state); + } + /** * @return array */ diff --git a/tests/Task/File/Csv/CsvReaderTaskTest.php b/tests/Task/File/Csv/CsvReaderTaskTest.php index b1a612fa..bb38a8dd 100644 --- a/tests/Task/File/Csv/CsvReaderTaskTest.php +++ b/tests/Task/File/Csv/CsvReaderTaskTest.php @@ -16,6 +16,7 @@ use CleverAge\ProcessBundle\Configuration\ProcessConfiguration; use CleverAge\ProcessBundle\Configuration\TaskConfiguration; use CleverAge\ProcessBundle\Context\ContextualOptionResolver; +use CleverAge\ProcessBundle\Exception\UnexpectedValueException; use CleverAge\ProcessBundle\Filesystem\CsvFile; use CleverAge\ProcessBundle\Filesystem\CsvResource; use CleverAge\ProcessBundle\Model\AbstractConfigurableTask; @@ -31,6 +32,7 @@ #[\PHPUnit\Framework\Attributes\CoversClass(CsvReaderTask::class)] #[\PHPUnit\Framework\Attributes\CoversClass(InputCsvReaderTask::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(UnexpectedValueException::class)] #[\PHPUnit\Framework\Attributes\UsesClass(AbstractCsvTask::class)] #[\PHPUnit\Framework\Attributes\UsesClass(AbstractCsvResourceTask::class)] #[\PHPUnit\Framework\Attributes\UsesClass(CsvFile::class)] @@ -97,6 +99,18 @@ public function testInputCsvReaderReadsDifferentFilesSuccessively(): void $task->finalize($state); } + public function testInputCsvReaderWithoutPathThrows(): void + { + $task = new InputCsvReaderTask(new NullLogger()); + $state = $this->createState([]); + $state->setInput(null); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('The input of the input_csv_reader task must be the path of the file to read, "null" given.'); + + $task->execute($state); + } + /** * Mimics the ProcessManager loop over an iterable task and returns the non-skipped outputs. *