diff --git a/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs index 341ca8bdfe..b45711c9c2 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs @@ -10,88 +10,113 @@ namespace Riok.Mapperly.Descriptors.Mappings; /// /// Null aware delegate mapping. Abstracts handling null values of the delegated mapping. /// -public class NullDelegateMapping : NewInstanceMapping +/// +/// This mapping handles null propagation by wrapping a delegate mapping with appropriate +/// null checks and fallback values. It optimizes away when no null handling is required. +/// +public class NullDelegateMapping( + ITypeSymbol nullableSourceType, + ITypeSymbol nullableTargetType, + INewInstanceMapping delegateMapping, + NullFallbackValue nullFallbackValue +) : NewInstanceMapping(nullableSourceType, nullableTargetType) { private const string NullableValueProperty = nameof(Nullable<>.Value); - private readonly INewInstanceMapping _delegateMapping; - private readonly NullFallbackValue _nullFallbackValue; - - public NullDelegateMapping( - ITypeSymbol nullableSourceType, - ITypeSymbol nullableTargetType, - INewInstanceMapping delegateMapping, - NullFallbackValue nullFallbackValue - ) - : base(nullableSourceType, nullableTargetType) - { - _delegateMapping = delegateMapping; - _nullFallbackValue = nullFallbackValue; - - // the mapping is synthetic (produces no code) - // if and only if the delegate mapping is synthetic (produces also no code) - // and no null handling is required - // (this is the case if the delegate mapping source type accepts nulls - // or the source type is not nullable and the target type is not a nullable value type (otherwise a conversion is needed)). - IsSynthetic = - _delegateMapping.IsSynthetic - && (_delegateMapping.SourceType.IsNullable() || !SourceType.IsNullable() && !TargetType.IsNullableValueType()); - } - - public override bool IsSynthetic { get; } + /// + /// Indicates whether this mapping produces no code (is synthetic). + /// + /// + /// The mapping is synthetic when: + /// + /// The delegate mapping is synthetic AND + /// No null handling is required (delegate accepts nulls, source isn't nullable, or both types are nullable) + /// + /// + public override bool IsSynthetic { get; } = ComputeIsSynthetic(delegateMapping, nullableSourceType, nullableTargetType); public override ExpressionSyntax Build(TypeMappingBuildContext ctx) { - if (_delegateMapping.SourceType.IsNullable()) - return _delegateMapping.Build(ctx); + // Fast paths: delegate handles nulls or no transformation needed + if (delegateMapping.SourceType.IsNullable() || IsSynthetic) + { + return delegateMapping.Build(ctx); + } + // Non-nullable source: may need cast for nullable value type targets if (!SourceType.IsNullable()) { - // if the target type is a nullable value type, there needs to be an additional cast in some cases - // (e.g. in a linq expression, int => int?) - return TargetType.IsNullableValueType() - ? CastExpression(FullyQualifiedIdentifier(TargetType), _delegateMapping.Build(ctx)) - : _delegateMapping.Build(ctx); + return BuildNonNullableSourceMapping(ctx); } - // source is nullable and the mapping method cannot handle nulls, - // call mapping only if source is not null. - // with coalesce: Map(source ?? throw) - // or with if-else: - // source == null ? : Map(source) - // or for nullable value types: - // source == null ? : Map(source.Value) - var nullSubstitute = NullSubstitute(TargetType, ctx.Source, _nullFallbackValue); - - // if throw is used instead of a default value - // and the delegate mapping is a synthetic or method mapping - // the coalesce operator can be used - // (the Map method isn't invoked in the null case since the exception is thrown, - // and it is ensured no parentheses are needed since it is directly used or is passed as argument). - // This simplifies the generated source code. - if ( - _nullFallbackValue == NullFallbackValue.ThrowArgumentNullException - && (_delegateMapping.IsSynthetic || _delegateMapping is MethodMapping) - ) + // Nullable source with delegate that can't handle nulls: add null check + return BuildNullableSourceMapping(ctx); + } + + private static bool ComputeIsSynthetic(INewInstanceMapping delegateMapping, ITypeSymbol sourceType, ITypeSymbol targetType) + { + if (!delegateMapping.IsSynthetic) + return false; + + // No null handling needed if: + // 1. Delegate already accepts nulls + // 2. Source isn't nullable and target isn't nullable value type (no conversion needed) + // 3. Both source and target are nullable (pass-through) + var delegateAcceptsNulls = delegateMapping.SourceType.IsNullable(); + var noConversionNeeded = !sourceType.IsNullable() && !targetType.IsNullableValueType(); + var bothNullable = sourceType.IsNullable() && targetType.IsNullable(); + + return delegateAcceptsNulls || noConversionNeeded || bothNullable; + } + + private ExpressionSyntax BuildNonNullableSourceMapping(TypeMappingBuildContext ctx) + { + var mappedExpression = delegateMapping.Build(ctx); + + // Cast required for nullable value type targets (e.g., int => int? in LINQ) + return TargetType.IsNullableValueType() ? CastExpression(FullyQualifiedIdentifier(TargetType), mappedExpression) : mappedExpression; + } + + private ExpressionSyntax BuildNullableSourceMapping(TypeMappingBuildContext ctx) + { + var nullSubstitute = NullSubstitute(TargetType, ctx.Source, nullFallbackValue); + + // Optimization: use coalesce operator for throw scenarios with simple mappings + // Generates: Map(source ?? throw new ArgumentNullException(...)) + if (CanUseCoalesceOptimization()) { - ctx = ctx.WithSource(Coalesce(ctx.Source, nullSubstitute)); - return _delegateMapping.Build(ctx); + var coalesceCtx = ctx.WithSource(Coalesce(ctx.Source, nullSubstitute)); + return delegateMapping.Build(coalesceCtx); } - var nonNullableSourceValue = ctx.Source; + // Standard path: conditional expression + // Generates: source == null ? : Map(source.Value) + var nonNullableSource = GetNonNullableSourceExpression(ctx.Source); + var mappedExpression = delegateMapping.Build(ctx.WithSource(nonNullableSource)); + + return Conditional(IsNull(ctx.Source), nullSubstitute, mappedExpression); + } + + private bool CanUseCoalesceOptimization() => + nullFallbackValue == NullFallbackValue.ThrowArgumentNullException + && (delegateMapping.IsSynthetic || delegateMapping is MethodMapping); + + private ExpressionSyntax GetNonNullableSourceExpression(ExpressionSyntax source) + { + var result = source; - // disable nullable waring if accessing array - if (nonNullableSourceValue is ElementAccessExpressionSyntax) + // Suppress nullable warning for array element access + if (result is ElementAccessExpressionSyntax) { - nonNullableSourceValue = PostfixUnaryExpression(SyntaxKind.SuppressNullableWarningExpression, nonNullableSourceValue); + result = PostfixUnaryExpression(SyntaxKind.SuppressNullableWarningExpression, result); } - // if it is a value type, access the value property + // Access .Value for nullable value types if (SourceType.IsNullableValueType()) { - nonNullableSourceValue = MemberAccess(nonNullableSourceValue, NullableValueProperty); + result = MemberAccess(result, NullableValueProperty); } - return Conditional(IsNull(ctx.Source), nullSubstitute, _delegateMapping.Build(ctx.WithSource(nonNullableSourceValue))); + return result; } } diff --git a/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs b/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs index ffe7e762d9..1519eee7c5 100644 --- a/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs +++ b/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs @@ -13,7 +13,7 @@ public static partial int DirectInt(int value) [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] public static partial int? DirectIntNullable(int? value) { - return value == null ? default(int?) : value.Value; + return value; } [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] diff --git a/test/Riok.Mapperly.Tests/Mapping/DictionaryTest.cs b/test/Riok.Mapperly.Tests/Mapping/DictionaryTest.cs index 52c5c90801..49b3a30771 100644 --- a/test/Riok.Mapperly.Tests/Mapping/DictionaryTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/DictionaryTest.cs @@ -39,14 +39,7 @@ public void DictionaryToSameDictionaryDeepCloningInDisabledNullableContext() .Should() .HaveSingleMethodBody( """ - if (source == null) - return default; - var target = new global::System.Collections.Generic.Dictionary(source.Count); - foreach (var item in source) - { - target[item.Key] = item.Value == null ? default : item.Value; - } - return target; + return source == null ? default : new global::System.Collections.Generic.Dictionary(source); """ ); } diff --git a/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs b/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs index d1580a2746..987f09bb13 100644 --- a/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs @@ -163,7 +163,14 @@ TestSourceBuilderOptions.Default with } [Fact] - public void NonNullableToNullableValueType() + public void NullableToNullableType() + { + var source = TestSourceBuilder.Mapping("int[]?", "int[]?"); + TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return source;"); + } + + [Fact] + public void NonNullableToNullableValueType2() { var source = TestSourceBuilder.Mapping("DateTime", "DateTime?"); TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (global::System.DateTime?)source;"); diff --git a/test/Riok.Mapperly.Tests/Mapping/ToObjectDeepCloningTest.cs b/test/Riok.Mapperly.Tests/Mapping/ToObjectDeepCloningTest.cs index 502b1352b1..8cbf1e5b1c 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ToObjectDeepCloningTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ToObjectDeepCloningTest.cs @@ -30,7 +30,7 @@ public void NullableObjectToNullableObjectDeepCloning() TestHelper .GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics) .Should() - .HaveMapMethodBody("return source == null ? default : source;") + .HaveMapMethodBody("return source;") .HaveDiagnostic(DiagnosticDescriptors.MappedObjectToObjectWithoutDeepClone) .HaveAssertedAllDiagnostics(); } diff --git a/test/Riok.Mapperly.Tests/_snapshots/DictionaryTest.DictionaryToDictionaryWithNonNullableKeyAndValueDeepCloningInDisabledNullableContext#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/DictionaryTest.DictionaryToDictionaryWithNonNullableKeyAndValueDeepCloningInDisabledNullableContext#Mapper.g.verified.cs index 8148c08cd6..09a1fc8a46 100644 --- a/test/Riok.Mapperly.Tests/_snapshots/DictionaryTest.DictionaryToDictionaryWithNonNullableKeyAndValueDeepCloningInDisabledNullableContext#Mapper.g.verified.cs +++ b/test/Riok.Mapperly.Tests/_snapshots/DictionaryTest.DictionaryToDictionaryWithNonNullableKeyAndValueDeepCloningInDisabledNullableContext#Mapper.g.verified.cs @@ -12,7 +12,7 @@ public partial class Mapper var target = new global::B(); if (source.Value != null) { - target.Value = MapToDictionaryOfStringAndString(source.Value); + target.Value = new global::System.Collections.Generic.Dictionary(source.Value); } else { @@ -20,15 +20,4 @@ public partial class Mapper } return target; } - - [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] - private global::System.Collections.Generic.Dictionary MapToDictionaryOfStringAndString(global::System.Collections.Generic.IReadOnlyDictionary source) - { - var target = new global::System.Collections.Generic.Dictionary(source.Count); - foreach (var item in source) - { - target[item.Key] = item.Value == null ? default : item.Value; - } - return target; - } } \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.NestedPropertyWithDeepCloneable#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.NestedPropertyWithDeepCloneable#Mapper.g.verified.cs index 79fcb4e556..5a2692a7a7 100644 --- a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.NestedPropertyWithDeepCloneable#Mapper.g.verified.cs +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.NestedPropertyWithDeepCloneable#Mapper.g.verified.cs @@ -30,7 +30,7 @@ public partial class Mapper var target = new global::B(); if (source.Nested?.Value0 != null) { - target.Value0 = MapToStringArray(source.Nested.Value0); + target.Value0 = (string?[])source.Nested.Value0.Clone(); } else { @@ -39,15 +39,4 @@ public partial class Mapper target.Value = source.Nested?.Value; return target; } - - [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] - private string?[] MapToStringArray(string?[] source) - { - var target = new string?[source.Length]; - for (var i = 0; i < source.Length; i++) - { - target[i] = source[i] == null ? default : source[i]!; - } - return target; - } } \ No newline at end of file