#117 - MappingTransformer: deprecate nested path target properties on an array destination

CHANGELOG.md

@@ -6,6 +6,7 @@ Latest
 * [#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.
+* [#117](https://github.com/cleverage/process-bundle/issues/117) MappingTransformer: writing a target property that is a nested path (e.g. `address.city`) on an array destination, which silently created an `address.city` literal key, is deprecated and will throw an `\UnexpectedValueException` in v6.0. Use `[address.city]` for a literal key or `[address][city]` for a nested array; a simple property name (e.g. `name`) is still added as a key. 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/transformers/mapping_transformer.md

@@ -137,6 +137,10 @@ Notes
   instead of throwing for a missing array index (`framework.property_access.throw_exception_on_invalid_index`), so
   `ignore_missing` mostly matters for objects.
 * When a sub-transformer fails, the thrown `TransformerException` reports the target property.
+* On an array destination, a target property that is not an index notation is added as a literal key: a simple
+  property name (e.g. `name`) gives the `name` key. Doing so with a nested path (e.g. `address.city`, `address[city]`)
+  is deprecated and will throw an `\UnexpectedValueException` in v6.0: use `[address.city]` to keep a literal key, or
+  `[address][city]` to write in a nested array.
 * A missing property of a `\stdClass` destination is added only for a simple target property name (e.g. `name`): a
   nested path (e.g. `address.city`) or an index notation (e.g. `[name]`) that is not writable throws an
   `\UnexpectedValueException` (`Property '...' is not writable`), as for any other object.

src/Transformer/MappingTransformer.php

@@ -114,8 +114,14 @@ public function transform(mixed $value, array $options = []): mixed
             } elseif ($this->accessor->isWritable($result, $targetProperty)) {
                 $this->accessor->setValue($result, $targetProperty, $transformedValue);
             } elseif (\is_array($result)) {
+                if (!$this->isSimplePropertyName($targetProperty)) {
+                    @trigger_error(
+                        "Setting the target property '{$targetProperty}' as a literal key of an array destination is deprecated, it will throw an \\UnexpectedValueException in v6.0. Use '[{$targetProperty}]' to keep a literal key, or the index notation for nested arrays (e.g. '[a][b]').",
+                        \E_USER_DEPRECATED
+                    );
+                }
                 $result[$targetProperty] = $transformedValue;
-            } elseif ($result instanceof \stdClass && 1 === preg_match('/^[^.[\]]+$/', $targetProperty)) {
+            } elseif ($result instanceof \stdClass && $this->isSimplePropertyName($targetProperty)) {
                 // Only a simple property name can be added to a \stdClass, nested paths are not created
                 $result->{$targetProperty} = $transformedValue;
             } else {
@@ -195,6 +201,15 @@ protected function extractInputValue(mixed $input, string $sourceProperty): mixe
         return $this->accessor->getValue($input, $sourceProperty);
     }
 
+    /**
+     * A simple property name (e.g. "name") is neither a nested path (e.g. "address.city") nor an index notation (e.g.
+     * "[name]").
+     */
+    private function isSimplePropertyName(string $property): bool
+    {
+        return 1 === preg_match('/^[^.[\]]+$/', $property);
+    }
+
     /**
      * Wrap error handling when there is an property access error.
      *

tests/Transformer/MappingTransformerTest.php

@@ -36,6 +36,11 @@
 #[\PHPUnit\Framework\Attributes\UsesClass(TransformerException::class)]
 class MappingTransformerTest extends TestCase
 {
+    /**
+     * @var list<string>
+     */
+    private array $deprecations = [];
+
     public function testGetCode(): void
     {
         self::assertSame('mapping', $this->createTransformer()->getCode());
@@ -442,6 +447,65 @@ public function testMissingNonSimpleTargetPropertyOfAStdClassThrows(string $targ
         $transformer->transform([], $options);
     }
 
+    public function testSimpleTargetPropertyIsAddedToAnArrayDestinationWithoutDeprecation(): void
+    {
+        $transformer = $this->createTransformer();
+        $options = $this->resolveOptions($transformer, [
+            'mapping' => [
+                'field2' => ['code' => '[field]'],
+            ],
+        ]);
+
+        $result = $this->transformCollectingDeprecations($transformer, ['field' => 'value'], $options);
+
+        self::assertSame(['field2' => 'value'], $result);
+        self::assertSame([], $this->deprecations);
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function nonSimpleArrayTargetPropertyProvider(): iterable
+    {
+        yield 'nested path' => ['field2.child'];
+        yield 'index after a property' => ['field2[child]'];
+        yield 'property after an index' => ['[field2].child'];
+    }
+
+    #[DataProvider('nonSimpleArrayTargetPropertyProvider')]
+    public function testNonSimpleTargetPropertyOfAnArrayDestinationIsDeprecated(string $targetProperty): void
+    {
+        $transformer = $this->createTransformer();
+        $options = $this->resolveOptions($transformer, [
+            'mapping' => [
+                $targetProperty => ['code' => '[field]'],
+            ],
+        ]);
+
+        $result = $this->transformCollectingDeprecations($transformer, ['field' => 'value'], $options);
+
+        self::assertSame([$targetProperty => 'value'], $result);
+        self::assertSame([
+            "Setting the target property '{$targetProperty}' as a literal key of an array destination is deprecated, it will throw an \\UnexpectedValueException in v6.0. Use '[{$targetProperty}]' to keep a literal key, or the index notation for nested arrays (e.g. '[a][b]').",
+        ], $this->deprecations);
+    }
+
+    public function testIndexNotationKeepsALiteralKeyWithoutDeprecation(): void
+    {
+        $transformer = $this->createTransformer();
+        $options = $this->resolveOptions($transformer, [
+            'mapping' => [
+                '[field2.child]' => ['code' => '[field]'],
+                '[field3][child]' => ['code' => '[field]'],
+            ],
+        ]);
+
+        $result = $this->transformCollectingDeprecations($transformer, ['field' => 'value'], $options);
+
+        self::assertSame(['field2.child' => 'value', 'field3' => ['child' => 'value']], $result);
+        self::assertSame([], $this->deprecations);
+    }
+
     public function testKeepInputCopiesAnArrayInput(): void
     {
         $transformer = $this->createTransformer();
@@ -584,6 +648,23 @@ public function log($level, \Stringable|string $message, array $context = []): v
         };
     }
 
+    /**
+     * @param array<string, mixed> $options
+     */
+    private function transformCollectingDeprecations(MappingTransformer $transformer, mixed $value, array $options): mixed
+    {
+        set_error_handler(function (int $errno, string $errstr): bool {
+            $this->deprecations[] = $errstr;
+
+            return true;
+        }, \E_USER_DEPRECATED);
+        try {
+            return $transformer->transform($value, $options);
+        } finally {
+            restore_error_handler();
+        }
+    }
+
     /**
      * @param array<string, mixed> $options
      *