handle nullable value types with MapValueAttribute and methods correct (

#1538)
riok · Oct 15, 2024 · 5333523 · 5333523
1 parent c2fc50a
commit 5333523
Show file tree

Hide file tree

Showing 3 changed files with 196 additions and 25 deletions.
diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/SourceValueBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/SourceValueBuilder.cs
@@ -181,10 +181,11 @@ private static bool ValidateValueProviderMethod(IMembersBuilderContext<IMapping>
             return false;
         }
 
-        // use non-nullable target type to allow non-null value type assignments
+        // use non-nullable to allow non-null value type assignments
         // to nullable value types
+        // nullable is checked with nullable annotation
         var methodCandidates = namedMethodCandidates.Where(x =>
-            SymbolEqualityComparer.Default.Equals(x.ReturnType, memberMappingInfo.TargetMember.MemberType.NonNullable())
+            SymbolEqualityComparer.Default.Equals(x.ReturnType.NonNullable(), memberMappingInfo.TargetMember.MemberType.NonNullable())
         );
 
         if (!memberMappingInfo.TargetMember.Member.IsNullable)

diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyValueMethodTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyValueMethodTest.cs
@@ -134,7 +134,7 @@ public void MethodReturnTypeMismatchShouldDiagnostic()
     }
 
     [Fact]
-    public void MethodReturnTypeNullMismatchShouldDiagnostic()
+    public void MethodReturnTypeNullableToNonNullableShouldDiagnostic()
     {
         var source = TestSourceBuilder.MapperWithBodyAndTypes(
             """
@@ -187,6 +187,31 @@ public void MethodReturnTypeNonNullableToNullable()
             );
     }
 
+    [Fact]
+    public void MethodReturnValueTypeNullableToNullable()
+    {
+        var source = TestSourceBuilder.MapperWithBodyAndTypes(
+            """
+            [MapValue("Value", Use = nameof(BuildC))] partial B Map(A source);
+            C? BuildC() => C.C1;
+            """,
+            "class A;",
+            "class B { public C? Value { get; set; } }",
+            "enum C { C1 }"
+        );
+
+        TestHelper
+            .GenerateMapper(source)
+            .Should()
+            .HaveSingleMethodBody(
+                """
+                var target = new global::B();
+                target.Value = BuildC();
+                return target;
+                """
+            );
+    }
+
     [Fact]
     public void MethodReturnValueTypeNonNullableToNullable()
     {
@@ -197,7 +222,7 @@ public void MethodReturnValueTypeNonNullableToNullable()
             """,
             "class A;",
             "class B { public C? Value { get; set; } }",
-            "enum C { C1 };"
+            "enum C { C1 }"
         );
 
         TestHelper
@@ -212,6 +237,36 @@ public void MethodReturnValueTypeNonNullableToNullable()
             );
     }
 
+    [Fact]
+    public void MethodReturnValueTypeNullableToNonNullableShouldDiagnostic()
+    {
+        var source = TestSourceBuilder.MapperWithBodyAndTypes(
+            """
+            [MapValue("Value", Use = nameof(BuildC))] partial B Map(A source);
+            System.Nullable<C> BuildC() => C.C1;
+            """,
+            "class A;",
+            "class B { public C Value { get; set; } }",
+            "enum C { C1 }"
+        );
+
+        TestHelper
+            .GenerateMapper(source, TestHelperOptions.AllowAndIncludeAllDiagnostics)
+            .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.MapValueMethodTypeMismatch,
+                "Cannot assign method return type C? of BuildC() to B.Value of type C"
+            )
+            .HaveDiagnostic(DiagnosticDescriptors.NoMemberMappings, "No members are mapped in the object mapping from A to B")
+            .HaveAssertedAllDiagnostics()
+            .HaveSingleMethodBody(
+                """
+                var target = new global::B();
+                return target;
+                """
+            );
+    }
+
     [Fact]
     public void MethodReturnTypeInDisabledNullableContext()
     {

diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyValueTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyValueTest.cs
@@ -595,13 +595,13 @@ public void EnumToProperty()
     }
 
     [Fact]
-    public void NamespacedEnumToProperty()
+    public void EnumToNullableProperty()
     {
         var source = TestSourceBuilder.MapperWithBodyAndTypes(
-            """[MapValue("EnumValue", MyNamespace.E1.Value2)] partial B Map(A source);""",
+            """[MapValue("EnumValue", E1.Value2)] partial B Map(A source);""",
             "class A;",
-            "namespace MyNamespace { enum E1 { Value1, Value2 } }",
-            "class B { public MyNamespace.E1 EnumValue { get; set; } }"
+            "enum E1 { Value1, Value2 }",
+            "class B { public E1? EnumValue { get; set; } }"
         );
 
         TestHelper
@@ -610,63 +610,148 @@ public void NamespacedEnumToProperty()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B();
-                target.EnumValue = global::MyNamespace.E1.Value2;
+                target.EnumValue = global::E1.Value2;
                 return target;
                 """
             );
     }
 
     [Fact]
-    public void EnumTypeMismatchShouldDiagnostic()
+    public void DefaultToEnumProperty()
     {
         var source = TestSourceBuilder.MapperWithBodyAndTypes(
-            """[MapValue("EnumValue", 1)] partial B Map(A source);""",
+            """[MapValue("EnumValue", default)] partial B Map(A source);""",
             "class A;",
             "enum E1 { Value1, Value2 }",
             "class B { public E1 EnumValue { get; set; } }"
         );
 
         TestHelper
-            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
+            .GenerateMapper(source)
+            .Should()
+            .HaveSingleMethodBody(
+                """
+                var target = new global::B();
+                target.EnumValue = default;
+                return target;
+                """
+            );
+    }
+
+    [Fact]
+    public void NullToEnumPropertyShouldDiagnostic()
+    {
+        var source = TestSourceBuilder.MapperWithBodyAndTypes(
+            """[MapValue("EnumValue", null)] partial B Map(A source);""",
+            "class A;",
+            "enum E1 { Value1, Value2 }",
+            "class B { public E1 EnumValue { get; set; } }"
+        );
+
+        TestHelper
+            .GenerateMapper(source, TestHelperOptions.AllowAndIncludeAllDiagnostics)
             .Should()
             .HaveDiagnostic(
-                DiagnosticDescriptors.MapValueTypeMismatch,
-                "Cannot assign constant value 1 of type int to B.EnumValue of type E1"
+                DiagnosticDescriptors.CannotMapValueNullToNonNullable,
+                "Cannot assign null to non-nullable member B.EnumValue of type E1"
             )
             .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B();
+                target.EnumValue = default;
                 return target;
                 """
             );
     }
 
     [Fact]
-    public void MapValueDuplicateForSameTargetMemberShouldDiagnostic()
+    public void NullToEnumNullableProperty()
     {
         var source = TestSourceBuilder.MapperWithBodyAndTypes(
-            """
-            [MapValue("Value", 10)]
-            [MapValue("Value", 20)]
-            partial B Map(A source);
-            """,
+            """[MapValue("EnumValue", null)] partial B Map(A source);""",
             "class A;",
-            "class B { public int Value { get; set; } }"
+            "enum E1 { Value1, Value2 }",
+            "class B { public E1? EnumValue { get; set; } }"
+        );
+
+        TestHelper
+            .GenerateMapper(source)
+            .Should()
+            .HaveSingleMethodBody(
+                """
+                var target = new global::B();
+                target.EnumValue = null;
+                return target;
+                """
+            );
+    }
+
+    [Fact]
+    public void DefaultToEnumNullableProperty()
+    {
+        var source = TestSourceBuilder.MapperWithBodyAndTypes(
+            """[MapValue("EnumValue", default)] partial B Map(A source);""",
+            "class A;",
+            "enum E1 { Value1, Value2 }",
+            "class B { public E1? EnumValue { get; set; } }"
+        );
+
+        TestHelper
+            .GenerateMapper(source)
+            .Should()
+            .HaveSingleMethodBody(
+                """
+                var target = new global::B();
+                target.EnumValue = default;
+                return target;
+                """
+            );
+    }
+
+    [Fact]
+    public void NamespacedEnumToProperty()
+    {
+        var source = TestSourceBuilder.MapperWithBodyAndTypes(
+            """[MapValue("EnumValue", MyNamespace.E1.Value2)] partial B Map(A source);""",
+            "class A;",
+            "namespace MyNamespace { enum E1 { Value1, Value2 } }",
+            "class B { public MyNamespace.E1 EnumValue { get; set; } }"
+        );
+
+        TestHelper
+            .GenerateMapper(source)
+            .Should()
+            .HaveSingleMethodBody(
+                """
+                var target = new global::B();
+                target.EnumValue = global::MyNamespace.E1.Value2;
+                return target;
+                """
+            );
+    }
+
+    [Fact]
+    public void EnumTypeMismatchShouldDiagnostic()
+    {
+        var source = TestSourceBuilder.MapperWithBodyAndTypes(
+            """[MapValue("EnumValue", 1)] partial B Map(A source);""",
+            "class A;",
+            "enum E1 { Value1, Value2 }",
+            "class B { public E1 EnumValue { get; set; } }"
         );
 
         TestHelper
             .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
             .HaveDiagnostic(
-                DiagnosticDescriptors.MultipleConfigurationsForTargetMember,
-                "Multiple mappings are configured for the same target member B.Value"
+                DiagnosticDescriptors.MapValueTypeMismatch,
+                "Cannot assign constant value 1 of type int to B.EnumValue of type E1"
             )
             .HaveAssertedAllDiagnostics()
-            .HaveMapMethodBody(
+            .HaveSingleMethodBody(
                 """
                 var target = new global::B();
-                target.Value = 10;
                 return target;
                 """
             );
@@ -696,6 +781,36 @@ public void EnumToNestedNullableProperty()
             );
     }
 
+    [Fact]
+    public void MapValueDuplicateForSameTargetMemberShouldDiagnostic()
+    {
+        var source = TestSourceBuilder.MapperWithBodyAndTypes(
+            """
+            [MapValue("Value", 10)]
+            [MapValue("Value", 20)]
+            partial B Map(A source);
+            """,
+            "class A;",
+            "class B { public int Value { get; set; } }"
+        );
+
+        TestHelper
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
+            .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.MultipleConfigurationsForTargetMember,
+                "Multiple mappings are configured for the same target member B.Value"
+            )
+            .HaveAssertedAllDiagnostics()
+            .HaveMapMethodBody(
+                """
+                var target = new global::B();
+                target.Value = 10;
+                return target;
+                """
+            );
+    }
+
     [Fact]
     public void MapValueAndPropertyAttributeForSameTargetShouldDiagnostic()
     {