🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@rvanlaak
Copy link
Contributor

@rvanlaak rvanlaak commented Sep 18, 2025

Q A
Branch? 7.3
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #61348
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.

@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.4" but it seems your PR description refers to branch "7.3".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@frozenbrain
Copy link

frozenbrain commented Sep 18, 2025

If non-promoted constructor parameters are now valid mapping targets, should we also support putting the Map attribute on those as well? Promoted parameters already support it, therefore I believe this should be also possible for non-promoted ones.

Also, one thing I noticed is that with this change a property with the same name as the constructor parameter has to exist, is that intentional? For example, the following code throws an exception, because Target::$name does not exist:

class Target {
    #[Map(if: false)]
    private string $nameUpper;

    public function __construct(string $name)
    {
        $this->nameUpper = strtoupper($name);
    }
}

$mapper = new ObjectMapper();

$source = new stdClass();
$source->name = 'John';

$target = $mapper->map($source, Target::class);

@rvanlaak
Copy link
Contributor Author

@frozenbrain nice catch, the PR now covers that case as well. See commit e6009b3 for details on the fix.

The issue related to the following comment:

// this may be filled later on see storeValue

When the property name differs from the constructor argument name, the constructor argument's value did actually not get filled in at a later time. It are the properties that get iterated over later on, which isn't aware of the constructor argument name anymore (as property field had a different name).

@rvanlaak
Copy link
Contributor Author

Failing check is unrelated.

@soyuka can you help with reviewing this PR?

Copy link
Contributor

@soyuka soyuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed good catch

@fabpot fabpot modified the milestones: 7.4, 7.3 Sep 20, 2025
@fabpot fabpot force-pushed the objectmapper-fix-61348 branch from d82e183 to 94fb2a6 Compare September 20, 2025 14:49
@fabpot
Copy link
Member

fabpot commented Sep 20, 2025

Thank you @rvanlaak.

@fabpot fabpot merged commit f324858 into symfony:7.3 Sep 20, 2025
10 of 11 checks passed
@rvanlaak rvanlaak deleted the objectmapper-fix-61348 branch September 23, 2025 11:06
@fabpot fabpot mentioned this pull request Sep 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ObjectMapper] ObjectMapper::map calling constructor without any arguments

5 participants