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

Access to private constructor with IncludedMembers in attribute #997

Open
cbrevik opened this issue Dec 12, 2023 · 4 comments
Open

Access to private constructor with IncludedMembers in attribute #997

cbrevik opened this issue Dec 12, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@cbrevik
Copy link

cbrevik commented Dec 12, 2023

Is your feature request related to a problem? Please describe.
Thanks for the new IncludedMembers features with private member mapping, I love it! What would put me over the moon though is if it could also use private constructors to initialise the object 😍

I'm not sure if that's possible at all, but I see the UnsafeAccessorKind-enum has a value for private constructors so I'm just guessing that it is possible somehow? Seems so from this blog post: https://www.meziantou.net/accessing-private-members-without-reflection-in-csharp.htm#constructors

Describe the solution you'd like
If I have a parameterless private constructor, the generated mapping code would still be able to initialise the object.

Describe alternatives you've considered
An alternative is exposing a public constructor of course, but I'm trying to map a database model to our domain models. The domain modal has explicit boundaries with Create-methods etc for our normal business code, but I'd still love to seamlessly map/project our domain model from database entities.

Additional context

public class TestStuff
{
    private TestStuff()
    {
    }

    public int Id { get; private set; }

    public static TestStuff Create(int id)
    {
        return new TestStuff { Id = id };
    }
}

public class AnotherStuff
{
    public int Id { get; set; }
}

[Mapper(IncludedMembers = MemberVisibility.All)]
internal static partial class TestStuffMapper
{
    public static partial TestStuff ToTestStuff(this AnotherStuff stuff);
}

https://github.com/riok/mapperly/assets/4932625/8037261c-a370-4e2a-898d-512fd16bd73b

@latonz
Copy link
Contributor

latonz commented Dec 12, 2023

That should definitely be possible and would be a great addition to Mapperly.

Would be happy to accept a PR implementing this.

The constructor mapping is built in Riok.Mapperly.Descriptors.MappingBodyBuilders.NewInstanceObjectMemberMappingBodyBuilder.

Contributing docs may give you a good introduction on how to contribute to Mapperly and about its tooling and architecture.

@latonz latonz added the enhancement New feature or request label Dec 12, 2023
@latonz
Copy link
Contributor

latonz commented Dec 13, 2023

Notes for the implementation:

  • Look for callers of SymbolAccessor.HasDirectlyAccessibleParameterlessConstructor and SymbolAccessor.IsDirectlyAccessible, decide for each caller whether private constructors should be supported
  • In IQueryable projections constructors not directly accessible should never be supported
  • CtorMapping: converting a type by calling a constructor of the target type with the source as single parameter
  • Object member mapping constructors: init only properties cannot be used then, logic should probably be similar as with an object factory
  • Collections: eg. EnumerableMappingBuilder.BuildCustomTypeMapping, DictionaryMappingBuilder

Idea:

  • implement a new UnsafeConstructorAccessor, this cannot work as extension method...
  • to keep PR's small, probably split in 3 PRs: CtorMapping, Object member mapping constructors and collections

@Ewerton
Copy link

Ewerton commented Feb 5, 2024

I would love to implement it, but, besides being an experienced developer I'm totally new to Code Generators, I briefly examined the code of Mapperly and I'm not sure I can implement it without proper guidance.
Is there any communication channel where I can get guidance and feedback to try to implement this enhancement?

@latonz
Copy link
Contributor

latonz commented Feb 5, 2024

Thank you in your interest implementing this. Currently we provide contributor documentation here. My recommendation would be to debug a few unit tests through the entire process. This helps you to get a general understanding on how things work (debugging Mapperly is described here). Let us know if you think anything is missing on the contribution docs or create a PR yourself to improve it 😊
We don't provide another communication channel for Mapperly beside GitHub, but you can ask all your questions here in the issue. As soon as you have an early version of the feature you can open a draft PR and we will provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants