From 2bab6f924994f0a67143da9294f3cfa3860f8c23 Mon Sep 17 00:00:00 2001 From: amirhosin6402 Date: Sat, 18 Jul 2026 15:13:31 +0330 Subject: [PATCH] fix: guard nullable paths in existing target mappings Use null-conditional source access when building existing-target null guards so nullable intermediate members are not dereferenced. Add regression coverage for nullable-aware and disabled-nullability contexts. --- .../NullDelegateExistingTargetMapping.cs | 7 +- .../MemberExistingTargetMapping.cs | 9 ++- .../Mapping/EnumerableExistingTargetTest.cs | 71 +++++++++++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/src/Riok.Mapperly/Descriptors/Mappings/ExistingTarget/NullDelegateExistingTargetMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/ExistingTarget/NullDelegateExistingTargetMapping.cs index e4a15cbcf7..c08664d7e5 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/ExistingTarget/NullDelegateExistingTargetMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/ExistingTarget/NullDelegateExistingTargetMapping.cs @@ -19,7 +19,10 @@ IExistingTargetMapping delegateMapping { private const string NullableValueProperty = nameof(Nullable<>.Value); - public override IEnumerable Build(TypeMappingBuildContext ctx, ExpressionSyntax target) + public override IEnumerable Build(TypeMappingBuildContext ctx, ExpressionSyntax target) => + Build(ctx, target, ctx.Source); + + public IEnumerable Build(TypeMappingBuildContext ctx, ExpressionSyntax target, ExpressionSyntax sourceNullCheckAccess) { // if the source or target type is nullable, add a null guard. if (!SourceType.IsNullable() && !TargetType.IsNullable()) @@ -32,7 +35,7 @@ public override IEnumerable Build(TypeMappingBuildContext ctx, return []; // if (source != null && target != null) { body } - var condition = IfNoneNull((SourceType, ctx.Source), (TargetType, target)); + var condition = IfNoneNull((SourceType, sourceNullCheckAccess), (TargetType, target)); var ifStatement = ctx.SyntaxFactory.If(condition, body); return [ifStatement]; } diff --git a/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberExistingTargetMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberExistingTargetMapping.cs index fcb869c0c0..3c9bef0d5d 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberExistingTargetMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberExistingTargetMapping.cs @@ -27,6 +27,13 @@ public IEnumerable Build(TypeMappingBuildContext ctx, Expressio { var source = sourcePath.BuildAccess(ctx.Source); var target = targetPath.BuildAccess(targetAccess); - return delegateMapping.Build(ctx.WithSource(source), target); + var mappingCtx = ctx.WithSource(source); + if (delegateMapping is NullDelegateExistingTargetMapping nullDelegateMapping) + { + var nullConditionalSource = sourcePath.BuildAccess(ctx.Source, nullConditional: true); + return nullDelegateMapping.Build(mappingCtx, target, nullConditionalSource); + } + + return delegateMapping.Build(mappingCtx, target); } } diff --git a/test/Riok.Mapperly.Tests/Mapping/EnumerableExistingTargetTest.cs b/test/Riok.Mapperly.Tests/Mapping/EnumerableExistingTargetTest.cs index ea61782d50..f80253797a 100644 --- a/test/Riok.Mapperly.Tests/Mapping/EnumerableExistingTargetTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/EnumerableExistingTargetTest.cs @@ -1,4 +1,5 @@ using Riok.Mapperly.Abstractions; +using Riok.Mapperly.Diagnostics; namespace Riok.Mapperly.Tests.Mapping; @@ -68,6 +69,76 @@ public void EnumerableToExistingNullableValueTypeCustomCollection() ); } + [Fact] + public void NestedEnumerableToExistingReadOnlyCollectionInDisabledNullableContext() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + "[MapProperty(new[] { nameof(Source.Nested), nameof(Nested.Items) }, nameof(Target.Items))] partial Target Map(Source source);", + "class Source { public Nested Nested { get; set; } }", + "class Nested { public IEnumerable Items { get; set; } }", + "class Target { public readonly List Items = []; }" + ); + + TestHelper + .GenerateMapper(source, TestHelperOptions.DisabledNullable) + .Should() + .HaveSingleMethodBody( + """ + if (source == null) + return default; + var target = new global::Target(); + if (source.Nested?.Items != null && target.Items != null) + { + if (global::System.Linq.Enumerable.TryGetNonEnumeratedCount(source.Nested.Items, out var sourceCount)) + { + target.Items.EnsureCapacity(sourceCount + target.Items.Count); + } + foreach (var item in source.Nested.Items) + { + target.Items.Add(item); + } + } + return target; + """ + ); + } + + [Fact] + public void NestedEnumerableToExistingReadOnlyCollectionWithNullableIntermediateMember() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + "[MapProperty(new[] { nameof(Source.Nested), nameof(Nested.Items) }, nameof(Target.Items))] partial Target Map(Source source);", + "class Source { public Nested? Nested { get; set; } }", + "class Nested { public IEnumerable Items { get; set; } = []; }", + "class Target { public readonly List Items = []; }" + ); + + TestHelper + .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .Should() + .HaveDiagnostic( + DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType, + "Mapping the nullable source of type System.Collections.Generic.IEnumerable? to target of type System.Collections.Generic.List which is not nullable" + ) + .HaveSingleMethodBody( + """ + var target = new global::Target(); + if (source.Nested?.Items != null) + { + if (global::System.Linq.Enumerable.TryGetNonEnumeratedCount(source.Nested.Items, out var sourceCount)) + { + target.Items.EnsureCapacity(sourceCount + target.Items.Count); + } + foreach (var item in source.Nested.Items) + { + target.Items.Add(item); + } + } + return target; + """ + ); + } + [Fact] public Task MapToExistingCollectionShouldWork() {