#94 - TransformerTrait: allow a list syntax for the transformers option

CHANGELOG.md

@@ -5,6 +5,7 @@ Latest
 * [#197](https://github.com/cleverage/process-bundle/issues/197) Add missing tests: every Task and Transformer is now covered by unit tests. Remove the obsolete `tests.old` directory.
 * [#143](https://github.com/cleverage/process-bundle/issues/143) Improve PHPStan configuration: remove all `ignoreErrors` and `@phpstan-ignore` comments (report unmatched ignored errors again), add missing iterable value types and generic types in PHPDoc, remove unreachable code in AbstractIterableOutputTask and InputAggregatorTask.
 * [#143](https://github.com/cleverage/process-bundle/issues/143) Improve PHPStan level from 6 to 7, and fix the level 8 errors that do not require a signature change (the remaining ones, due to nullable return types such as `AbstractConfigurableTask::getOptions(): ?array`, will be fixed in v6.0). Default to a `ContextualOptionResolver` in ProcessState and to a property accessor in ConditionTrait when none is set.
+* [#94](https://github.com/cleverage/process-bundle/issues/94) TransformerTrait: the `transformers` option (TransformerTask, MappingTransformer, ArrayMapTransformer, CachedTransformer, RulesTransformer, generic transformers) also accepts a list, whose items are a transformer code without options (`- trim`) or a single `code: options` map (`- callback: {...}`), to chain the same transformer without `#` suffix. The map syntax is still supported. Update documentation, add tests.
 
 ## Fixes
 * [#143](https://github.com/cleverage/process-bundle/issues/143) Fix InputIteratorTask: an `\IteratorAggregate` input whose `getIterator()` does not return an `\Iterator` (e.g. another `\IteratorAggregate`) is iterated instead of failing with a `TypeError`. Update documentation, add tests.

docs/reference/03-generic_transformers_definition.md

@@ -36,8 +36,8 @@ Note that an option with a default value is still required by default, which has
 set `required: false` for an option without default value that may be omitted: when omitted, its placeholders are
 replaced by `null`.
 
-The `transformers` list uses the same syntax as any other transformer using a sub-list of transformers (see
-[TransformerTrait](traits/transformer_trait.md)).
+The `transformers` list uses the same syntax as any other transformer using a sub-list of transformers, a map or a list
+(see [TransformerTrait](traits/transformer_trait.md)).
 You can use the syntax for contextual values (`{{ contextual_option_code }}`) to put placeholders that will be filled by
 those contextual options, with the values given when the generic transformer is used. If the whole value is a
 placeholder, the raw option value is injected (it can be an array, an integer...).

docs/reference/tasks/transformer_task.md

@@ -25,9 +25,9 @@ added to the error context as `error`) and handled according to the task `error_
 Options
 -------
 
-| Code           | Type    | Required | Default | Description                                                                                          |
-|----------------|---------|:--------:|---------|------------------------------------------------------------------------------------------------------|
-| `transformers` | `array` |          | `[]`    | Ordered map of `transformer code => options`, see [TransformerTrait](../traits/transformer_trait.md) |
+| Code           | Type    | Required | Default | Description                                                                                                                   |
+|----------------|---------|:--------:|---------|-------------------------------------------------------------------------------------------------------------------------------|
+| `transformers` | `array` |          | `[]`    | Ordered map of `transformer code => options`, or list of transformers, see [TransformerTrait](../traits/transformer_trait.md) |
 
 Examples
 --------
@@ -67,5 +67,26 @@ transform:
   outputs: [load]
 ```
 
+* Same chain using the list syntax: the same transformer can be used several times without suffix
+
+```yaml
+# Task configuration level
+transform:
+  service: '@CleverAge\ProcessBundle\Task\TransformerTask'
+  options:
+    transformers:
+      - mapping:
+          mapping:
+            name:
+              code: '[firstname]'
+              transformers:
+                - trim
+      - callback:
+          callback: array_filter
+      - callback:
+          callback: array_reverse
+  outputs: [load]
+```
+
 See [MappingTransformer](../transformers/mapping_transformer.md) and
 [generic transformers](../03-generic_transformers_definition.md).

docs/reference/traits/transformer_trait.md

@@ -7,14 +7,17 @@ Allow to hold a list of sub-transformers, configure their options at initializat
 
 * Namespace: `CleverAge\ProcessBundle\Transformer\TransformerTrait`
 * Options algorithm:
-  - the option (`transformers` by default) is an ordered list of `transformer code => transformer options`
+  - the option (`transformers` by default) is either an ordered map of `transformer code => transformer options`, or a
+    list (see [List syntax](#list-syntax)) whose items are a transformer code without options (`- trim`) or a single
+    `transformer code: transformer options` map (`- callback: { callback: array_filter }`)
   - options must be an `array` or `null` (`~`); anything else throws an `InvalidArgumentException`
   - options are resolved once, when the parent options are resolved, with the `configureOptions` method of the
     matching transformer; a transformer that is not configurable must not receive options
   - an unknown transformer code throws a `MissingTransformerException`
   - at runtime, transformers are applied in the declared order, each one receiving the output of the previous one
   - any error thrown by a transformer is wrapped in a `TransformerException` ("Transformation '<code>' have failed:
-    <original message>"), the original exception being available as previous exception
+    <original message>"), the original exception being available as previous exception. With the list syntax, the
+    code is followed by `#` and the position of the transformer in the list, starting at 0 (`callback#1`)
   - as YAML keys must be unique, a suffix starting with `#` can be added to the code to use the same transformer
     several times. Any non-empty suffix is accepted: digits (`callback#1`) or a name describing the step
     (`callback#reverse`). The part before the first `#` is used as the transformer code if it is registered (otherwise
@@ -28,6 +31,21 @@ transformers:
     callback: array_reverse
 ```
 
+### List syntax
+
+The list syntax allows to use the same transformer several times without suffix. Each item is either a transformer
+code (for a transformer without options) or a map with a single `transformer code: transformer options` entry; any
+other item throws an `InvalidArgumentException`. Example:
+
+```yaml
+transformers:
+  - trim
+  - callback:
+      callback: array_filter
+  - callback:
+      callback: array_reverse
+```
+
 ## Usage
 
 * Set the `$transformerRegistry` property with the `TransformerRegistry` service (e.g. in the constructor)

src/Transformer/TransformerTrait.php

@@ -26,16 +26,30 @@ trait TransformerTrait
     /**
      * Transform the list of transformer codes + options into a list of Closure (better performances).
      *
-     * @param Options<array<string, mixed>>            $options
-     * @param array<string, array<string, mixed>|null> $transformers
+     * The list is either a map of "transformer code => options", or a list whose items are a transformer code (without
+     * options) or a single "transformer code => options" map. In a list, the closures are keyed by the transformer code
+     * followed by "#" and the item position, to keep the keys unique.
+     *
+     * @param Options<array<string, mixed>> $options
+     * @param array<int|string, mixed>      $transformers
      *
      * @return array<string, \Closure>
      */
     public function normalizeTransformers(Options $options, array $transformers): array
     {
         $transformerClosures = [];
+        $isList = array_is_list($transformers);
+
+        foreach ($transformers as $key => $transformerDefinition) {
+            if ($isList && \is_int($key)) {
+                [$origTransformerCode, $transformerOptions] = $this->parseListedTransformer($transformerDefinition, $key);
+                $closureKey = "{$origTransformerCode}#{$key}";
+            } else {
+                $origTransformerCode = (string) $key;
+                $transformerOptions = $transformerDefinition;
+                $closureKey = $origTransformerCode;
+            }
 
-        foreach ($transformers as $origTransformerCode => $transformerOptions) {
             $transformerOptionsResolver = new OptionsResolver();
             $transformerCode = $this->getCleanedTransfomerCode($origTransformerCode);
             $transformer = $this->getTransformerRegistry()->getTransformer($transformerCode);
@@ -48,7 +62,7 @@ public function normalizeTransformers(Options $options, array $transformers): ar
             }
 
             $closure = static fn ($value) => $transformer->transform($value, $transformerOptions);
-            $transformerClosures[$origTransformerCode] = $closure;
+            $transformerClosures[$closureKey] = $closure;
         }
 
         return $transformerClosures;
@@ -129,6 +143,29 @@ private function checkTransformerOptions(mixed $transformerOptions, string $tran
         throw new \InvalidArgumentException("Options for transformer {$transformerCode} are invalid : found {$type}, expected array or null");
     }
 
+    /**
+     * Read the code and the options of an item of a transformer list: either a transformer code (without options), or
+     * a single "transformer code => options" map.
+     *
+     * @return array{string, mixed}
+     */
+    private function parseListedTransformer(mixed $transformerDefinition, int $position): array
+    {
+        if (\is_string($transformerDefinition) && '' !== $transformerDefinition) {
+            return [$transformerDefinition, null];
+        }
+        if (\is_array($transformerDefinition) && 1 === \count($transformerDefinition)) {
+            $transformerCode = array_key_first($transformerDefinition);
+            if (\is_string($transformerCode)) {
+                return [$transformerCode, $transformerDefinition[$transformerCode]];
+            }
+        }
+
+        $type = get_debug_type($transformerDefinition);
+
+        throw new \InvalidArgumentException("Transformer at position {$position} is invalid : found {$type}, expected a transformer code or a single \"code: options\" map");
+    }
+
     private function getTransformerRegistry(): TransformerRegistry
     {
         if (!$this->transformerRegistry instanceof TransformerRegistry) {

tests/Task/TransformerTaskTest.php

@@ -110,6 +110,21 @@ public function testSameTransformerCanBeChainedWithSuffixes(): void
         self::assertSame([2, 4, 3], $state->getOutput());
     }
 
+    public function testTransformersCanBeGivenAsAList(): void
+    {
+        $state = $this->execute([
+            'transformers' => [
+                ['mapping' => ['mapping' => ['values' => ['code' => '[values]', 'transformers' => ['trim' => null]]]]],
+                'array_last',
+                ['callback' => ['callback' => 'explode', 'left_parameters' => [',']]],
+                ['callback' => ['callback' => 'array_reverse']],
+            ],
+        ], ['values' => ' 1,2,3 ']);
+
+        self::assertNull($state->getException());
+        self::assertSame(['3', '2', '1'], $state->getOutput());
+    }
+
     public function testNonConfigurableTransformerAcceptsNullOptions(): void
     {
         $state = $this->execute(['transformers' => ['array_last' => null]], [1, 2, 3]);

tests/Transformer/GenericTransformerTest.php

@@ -71,6 +71,23 @@ public function testOptionalOptionWithoutDefaultIsNullWhenOmitted(): void
         $this->assertSame('ello', $transformer->transform('hello', $options));
     }
 
+    public function testTransformersCanBeGivenAsAList(): void
+    {
+        $registry = new TransformerRegistry();
+        $registry->addTransformer(new CallbackTransformer());
+
+        $transformer = new GenericTransformer(new ContextualOptionResolver(), $registry);
+        $transformer->initialize('substr_upper', [
+            'contextual_options' => ['offset' => ['required' => true]],
+            'transformers' => [
+                ['callback' => ['callback' => 'substr', 'right_parameters' => ['{{ offset }}']]],
+                ['callback' => ['callback' => 'strtoupper']],
+            ],
+        ]);
+
+        $this->assertSame('LLO', $transformer->transform('hello', $this->resolveOptions($transformer, ['offset' => 2])));
+    }
+
     public function testGetCodeRequiresInitialization(): void
     {
         $transformer = new GenericTransformer(new ContextualOptionResolver(), new TransformerRegistry());

tests/Transformer/MappingTransformerTest.php

@@ -304,6 +304,25 @@ public function testSameSubTransformerCanBeUsedMultipleTimesWithSuffixes(): void
         self::assertSame(['field2' => [2, 4, 3]], $transformer->transform(['field' => [3, null, 4, 2]], $options));
     }
 
+    public function testSubTransformersCanBeGivenAsAList(): void
+    {
+        $transformer = $this->createTransformer();
+        $options = $this->resolveOptions($transformer, [
+            'mapping' => [
+                'field2' => [
+                    'code' => '[field]',
+                    'transformers' => [
+                        ['callback' => ['callback' => 'array_filter']],
+                        ['callback' => ['callback' => 'array_reverse']],
+                        ['callback' => ['callback' => 'array_values']],
+                    ],
+                ],
+            ],
+        ]);
+
+        self::assertSame(['field2' => [2, 4, 3]], $transformer->transform(['field' => [3, null, 4, 2]], $options));
+    }
+
     public function testFailingTransformerReportsTheTargetProperty(): void
     {
         $logger = $this->createCollectingLogger();

tests/Transformer/TransformerTraitHolder.php

@@ -35,7 +35,7 @@ public function cleanCode(string $code): string
     }
 
     /**
-     * @param array<string, mixed> $transformers
+     * @param array<int|string, mixed> $transformers
      *
      * @return array<string, \Closure>
      */

tests/Transformer/TransformerTraitTest.php

@@ -14,16 +14,20 @@
 namespace CleverAge\ProcessBundle\Tests\Transformer;
 
 use CleverAge\ProcessBundle\Exception\MissingTransformerException;
+use CleverAge\ProcessBundle\Exception\TransformerException;
 use CleverAge\ProcessBundle\Registry\TransformerRegistry;
 use CleverAge\ProcessBundle\Transformer\CallbackTransformer;
+use CleverAge\ProcessBundle\Transformer\String\TrimTransformer;
 use CleverAge\ProcessBundle\Transformer\TransformerTrait;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 #[\PHPUnit\Framework\Attributes\CoversTrait(TransformerTrait::class)]
 #[\PHPUnit\Framework\Attributes\UsesClass(TransformerRegistry::class)]
 #[\PHPUnit\Framework\Attributes\UsesClass(CallbackTransformer::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(TrimTransformer::class)]
 #[\PHPUnit\Framework\Attributes\UsesClass(MissingTransformerException::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(TransformerException::class)]
 class TransformerTraitTest extends TestCase
 {
     /**
@@ -66,6 +70,68 @@ public function testEmptySuffixIsAnUnknownTransformer(): void
         $this->createHolder()->resolve(['callback#' => ['callback' => 'trim']]);
     }
 
+    public function testTransformersCanBeGivenAsAList(): void
+    {
+        $holder = $this->createHolder();
+
+        $transformers = $holder->resolve([
+            ['callback' => ['callback' => 'trim']],
+            ['callback' => ['callback' => 'strtoupper']],
+            ['callback#reverse' => ['callback' => 'strrev']],
+        ]);
+
+        self::assertSame(['callback#0', 'callback#1', 'callback#reverse#2'], array_keys($transformers));
+        self::assertSame('CBA', $holder->apply($transformers, ' abc '));
+    }
+
+    public function testListedTransformerCodeWithoutOptions(): void
+    {
+        $registry = new TransformerRegistry();
+        $registry->addTransformer(new CallbackTransformer());
+        $registry->addTransformer(new TrimTransformer());
+        $holder = new TransformerTraitHolder($registry);
+
+        $transformers = $holder->resolve(['trim', ['trim' => null], ['callback' => ['callback' => 'strrev']]]);
+
+        self::assertSame('cba', $holder->apply($transformers, ' abc '));
+    }
+
+    /**
+     * @return iterable<string, array{mixed, string}>
+     */
+    public static function invalidListedTransformerProvider(): iterable
+    {
+        yield 'integer' => [1, 'found int'];
+        yield 'null' => [null, 'found null'];
+        yield 'empty string' => ['', 'found string'];
+        yield 'empty map' => [[], 'found array'];
+        yield 'map with several codes' => [['callback' => null, 'trim' => null], 'found array'];
+        yield 'list' => [['callback'], 'found array'];
+    }
+
+    #[DataProvider('invalidListedTransformerProvider')]
+    public function testInvalidListedTransformerThrows(mixed $definition, string $found): void
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage("Transformer at position 1 is invalid : {$found}, expected a transformer code or a single \"code: options\" map");
+
+        $this->createHolder()->resolve([['callback' => ['callback' => 'trim']], $definition]);
+    }
+
+    public function testListedTransformerFailureReportsItsPosition(): void
+    {
+        $holder = $this->createHolder();
+        $transformers = $holder->resolve([
+            ['callback' => ['callback' => 'intval']],
+            ['callback' => ['callback' => 'intdiv', 'right_parameters' => [0]]],
+        ]);
+
+        $this->expectException(TransformerException::class);
+        $this->expectExceptionMessage("Transformation 'callback#1' have failed: Division by zero");
+
+        $holder->apply($transformers, ' 1 ');
+    }
+
     public function testMissingRegistryThrows(): void
     {
         $this->expectException(\LogicException::class);