Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: error when mapping from stdClass to constructor with nullable/optional arguments #184

Merged

Conversation

MrMeshok
Copy link
Contributor

@MrMeshok MrMeshok commented Sep 12, 2024

I was trying to understand why generated code looks weird when mapping from array/stdClass to class which has constructor with nullable/optional arguments.
NullableTransformer is not supposed to be applied when creating from constructor because nullable/optional already handled.

With this fix, generated code instead of looking like this:

$value_1 = null;
if (null !== $value->ResponseDescription) {
    $value_1 = $value->ResponseDescription;
}
$constructarg_1 = $value_1 ?? NULL;

Look like this:

$constructarg_1 = $value->ResponseDescription ?? NULL;

Which also fixes #181

Copy link
Member

@Korbeil Korbeil left a comment

Choose a reason for hiding this comment

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

Thanks @MrMeshok for this fix, it looks nice ! ✔️ Could you also add a note in the CHANGELOG about your change please ? 🙏

@MrMeshok MrMeshok force-pushed the fix/undefined-property-from-stdClass branch from 018ab22 to f9119da Compare September 12, 2024 16:01
@MrMeshok
Copy link
Contributor Author

@Korbeil Done. Can you merge it today? I am working on another PR and don't want to deal with conflicts in CHANGELOG😁

@Korbeil Korbeil merged commit 7585318 into jolicode:main Sep 12, 2024
5 checks passed
@Korbeil
Copy link
Member

Korbeil commented Sep 12, 2024

@MrMeshok merged, thanks for your contribution ! 🙏

@MrMeshok MrMeshok deleted the fix/undefined-property-from-stdClass branch September 12, 2024 16:04
@MrMeshok
Copy link
Contributor Author

@Korbeil I got regression errors after this fix with classes that have nullable objects as arguments. I'm working on fix and tests for this

MrMeshok added a commit to MrMeshok/automapper that referenced this pull request Sep 13, 2024
MrMeshok added a commit to MrMeshok/automapper that referenced this pull request Sep 13, 2024
Korbeil added a commit that referenced this pull request Sep 13, 2024
Turns out NullableTransformer is needed when creating from constructor
if we have argument what expects nullable object
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undefined property error when mapping from stdClass to class with nullable properties
2 participants