diff --git a/src/Transformer/AbstractArrayTransformer.php b/src/Transformer/AbstractArrayTransformer.php index 4e2f69ca..09d69939 100644 --- a/src/Transformer/AbstractArrayTransformer.php +++ b/src/Transformer/AbstractArrayTransformer.php @@ -44,8 +44,15 @@ public function transform(Expr $input, Expr $target, PropertyMetadata $propertyM $loopKeyVar = new Expr\Variable($uniqueVariableScope->getUniqueName('key')); $itemStatements = []; - $existingValue = new Expr\Variable($uniqueVariableScope->getUniqueName('existingValue')); $assignByRef = $this->itemTransformer instanceof AssignedByReferenceTransformerInterface && $this->itemTransformer->assignByRef(); + $existingValue = null; + + if ($propertyMapping->target->readAccessor !== null && $this->itemTransformer instanceof IdentifierHashInterface) { + $existingValue = new Expr\Variable($uniqueVariableScope->getUniqueName('existingValue')); + $hashValueTargetVariable = new Expr\Variable($uniqueVariableScope->getUniqueName('hashValueTarget')); + $itemStatements[] = new Stmt\Expression(new Expr\Assign($hashValueTargetVariable, $this->itemTransformer->getSourceHashExpression($loopValueVar))); + $itemStatements[] = new Stmt\Expression(new Expr\Assign($existingValue, new Expr\BinaryOp\Coalesce(new Expr\ArrayDimFetch($exisingValuesIndexed, $hashValueTargetVariable), new Expr\ConstFetch(new Name('null'))))); + } /* Get the transform statements for the source property */ [$output, $transformStatements] = $this->itemTransformer->transform($loopValueVar, $target, $propertyMapping, $uniqueVariableScope, $source, $existingValue); @@ -89,12 +96,7 @@ public function transform(Expr $input, Expr $target, PropertyMetadata $propertyM } $mappedValueVar = new Expr\Variable($uniqueVariableScope->getUniqueName('mappedValue')); - $hashValueTargetVariable = new Expr\Variable($uniqueVariableScope->getUniqueName('hashValueTarget')); - if ($propertyMapping->target->readAccessor !== null && $this->itemTransformer instanceof IdentifierHashInterface) { - $itemStatements[] = new Stmt\Expression(new Expr\Assign($hashValueTargetVariable, $this->itemTransformer->getSourceHashExpression($loopValueVar))); - $itemStatements[] = new Stmt\Expression(new Expr\Assign($existingValue, new Expr\BinaryOp\Coalesce(new Expr\ArrayDimFetch($exisingValuesIndexed, $hashValueTargetVariable), new Expr\ConstFetch(new Name('null'))))); - } $itemStatements[] = new Stmt\Expression(new Expr\Assign($mappedValueVar, $output)); $itemStatements[] = new Stmt\If_(new Expr\BinaryOp\NotIdentical(new Expr\ConstFetch(new Name('null')), $mappedValueVar), [ 'stmts' => [ @@ -132,10 +134,6 @@ public function transform(Expr $input, Expr $target, PropertyMetadata $propertyM ]), ], ]); - - $hashValueTargetVariable = new Expr\Variable($uniqueVariableScope->getUniqueName('hashValueTarget')); - $itemStatements[] = new Stmt\Expression(new Expr\Assign($hashValueTargetVariable, $this->itemTransformer->getSourceHashExpression($loopValueVar))); - $itemStatements[] = new Stmt\Expression(new Expr\Assign($existingValue, new Expr\BinaryOp\Coalesce(new Expr\ArrayDimFetch($exisingValuesIndexed, $hashValueTargetVariable), new Expr\ConstFetch(new Name('null'))))); } $itemStatements[] = new Stmt\Expression($this->getAssignExpr($valuesVar, $output, $loopKeyVar, $assignByRef)); diff --git a/tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data new file mode 100644 index 00000000..51aa4257 --- /dev/null +++ b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data @@ -0,0 +1,12 @@ +AutoMapper\Tests\AutoMapperTest\CollectionWithoutDeepPopulate\Target { + -items: [ + AutoMapper\Tests\AutoMapperTest\CollectionWithoutDeepPopulate\TargetItem { + +id: 1 + +label: "first" + } + AutoMapper\Tests\AutoMapperTest\CollectionWithoutDeepPopulate\TargetItem { + +id: 2 + +label: "second" + } + ] +} diff --git a/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php new file mode 100644 index 00000000..abc50775 --- /dev/null +++ b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php @@ -0,0 +1,100 @@ + + * AbstractArrayTransformer takes the `isAdderRemover()` branch + * - source collection has 2 items -> the inner foreach body executes + * - no `deep_target_to_populate` -> the existing target items are + * not indexed, but $existingvalue is still read inside + * withNewContext() before being assigned (root cause) + * + * On 10.2.0 / current main this raises: + * Warning: Undefined variable $existingvalue + * + * After the patch the warning is gone and the dump matches expected.data. + * + * To catch the warning automatically in CI, set the PHPUnit config: + * + * (already the default in the repo's phpunit.xml.dist). + */ + +namespace AutoMapper\Tests\AutoMapperTest\CollectionWithoutDeepPopulate; + +use AutoMapper\Attribute\MapFrom; +use AutoMapper\Tests\AutoMapperBuilder; + +class SourceItem +{ + public int $id; + public string $label; +} + +class Source +{ + /** @var SourceItem[] */ + public array $items = []; +} + +class TargetItem +{ + #[MapFrom(source: SourceItem::class, identifier: true)] + public int $id; + + public string $label; +} + +class Target +{ + /** @var TargetItem[] */ + private array $items = []; + + public function addItem(TargetItem $item): void + { + $this->items[] = $item; + } + + public function removeItem(TargetItem $item): void + { + foreach ($this->items as $key => $existing) { + if ($existing->id === $item->id) { + unset($this->items[$key]); + $this->items = array_values($this->items); + + return; + } + } + } +} + +return (static function () { + $autoMapper = AutoMapperBuilder::buildAutoMapper(); + + $source = new Source(); + + $a = new SourceItem(); + $a->id = 1; + $a->label = 'first'; + + $b = new SourceItem(); + $b->id = 2; + $b->label = 'second'; + + $source->items = [$a, $b]; + + // No `deep_target_to_populate` and no pre-existing target. Pre-patch: + // raises Warning: Undefined variable $existingvalue. Post-patch: + // clean mapping. + return $autoMapper->map($source, Target::class); +})();