[v6] #234 - ArrayFirstTransformer: restore the meaning of allow_not_iterable

CHANGELOG.md

@@ -31,6 +31,9 @@ Latest
 * [#189](https://github.com/cleverage/process-bundle/issues/189) EventDispatcherTask: when `event_name` is set, listening to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent` is deprecated (the event is still dispatched under its class name, with an `E_USER_DEPRECATED` error, if it has listeners). Listen to the configured `event_name` instead: the BC layer will be removed in v6.0.
 * [#222](https://github.com/cleverage/process-bundle/issues/222) ProcessManager: going on with the process after an exception thrown by a task `initialize()` is deprecated (an `E_USER_DEPRECATED` error is triggered). In v6.0, the process will fail before executing any task.
 
+## BC breaks
+* [#234](https://github.com/cleverage/process-bundle/issues/234) ArrayFirstTransformer: restore the meaning of the `allow_not_iterable` option, inverted since v4.0. By default (`false`), a non-iterable input now throws an `\UnexpectedValueException` (it was returned unchanged); with `allow_not_iterable: true`, it is returned unchanged (it threw a `\TypeError`). Set `allow_not_iterable: true` to keep the former default behaviour. Traversable inputs return their first element. Update documentation, add tests.
+
 v5.0
 -----
 

docs/reference/transformers/array_first_transformer.md

@@ -1,7 +1,7 @@
 ArrayFirstTransformer
 =====================
 
-Return the first element of an array, using `reset()`.
+Return the first element of an array, or of any iterable.
 
 Transformer reference
 ---------------------
@@ -12,21 +12,22 @@ Transformer reference
 Accepted inputs
 ---------------
 
-`array`. With the default options, any non-iterable value is accepted and returned unchanged.
+`iterable` (`array`, `\Traversable`). Any other value throws an `\UnexpectedValueException`, unless
+`allow_not_iterable` is `true`.
 
 Possible outputs
 ----------------
 
-* `any`: the first element of the array
-* `false` if the array is empty
-* the input value itself if it is not iterable and `allow_not_iterable` is `false`
+* `any`: the first element of the iterable
+* `false` if the iterable is empty
+* the input value itself if it is not iterable and `allow_not_iterable` is `true`
 
 Options
 -------
 
-| Code                 | Type   | Required | Default | Description                                                                      |
-|----------------------|--------|:--------:|---------|----------------------------------------------------------------------------------|
-| `allow_not_iterable` | `bool` |          | `false` | When `false`, a non-iterable input is returned unchanged (see [Notes](#notes))   |
+| Code                 | Type   | Required | Default | Description                                                                              |
+|----------------------|--------|:--------:|---------|------------------------------------------------------------------------------------------|
+| `allow_not_iterable` | `bool` |          | `false` | When `true`, a non-iterable input is returned unchanged instead of throwing an exception |
 
 Examples
 --------
@@ -38,9 +39,18 @@ Examples
 array_first: ~
 ```
 
+* Get the first element of a list, or keep a single value as is: `['foo', 'bar']` becomes `'foo'`, `'foo'` stays
+  `'foo'`
+
+```yaml
+# Transformer options level
+array_first:
+  allow_not_iterable: true
+```
+
 Notes
 -----
 
-The `allow_not_iterable` option behaves counter-intuitively: when set to `true`, the non-iterable check is skipped and
-`reset()` is called on the raw value, which throws a `\TypeError` for scalar or `null` inputs (objects are accepted by
-`reset()`, which then returns their first public property). Keep the default value.
+Since v6.0, a non-iterable input throws an exception by default. From v4.0 to v5.x, the option was inverted (a
+non-iterable input was returned unchanged by default, and `allow_not_iterable: true` threw a `\TypeError`): to keep the
+former default behaviour, set `allow_not_iterable: true`.

src/Transformer/Array/ArrayFirstTransformer.php

@@ -17,7 +17,10 @@
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
 /**
- * Return the first element of an array.
+ * Return the first element of an array (or of any iterable).
+ *
+ * A non-iterable input throws an exception, unless the "allow_not_iterable" option is true: it is then returned
+ * unchanged.
  */
 class ArrayFirstTransformer implements ConfigurableTransformerInterface
 {
@@ -26,11 +29,23 @@ class ArrayFirstTransformer implements ConfigurableTransformerInterface
      */
     public function transform(mixed $value, array $options = []): mixed
     {
-        if (false === $options['allow_not_iterable'] && !is_iterable($value)) {
-            return $value;
+        if (!is_iterable($value)) {
+            if ($options['allow_not_iterable']) {
+                return $value;
+            }
+
+            throw new \UnexpectedValueException(\sprintf('Given value is not iterable (%s), set the "allow_not_iterable" option to true to return it unchanged', get_debug_type($value)));
+        }
+
+        if (\is_array($value)) {
+            return reset($value);
+        }
+
+        foreach ($value as $item) {
+            return $item;
         }
 
-        return reset($value);
+        return false;
     }
 
     /**
@@ -46,5 +61,6 @@ public function configureOptions(OptionsResolver $resolver): void
         $resolver->setDefaults([
             'allow_not_iterable' => false,
         ]);
+        $resolver->setAllowedTypes('allow_not_iterable', ['bool']);
     }
 }

tests/Transformer/Array/ArrayFirstTransformerTest.php

@@ -14,67 +14,102 @@
 namespace CleverAge\ProcessBundle\Tests\Transformer\Array;
 
 use CleverAge\ProcessBundle\Transformer\Array\ArrayFirstTransformer;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
 #[\PHPUnit\Framework\Attributes\CoversClass(ArrayFirstTransformer::class)]
-#[\PHPUnit\Framework\Attributes\CoversMethod(ArrayFirstTransformer::class, 'transform')]
-#[\PHPUnit\Framework\Attributes\CoversMethod(ArrayFirstTransformer::class, 'getCode')]
-#[\PHPUnit\Framework\Attributes\CoversMethod(ArrayFirstTransformer::class, 'configureOptions')]
 class ArrayFirstTransformerTest extends TestCase
 {
-    public function testTransformReturnsFirstElementIfIterableAndAllowed(): void
+    /**
+     * @return iterable<string, array{iterable<mixed>, mixed}>
+     */
+    public static function iterableProvider(): iterable
     {
-        $transformer = new ArrayFirstTransformer();
-        $value = [1, 2, 3];
-        $options = ['allow_not_iterable' => false];
+        yield 'list' => [[1, 2, 3], 1];
+        yield 'associative array' => [['a' => 'foo', 'b' => 'bar'], 'foo'];
+        yield 'empty array' => [[], false];
+        yield 'iterator' => [new \ArrayIterator(['foo', 'bar']), 'foo'];
+        yield 'generator' => [(static function (): \Generator {
+            yield 'foo';
+            yield 'bar';
+        })(), 'foo'];
+        yield 'empty iterator' => [new \ArrayIterator([]), false];
+    }
 
-        $result = $transformer->transform($value, $options);
+    /**
+     * @param iterable<mixed> $value
+     */
+    #[DataProvider('iterableProvider')]
+    public function testTransformReturnsTheFirstElement(iterable $value, mixed $expected): void
+    {
+        $transformer = new ArrayFirstTransformer();
 
-        $this->assertEquals(1, $result);
+        self::assertSame($expected, $transformer->transform($value, $this->resolveOptions($transformer)));
     }
 
-    public function testTransformReturnsValueIfNotIterableAndAllowed(): void
+    public function testAllowNotIterableDoesNotChangeIterableValues(): void
     {
-        $this->expectException(\TypeError::class);
-
         $transformer = new ArrayFirstTransformer();
-        $value = 'not_iterable_value';
-        $options = ['allow_not_iterable' => true];
+        $options = $this->resolveOptions($transformer, ['allow_not_iterable' => true]);
 
-        $result = $transformer->transform($value, $options);
+        self::assertSame(1, $transformer->transform([1, 2, 3], $options));
+        self::assertSame('foo', $transformer->transform(new \ArrayIterator(['foo', 'bar']), $options));
+    }
 
-        $this->assertEquals('not_iterable_value', $result);
+    /**
+     * @return iterable<string, array{mixed}>
+     */
+    public static function notIterableProvider(): iterable
+    {
+        yield 'string' => ['not_iterable_value'];
+        yield 'int' => [42];
+        yield 'null' => [null];
+        yield 'object' => [new \stdClass()];
     }
 
-    public function testTransformThrowsExceptionIfNotIterableAndNotAllowed(): void
+    #[DataProvider('notIterableProvider')]
+    public function testNotIterableValueThrowsByDefault(mixed $value): void
     {
         $transformer = new ArrayFirstTransformer();
-        $value = 'not_iterable_value';
-        $options = ['allow_not_iterable' => false];
 
-        $result = $transformer->transform($value, $options);
+        $this->expectException(\UnexpectedValueException::class);
+        $this->expectExceptionMessage('Given value is not iterable');
 
-        $this->assertEquals($value, $result);
+        $transformer->transform($value, $this->resolveOptions($transformer));
     }
 
-    public function testGetCodeReturnsCorrectCode(): void
+    #[DataProvider('notIterableProvider')]
+    public function testNotIterableValueIsReturnedUnchangedWhenAllowed(mixed $value): void
     {
         $transformer = new ArrayFirstTransformer();
 
-        $code = $transformer->getCode();
+        self::assertSame($value, $transformer->transform($value, $this->resolveOptions($transformer, ['allow_not_iterable' => true])));
+    }
 
-        $this->assertEquals('array_first', $code);
+    public function testAllowNotIterableMustBeABoolean(): void
+    {
+        $this->expectException(InvalidOptionsException::class);
+
+        $this->resolveOptions(new ArrayFirstTransformer(), ['allow_not_iterable' => 'yes']);
     }
 
-    public function testConfigureOptionsSetsDefaultOptions(): void
+    public function testGetCode(): void
     {
-        $resolver = new OptionsResolver();
-        $transformer = new ArrayFirstTransformer();
+        self::assertSame('array_first', (new ArrayFirstTransformer())->getCode());
+    }
 
+    /**
+     * @param array<string, mixed> $options
+     *
+     * @return array<string, mixed>
+     */
+    private function resolveOptions(ArrayFirstTransformer $transformer, array $options = []): array
+    {
+        $resolver = new OptionsResolver();
         $transformer->configureOptions($resolver);
-        $resolvedOptions = $resolver->resolve();
 
-        $this->assertEquals(['allow_not_iterable' => false], $resolvedOptions);
+        return $resolver->resolve($options);
     }
 }