#365 - array_element / array_last without warning, documentation gaps

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. `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 <code> stopped` log is written at the `success_level` (instead of `Process <code> 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.

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
 -------
 

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` |
 

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.
 
@@ -36,7 +37,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) |
 

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
 

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
+```

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.

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`)                                                                 |

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<string, mixed>
      */

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

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

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.
      *

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<string, array{array<mixed>, 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<mixed> $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();

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);