feat: support [MappingTargetOriginalValue] parameter in user-defined mapping methods by juliancarrivick · Pull Request #2306 · riok/mapperly

juliancarrivick · 2026-06-06T17:08:57Z

Support [MappingTargetOriginalValue] parameter in user-defined mapping methods

Description

Following feedback in #2242, fixing #276 instead to cover the Optional use-case.

Here we add MappingTargetOriginalValueAttribute, which marks a parameter in a user-implemented mapping method to receive the existing target member value. Useful for merge-style mappings that need to combine source and destination data rather than fully overwrite.

Support for [MappingTarget] is not included as the use-case there is to work with entire object references instead of members. In that case both the "target" and the "original" value will end up being the same reference lead to potential confusion.

Thank you for considering this PR.

Checklist

I did not use AI tools to generate this PR, or I have manually verified that the code is correct, optimal, and follows the project guidelines and architecture
- Used to implement tests and generate first pass at implementation. Iterated heavily after that point to avoid leaking state everywhere (particularly around setting MemberAssignmentMapping.TargetValueGetter in MemberMappingBuilder)
I understand that low-quality, AI-generated PRs will be closed immediately without further explanation
The existing code style is followed
The commit message follows our guidelines
Performed a self-review of my code
Hard-to-understand areas of my code are commented
The documentation is updated (as applicable)
Unit tests are added/updated
Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

juliancarrivick · 2026-06-06T17:11:44Z

    public static bool TryBuild(
        IMembersBuilderContext<IMapping> ctx,
        MemberMappingInfo memberMappingInfo,
        CodeStyle codeStyle,
-        [NotNullWhen(true)] out ISourceValue? sourceValue
+        [NotNullWhen(true)] out SourceValueBuilder.MappedSourceValue? result
    )


note: This function really belongs in SourceValueBuilder - that is the only consumer and it leads to the confusing MemberMappingBuilder.TryBuildContainerAssignment/TryBuildAssignment() -> SourceValueBuilder.TryBuildMappedSourceValue() -> MemberMappingBuilder.TryBuild(). I haven't refactored that here to avoid an even larger diff, but can do so if you like.

Thanks for the note. I'd be happy to see that refactoring, but I'd prefer to keep it separate PR from this change. If you're willing to, feel free to submit a follow-up PR 😊

latonz

Thank you for this contribution! I added my feedback.

latonz · 2026-06-22T13:45:26Z

    public static bool TryBuild(
        IMembersBuilderContext<IMapping> ctx,
        MemberMappingInfo memberMappingInfo,
        CodeStyle codeStyle,
-        [NotNullWhen(true)] out ISourceValue? sourceValue
+        [NotNullWhen(true)] out SourceValueBuilder.MappedSourceValue? result
    )


Thanks for the note. I'd be happy to see that refactoring, but I'd prefer to keep it separate PR from this change. If you're willing to, feel free to submit a follow-up PR 😊

latonz · 2026-06-22T13:49:47Z

The vulnerability scanning which fails the test GH Action job should be gone once you rebase onto main.

juliancarrivick · 2026-06-23T07:51:39Z

Thanks for the review @latonz ! I'm currently on holiday but will address your comments after I'm back next week :)

latonz · 2026-06-23T07:52:15Z

enjoy your time off!

juliancarrivick

Alright, I think I've covered everything - let me know if you require any further changes (and build should pass this time).
Thanks!

juliancarrivick · 2026-07-04T07:22:58Z

+
+- Only one `[MappingTargetOriginalValue]` parameter is allowed per method.
+- Only supported on user-implemented (non-`partial`) mapping helper methods. It cannot be used on generated `partial` mapper method declarations.
+- When mapping to a new instance (no `[MappingTarget]`), the destination value is `default(T)` — the type's default — since there is no existing value to read.


This is for the case where the mapper creates an instance of the target itself e.g. in test/Riok.Mapperly.IntegrationTests/MappingTargetOriginalValueMapperTest.cs and there is no opportunity to put a setter.

[Fact] public void MapToDtoShouldUseDefaultWhenOptionalEmpty() { var mapper = new MappingTargetOriginalValueMapper(); var source = new OptionalObject(); var dto = mapper.MapToDto(source); dto.Name.ShouldBeNull(); }

I guess though this is slightly misleading because it won't receive default(T) it will receive the default constructed value on the DTO instead which might be something else.

In any case, I have removed the block as requested :)

juliancarrivick · 2026-07-04T08:30:42Z

    MethodParameter Source,
    MethodParameter? Target,
    MethodParameter? ReferenceHandler,
+    MethodParameter? OriginalValueParameter,


juliancarrivick · 2026-07-04T09:35:14Z

+public class UserMethodMappingTargetOriginalValueTest
+{
+    [Fact]
+    public Task BasicDestinationValueParameter()


Another missed update 😅 Did this and updated the generated files ✔️

latonz

Thanks for the updates, I added my feedback.

latonz · 2026-07-06T10:57:55Z

+                else if (targetOriginalValueParameter is not null && param.Ordinal == targetOriginalValueParameter.Value.Ordinal)
+                    yield return targetOriginalValueParameter.Value.WithArgument(
+                        targetValue
+                            ?? DefaultExpression(SyntaxFactoryHelper.FullyQualifiedIdentifier(targetOriginalValueParameter.Value.Type))


When targetValue is absent, this fallback uses the original parameter symbol type. For generic user-implemented methods used in constructor/init-only mappings, that can emit an unsubstituted type parameter at the call site, e.g. FromOptional<int>(src.Age, default(T?)). Use the concrete target/original-value type for the fallback and add a test for generic [MappingTargetOriginalValue] with an init-only or constructor-mapped member.

Oooh excellent catch. Updated so that GenericUserImplementedNewInstanceMethodMapping will inject the concrete type so the type at this call-site is correct ✅

latonz · 2026-07-06T11:00:33Z

        return ctx.SymbolAccessor.WrapOptionalMethodParameter(targetParameterSymbol);
    }

+    private static MethodParameter? TryFindFirstTargetOriginalValueParameter(


This method name could be simplified to FindTargetOriginalValueParameter as all of these methods only use the first param and also don't include it in the name.

latonz · 2026-07-06T11:01:40Z

-        var refHandlerParameterOrdinal = refHandlerParameter?.Ordinal ?? -1;
+        matchedParameters.AddParameter(refHandlerParameter);

        var sourceParameter = FindSourceParameter(ctx, method, refHandlerParameter);


Shouldn't the source parameter be matched after the targetOriginalValueParameter? Otherwise signatures like FromOptional([MappingTargetOriginalValue] int? original, Optional source) won't work, do they?

Ah yep, another good catch. I've fixed this and added a test case.

latonz · 2026-07-06T11:03:45Z

        return refHandlerParameter;
    }
+
+    private readonly struct MatchedParameterOrdinalList


Does this really add any value? Wouldn't a simple HashSet<int> matchedParamOrdinals be easier to read and have better performance?

I wouldn't expect it to have better performance actually since we're only dealing with O(3) and HashMap will have the hashing overhead. I thought I'd quantitatively check though:

➜ dotnet new console ➜ dotnet add package BenchmarkDotNet ➜ cat > Program.cs << 'EOF' using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Running; BenchmarkRunner.Run<HashSetVsListBenchmark>(); [MemoryDiagnoser] public class HashSetVsListBenchmark { private readonly List<int> _list = new() { 1, 2, 3 }; private readonly HashSet<int> _set = new() { 1, 2, 3 }; [Params(1, 2, 3, 99)] public int Target; [Benchmark(Baseline = true)] public bool ListContains() => _list.Contains(Target); [Benchmark] public bool HashSetContains() => _set.Contains(Target); } EOF ➜ dotnet run -c Release ... | Method | Target | Mean | Error | StdDev | Ratio | Allocated | Alloc Ratio | |---------------- |------- |---------:|----------:|----------:|------:|----------:|------------:| | ListContains | 1 | 1.304 ns | 0.0016 ns | 0.0013 ns | 1.00 | - | NA | | HashSetContains | 1 | 2.197 ns | 0.0124 ns | 0.0097 ns | 1.68 | - | NA | | | | | | | | | | | ListContains | 2 | 1.816 ns | 0.0035 ns | 0.0031 ns | 1.00 | - | NA | | HashSetContains | 2 | 2.206 ns | 0.0156 ns | 0.0130 ns | 1.21 | - | NA | | | | | | | | | | | ListContains | 3 | 1.985 ns | 0.0043 ns | 0.0033 ns | 1.00 | - | NA | | HashSetContains | 3 | 2.209 ns | 0.0121 ns | 0.0113 ns | 1.11 | - | NA | | | | | | | | | | | ListContains | 99 | 2.272 ns | 0.0095 ns | 0.0084 ns | 1.00 | - | NA | | HashSetContains | 99 | 2.216 ns | 0.0135 ns | 0.0120 ns | 0.98 | - | NA |

However, this isn't performance critical code, so HashMap will probably still be fast enough, given you'd prefer it for readability. I wanted to avoid repeating the p?.Ordinal ?? -1 logic but if you feel that's more readable than a separate helper I'm happy to defer to you here.

Noticed on closer inspection that the !Contains() HashSet case is actually marginally faster so maybe this evens out in the wash, regardless, I've updated 👍

…mapping methods Adds MappingTargetOriginalValueAttribute, which marks a parameter in a user-implemented mapping method to receive the existing target member value. Useful for merge-style mappings that need to combine source and destination data rather than fully overwrite. Support for [MappingTarget] is not included as the common use-case there is to take the entire target object and this will end up being the exact same reference as the "original value". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

juliancarrivick commented Jun 6, 2026

View reviewed changes

latonz added the enhancement New feature or request label Jun 22, 2026

latonz requested changes Jun 22, 2026

View reviewed changes

juliancarrivick force-pushed the user-mapping-target-value branch from e5a4ba3 to 4e98d97 Compare July 4, 2026 09:48

juliancarrivick commented Jul 4, 2026

View reviewed changes

latonz requested changes Jul 6, 2026

View reviewed changes

juliancarrivick force-pushed the user-mapping-target-value branch from 4e98d97 to 40b6a78 Compare July 11, 2026 11:38

Uh oh!

Uh oh!

Conversation

juliancarrivick commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Support [MappingTargetOriginalValue] parameter in user-defined mapping methods

Description

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

latonz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

latonz commented Jun 22, 2026

Uh oh!

juliancarrivick commented Jun 23, 2026

Uh oh!

latonz commented Jun 23, 2026

Uh oh!

juliancarrivick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

latonz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

juliancarrivick commented Jun 6, 2026 •

edited

Loading