fix: remove unnecessary null conditional access in conditions of prop…

…erty mappings (riok#788) Doesn't emit null conditional access in conditions of property mappings if the null conditional part was already checked by a parent not-null condition.
TimothyMakkison · Oct 3, 2023 · c4eed62 · c4eed62
1 parent 1e55852
commit c4eed62
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 7 deletions.
diff --git a/...Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersContainerBuilderContext.cs b/...Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersContainerBuilderContext.cs
@@ -61,18 +61,24 @@ private MemberNullDelegateAssignmentMapping GetOrCreateNullDelegateMappingForPat
         IMemberAssignmentMappingContainer parentMapping = Mapping;
 
         // try to reuse parent path mappings and wrap inside them
+        // if the parentMapping is the first nullable path, no need to access the path in the condition in a null-safe way.
+        var needsNullSafeAccess = false;
         foreach (var nullablePath in nullConditionSourcePath.ObjectPathNullableSubPaths().Reverse())
         {
             if (_nullDelegateMappings.TryGetValue(new MemberPath(nullablePath), out var parentMappingHolder))
             {
                 parentMapping = parentMappingHolder;
+                break;
             }
+
+            needsNullSafeAccess = true;
         }
 
         mapping = new MemberNullDelegateAssignmentMapping(
             nullConditionSourcePath,
             parentMapping,
-            BuilderContext.MapperConfiguration.ThrowOnPropertyMappingNullMismatch
+            BuilderContext.MapperConfiguration.ThrowOnPropertyMappingNullMismatch,
+            needsNullSafeAccess
         );
         _nullDelegateMappings[nullConditionSourcePath] = mapping;
         parentMapping.AddMemberMappingContainer(mapping);

diff --git a/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberNullDelegateAssignmentMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberNullDelegateAssignmentMapping.cs
@@ -13,14 +13,17 @@ public class MemberNullDelegateAssignmentMapping : MemberAssignmentMappingContai
 {
     private readonly MemberPath _nullConditionalSourcePath;
     private readonly bool _throwInsteadOfConditionalNullMapping;
+    private readonly bool _needsNullSafeAccess;
 
     public MemberNullDelegateAssignmentMapping(
         MemberPath nullConditionalSourcePath,
         IMemberAssignmentMappingContainer parent,
-        bool throwInsteadOfConditionalNullMapping
+        bool throwInsteadOfConditionalNullMapping,
+        bool needsNullSafeAccess
     )
         : base(parent)
     {
+        _needsNullSafeAccess = needsNullSafeAccess;
         _nullConditionalSourcePath = nullConditionalSourcePath;
         _throwInsteadOfConditionalNullMapping = throwInsteadOfConditionalNullMapping;
     }
@@ -31,8 +34,8 @@ public override IEnumerable<StatementSyntax> Build(TypeMappingBuildContext ctx,
         //   target.Value = Map(Source.Name);
         // else
         //   throw ...
-        var sourceNullConditionalAccess = _nullConditionalSourcePath.BuildAccess(ctx.Source, true, true, true);
-        var nameofSourceAccess = _nullConditionalSourcePath.BuildAccess(ctx.Source, true, false, true);
+        var sourceNullConditionalAccess = _nullConditionalSourcePath.BuildAccess(ctx.Source, false, _needsNullSafeAccess, true);
+        var nameofSourceAccess = _nullConditionalSourcePath.BuildAccess(ctx.Source, false, false, true);
         var condition = IsNotNull(sourceNullConditionalAccess);
         var conditionCtx = ctx.AddIndentation();
         var trueClause = base.Build(conditionCtx, targetAccess);

diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyFlatteningTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyFlatteningTest.cs
@@ -510,7 +510,7 @@ public void ManualNestedPropertyNullablePath()
                 if (source.Value1 != null)
                 {
                     target.Value2 ??= new();
-                    if (source.Value1?.Value1 != null)
+                    if (source.Value1.Value1 != null)
                     {
                         target.Value2.Value2 ??= new();
                         target.Value2.Value2.Id2 = source.Value1.Value1.Id1;

diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyNullableTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyNullableTest.cs
@@ -106,7 +106,7 @@ TestSourceBuilderOptions.Default with
                 }
                 else
                 {
-                    throw new System.ArgumentNullException(nameof(source.Value.Value));
+                    throw new System.ArgumentNullException(nameof(source.Value));
                 }
                 return target;
                 """

diff --git a/...st.NullableIntWithAdditionalFlattenedValueToNonNullableIntProperties#Mapper.g.verified.cs b/...st.NullableIntWithAdditionalFlattenedValueToNonNullableIntProperties#Mapper.g.verified.cs
@@ -8,7 +8,7 @@ public partial class Mapper
         var target = new global::B();
         if (source.Nested != null)
         {
-            if (source.Nested?.Value2 != null)
+            if (source.Nested.Value2 != null)
             {
                 target.NestedValue2 = source.Nested.Value2.Value;
             }