bug symfony#61785 [ObjectMapper] Preserve non-promoted constructor parameters (rvanlaak)

This PR was squashed before being merged into the 7.3 branch.

Discussion
----------

[ObjectMapper] Preserve non-promoted constructor parameters

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | yes
| New feature?  | no <!-- if yes, also update src/**/CHANGELOG.md -->
| Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md -->
| Issues        | Fix symfony#61348 <!-- prefix each issue number with "Fix #"; no need to create an issue if none exists, explain below -->
| License       | MIT

The object mapper does already call the constructor, but its argument values were not determined / collected correctly as non-promoted parameters were skipped. Can not find or think of a valid reason for that. This PR fixes that the non-promoted constructor parameter values will get resolved and thereafter get mapped.

Commits
-------

94fb2a6 [ObjectMapper] Preserve non-promoted constructor parameters

Author: fabpot

src/Symfony/Component/ObjectMapper/ObjectMapper.php

@@ -92,21 +92,23 @@ public function map(object $source, object|string|null $target = null): object
 
         $this->objectMap[$source] = $mappedTarget;
         $ctorArguments = [];
-        $constructor = $targetRefl->getConstructor();
-        foreach ($constructor?->getParameters() ?? [] as $parameter) {
-            if (!$parameter->isPromoted()) {
-                continue;
-            }
-
+        $targetConstructor = $targetRefl->getConstructor();
+        foreach ($targetConstructor?->getParameters() ?? [] as $parameter) {
             $parameterName = $parameter->getName();
-            $property = $targetRefl->getProperty($parameterName);
 
-            if ($property->isReadOnly() && $property->isInitialized($mappedTarget)) {
-                continue;
+            if ($targetRefl->hasProperty($parameterName)) {
+                $property = $targetRefl->getProperty($parameterName);
+
+                if ($property->isReadOnly() && $property->isInitialized($mappedTarget)) {
+                    continue;
+                }
             }
 
-            // this may be filled later on see storeValue
-            $ctorArguments[$parameterName] = $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null;
+            if ($this->isReadable($source, $parameterName)) {
+                $ctorArguments[$parameterName] = $this->getRawValue($source, $parameterName);
+            } else {
+                $ctorArguments[$parameterName] = $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null;
+            }
         }
 
         $readMetadataFrom = $source;
@@ -160,7 +162,7 @@ public function map(object $source, object|string|null $target = null): object
             }
         }
 
-        if (!$mappingToObject && !$map?->transform && $constructor) {
+        if (!$mappingToObject && !$map?->transform && $targetConstructor) {
             try {
                 $mappedTarget->__construct(...$ctorArguments);
             } catch (\ReflectionException $e) {
@@ -187,6 +189,19 @@ public function map(object $source, object|string|null $target = null): object
         return $mappedTarget;
     }
 
+    private function isReadable(object $source, string $propertyName): bool
+    {
+        if ($this->propertyAccessor) {
+            return $this->propertyAccessor->isReadable($source, $propertyName);
+        }
+
+        if (!property_exists($source, $propertyName) && !isset($source->{$propertyName})) {
+            return false;
+        }
+
+        return true;
+    }
+
     private function getRawValue(object $source, string $propertyName): mixed
     {
         if ($this->propertyAccessor) {

src/Symfony/Component/ObjectMapper/Tests/Fixtures/InitializedConstructor/C.php

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\ObjectMapper\Tests\Fixtures\InitializedConstructor;
+
+class C
+{
+    public string $bar;
+
+    public function __construct(string $bar)
+    {
+        $this->bar = $bar;
+    }
+}

src/Symfony/Component/ObjectMapper/Tests/Fixtures/InitializedConstructor/D.php

@@ -0,0 +1,25 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\ObjectMapper\Tests\Fixtures\InitializedConstructor;
+
+use Symfony\Component\ObjectMapper\Attribute\Map;
+
+class D
+{
+    #[Map(if: false)]
+    public string $barUpperCase;
+
+    public function __construct(string $bar)
+    {
+        $this->barUpperCase = strtoupper($bar);
+    }
+}

src/Symfony/Component/ObjectMapper/Tests/ObjectMapperTest.php

@@ -37,6 +37,8 @@
 use Symfony\Component\ObjectMapper\Tests\Fixtures\HydrateObject\SourceOnly;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\InitializedConstructor\A as InitializedConstructorA;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\InitializedConstructor\B as InitializedConstructorB;
+use Symfony\Component\ObjectMapper\Tests\Fixtures\InitializedConstructor\C as InitializedConstructorC;
+use Symfony\Component\ObjectMapper\Tests\Fixtures\InitializedConstructor\D as InitializedConstructorD;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\InstanceCallback\A as InstanceCallbackA;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\InstanceCallback\B as InstanceCallbackB;
 use Symfony\Component\ObjectMapper\Tests\Fixtures\InstanceCallbackWithArguments\A as InstanceCallbackWithArgumentsA;
@@ -171,6 +173,36 @@ public function testMapWithInitializedConstructor()
         $this->assertEquals($b->tags, ['foo', 'bar']);
     }
 
+    public function testMapReliesOnConstructorsOwnInitialization()
+    {
+        $expected = 'bar';
+
+        $mapper = new ObjectMapper(propertyAccessor: PropertyAccess::createPropertyAccessor());
+
+        $source = new \stdClass();
+        $source->bar = $expected;
+
+        $c = $mapper->map($source, InitializedConstructorC::class);
+
+        $this->assertInstanceOf(InitializedConstructorC::class, $c);
+        $this->assertEquals($expected, $c->bar);
+    }
+
+    public function testMapConstructorArgumentsDifferFromClassFields()
+    {
+        $expected = 'bar';
+
+        $mapper = new ObjectMapper(propertyAccessor: PropertyAccess::createPropertyAccessor());
+
+        $source = new \stdClass();
+        $source->bar = $expected;
+
+        $actual = $mapper->map($source, InitializedConstructorD::class);
+
+        $this->assertInstanceOf(InitializedConstructorD::class, $actual);
+        $this->assertStringContainsStringIgnoringCase($expected, $actual->barUpperCase);
+    }
+
     public function testMapToWithInstanceHook()
     {
         $a = new InstanceCallbackA();