From 23359d4afc3b0827b4fbbbb9c8aee9102c8be65f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Hubert?= Date: Tue, 9 Jun 2026 17:01:25 +0200 Subject: [PATCH 1/2] Fix "Undefined variable $existingValue" in generated collection mappers --- src/Transformer/AbstractArrayTransformer.php | 18 +-- .../expected.data | 12 ++ .../CollectionWithoutDeepPopulate/map.php | 105 ++++++++++++++++++ 3 files changed, 126 insertions(+), 9 deletions(-) create mode 100644 tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data create mode 100644 tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php diff --git a/src/Transformer/AbstractArrayTransformer.php b/src/Transformer/AbstractArrayTransformer.php index 4e2f69ca..f29a3c8c 100644 --- a/src/Transformer/AbstractArrayTransformer.php +++ b/src/Transformer/AbstractArrayTransformer.php @@ -47,6 +47,15 @@ public function transform(Expr $input, Expr $target, PropertyMetadata $propertyM $existingValue = new Expr\Variable($uniqueVariableScope->getUniqueName('existingValue')); $assignByRef = $this->itemTransformer instanceof AssignedByReferenceTransformerInterface && $this->itemTransformer->assignByRef(); + // Pre-compute $existingValue BEFORE the inner transformer is called: it consumes + // $existingValue (via withNewContext) and emitting the assignment afterwards + // produced "Undefined variable" warnings in PHP 8+. + if ($propertyMapping->target->readAccessor !== null && $this->itemTransformer instanceof IdentifierHashInterface) { + $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 +98,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 +136,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..edaf9e1a --- /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" + } + ] +} \ No newline at end of file diff --git a/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php new file mode 100644 index 00000000..4af41115 --- /dev/null +++ b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php @@ -0,0 +1,105 @@ + + * 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 TargetItem[] */ + public function getItems(): array + { + return $this->items; + } +} + +return (function () { + $autoMapper = AutoMapperBuilder::buildAutoMapper(mapPrivatePropertiesAndMethod: true); + + $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); +})(); From baecef1d3ae185e9ac2e4631d357f9dac5e82673 Mon Sep 17 00:00:00 2001 From: Joel Wurtz Date: Wed, 10 Jun 2026 11:16:11 +0200 Subject: [PATCH 2/2] fix(transformer): fix existing value undefined in some cases --- src/Transformer/AbstractArrayTransformer.php | 6 ++---- .../CollectionWithoutDeepPopulate/expected.data | 2 +- .../CollectionWithoutDeepPopulate/map.php | 11 +++-------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/Transformer/AbstractArrayTransformer.php b/src/Transformer/AbstractArrayTransformer.php index f29a3c8c..09d69939 100644 --- a/src/Transformer/AbstractArrayTransformer.php +++ b/src/Transformer/AbstractArrayTransformer.php @@ -44,13 +44,11 @@ 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; - // Pre-compute $existingValue BEFORE the inner transformer is called: it consumes - // $existingValue (via withNewContext) and emitting the assignment afterwards - // produced "Undefined variable" warnings in PHP 8+. 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'))))); diff --git a/tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data index edaf9e1a..51aa4257 100644 --- a/tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data +++ b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/expected.data @@ -9,4 +9,4 @@ AutoMapper\Tests\AutoMapperTest\CollectionWithoutDeepPopulate\Target { +label: "second" } ] -} \ No newline at end of file +} diff --git a/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php index 4af41115..abc50775 100644 --- a/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php +++ b/tests/AutoMapperTest/CollectionWithoutDeepPopulate/map.php @@ -71,20 +71,15 @@ public function removeItem(TargetItem $item): void if ($existing->id === $item->id) { unset($this->items[$key]); $this->items = array_values($this->items); + return; } } } - - /** @return TargetItem[] */ - public function getItems(): array - { - return $this->items; - } } -return (function () { - $autoMapper = AutoMapperBuilder::buildAutoMapper(mapPrivatePropertiesAndMethod: true); +return (static function () { + $autoMapper = AutoMapperBuilder::buildAutoMapper(); $source = new Source();