Allow passing parameters to projections and property mappings

[Demivan]

Allow passing parameters to projections and property mappings

Description

This PR introduces parameter support for projections and property mappings:

• Allow passing parameters to projections
• Use mapping with parameters to configure projection with parameters
• Pass additional parameters to property mappings

Fixes #1472

Checklist

• [x] The existing code style is followed
• [x] The commit message follows our guidelines
• [x] Performed a self-review of my code
• [x] Hard-to-understand areas of my code are commented
• [ ] The documentation is updated (as applicable)
• [x] Unit tests are added/updated
• [x] Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

[latonz]

Thank you for the contribution. Before I dive into a detailed review, I think we should first align on the intended behavior. As far as I can see, the current approach doesn’t work with existing target mappings (and [MappingTarget]), right? See Riok.Mapperly.Tests.Mapping.UserMethodAdditionalParametersTest.ExistingTarget...

At the moment, Mapperly matches additional mapping parameters by name. I believe we should also match them by name when passing them to downstream mapping methods.

How does your approach handle the following scenario:

record A(ANested Nested);
record B(BNested Nested);
record ANested(int ValueA);
record BNested(int ValueB, int Ctx);

public partial B Map(A src, int ctx);

Does this automatically generate private BNested MapToB(B src, int ctx) and pass the ctx along? How does this work if there are multiple levels of nesting with an in-between level which is user-defined without the additional parameters?

[Demivan]

@latonz All of these are valid use cases. I will investigate and fix them. My implementation was mostly about passing parameters to [MapProperty] methods and using the correct Map method for the Project method with parameters. Or maybe these mappings can be fixed in a separate PR?

[latonz]

These issues can be addressed in a separate PR (which might make more sense, otherwise this one could become too big). However, the overall concept of how everything should work needs to be clarified first.

[Demivan]

However, the overall concept of how everything should work needs to be clarified first.

Sure.

At the moment, Mapperly matches additional mapping parameters by name. I believe we should also match them by name when passing them to downstream mapping methods.

In this PR, I match parameters by name and type. I think, it would make sense to match them only by name and map them if needed.

As far as I can see, the current approach doesn’t work with existing target mappings (and [MappingTarget]), right?

I didn't consider [MappingTarget]. But it would make sense to support it.

Does this automatically generate private BNested MapToB(B src, int ctx) and pass the ctx along?

Currently, this functionality does not work. And I think, it would be extremely tricky to do this automatically. Maybe it is better to only support this when mapping methods are manually defined.

Currently, some diagnostics are incorrect. I'll fix those.

[trejjam]

Hi, thank you for opening this. I am also looking forward to this feature.

When I was thinking about how to implement this, I thought to support only existing "extended" mapping:

record A(ANested Nested);
record B(BNested Nested);
record ANested(int ValueA);
record BNested(int ValueB, int Ctx);

public partial B Map(A src, int ctx);
private partial BNested Map(ANested src, int ctx); // <-- this is mandatory

If there is no BNested Map(ANested src, int ctx); the B Map(A src, int ctx) does not try to pass the parameter to the nested objects and simply reports "no target for ctx". I find such an automated search a bit dangerous.

[latonz]

Sounds good to me:

• Match by name
• Don't generate methods with additional parameters (only user-defined methods with additional parameters are supported)
• Support existing target mappings and [MappingTarget]

[latonz]

I’ve converted this PR to a draft. Please mark it as ready for review once it’s complete and request a review from me. Thank you!

[Demivan]

Sorry, I don't have time to continue with this right now. And it needs a different approach anyway. With the current approach, there is no easy way to set a mapping parameter as used. I'm closing this for now. I will open a new PR, if I find time to work on this more.

[latonz]

Thank you for your efforts anyway!