diff --git a/CHANGELOG.md b/CHANGELOG.md index 74a31fd2..92d744af 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/docs/reference/transformers/mapping_transformer.md b/docs/reference/transformers/mapping_transformer.md index 9a42349f..af877a5e 100644 --- a/docs/reference/transformers/mapping_transformer.md +++ b/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. diff --git a/src/Transformer/MappingTransformer.php b/src/Transformer/MappingTransformer.php index b7debb35..dbfc5890 100644 --- a/src/Transformer/MappingTransformer.php +++ b/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. * diff --git a/tests/Transformer/MappingTransformerTest.php b/tests/Transformer/MappingTransformerTest.php index dbc665ee..d341c559 100644 --- a/tests/Transformer/MappingTransformerTest.php +++ b/tests/Transformer/MappingTransformerTest.php @@ -36,6 +36,11 @@ #[\PHPUnit\Framework\Attributes\UsesClass(TransformerException::class)] class MappingTransformerTest extends TestCase { + /** + * @var list + */ + 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 + */ + 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 $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 $options *