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); +})();