feat: Diagnostic for mapping null mismatch

Adds diagnostics (RMG089 and RMG090) when a nullable source value is mapped to a non-nullable target value
riok · Nov 27, 2024 · 48b4334 · 48b4334
1 parent d3c4fcf
commit 48b4334
Show file tree

Hide file tree

Showing 32 changed files with 731 additions and 36 deletions.
diff --git a/src/Riok.Mapperly/AnalyzerReleases.Shipped.md b/src/Riok.Mapperly/AnalyzerReleases.Shipped.md
@@ -191,6 +191,8 @@ RMG085  | Mapper   | Error    | Invalid usage of fallback value
 RMG086  | Mapper   | Error    | The source of the explicit mapping from a string to an enum is not of type string
 RMG087  | Mapper   | Error    | The target of the explicit mapping from an enum to a string is not of type string
 RMG088  | Mapper   | Info     | The attribute to build the name of the enum member is missing
+RMG089  | Mapper   | Info     | Mapping nullable source to non-nullable target member
+RMG090  | Mapper   | Info     | Mapping nullable source type to non-nullable target type
 
 ### Removed Rules
 

diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/MemberMappingBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/MemberMappingBuilder.cs
@@ -73,6 +73,7 @@ public static bool TryBuild(
 
         var sourceMember = memberMappingInfo.SourceMember!;
         var targetMember = memberMappingInfo.TargetMember;
+
         if (delegateMapping == null)
         {
             ctx.BuilderContext.ReportDiagnostic(
@@ -84,6 +85,22 @@ public static bool TryBuild(
             return false;
         }
 
+        var memberTargetNullable = memberMappingInfo.TargetMember.MemberType.IsNullable();
+        var delegateTargetNullable = delegateMapping.TargetType.IsNullable();
+        var memberSourceNullable = memberMappingInfo.IsSourceNullable;
+        var delegateSourceNullable = delegateMapping.SourceType.IsNullable();
+
+        if (memberSourceNullable && !memberTargetNullable && !(delegateSourceNullable && !delegateTargetNullable))
+        {
+            ctx.BuilderContext.ReportDiagnostic(
+                DiagnosticDescriptors.NullableSourceValueToNonNullableTargetValue,
+                sourceMember.MemberPath.FullName,
+                sourceMember.MemberPath.RootType.ToDisplayString(),
+                targetMember.FullName,
+                targetMember.RootType.ToDisplayString()
+            );
+        }
+
         if (codeStyle == CodeStyle.Statement)
         {
             sourceValue = BuildBlockNullHandlingMapping(ctx, delegateMapping, sourceMember.MemberPath, targetMember);

diff --git a/src/Riok.Mapperly/Descriptors/MappingBuilders/NullableMappingBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBuilders/NullableMappingBuilder.cs
@@ -1,5 +1,6 @@
 using Riok.Mapperly.Descriptors.Mappings;
 using Riok.Mapperly.Descriptors.Mappings.ExistingTarget;
+using Riok.Mapperly.Diagnostics;
 using Riok.Mapperly.Helpers;
 
 namespace Riok.Mapperly.Descriptors.MappingBuilders;
@@ -12,6 +13,7 @@ public static class NullableMappingBuilder
             return null;
 
         var delegateMapping = ctx.BuildMapping(mappingKey, MappingBuildingOptions.KeepUserSymbol);
+
         return delegateMapping == null ? null : BuildNullDelegateMapping(ctx, delegateMapping);
     }
 
@@ -35,12 +37,19 @@ private static bool TryBuildNonNullableMappingKey(MappingBuilderContext ctx, out
         }
 
         mappingKey = new TypeMappingKey(sourceNonNullable ?? ctx.Source, targetNonNullable ?? ctx.Target);
+
+        if (sourceIsNullable && !targetIsNullable)
+        {
+            ctx.ReportDiagnostic(DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType, ctx.Source, ctx.Target);
+        }
+
         return true;
     }
 
     private static INewInstanceMapping BuildNullDelegateMapping(MappingBuilderContext ctx, INewInstanceMapping mapping)
     {
         var nullFallback = ctx.GetNullFallbackValue();
+
         return mapping switch
         {
             NewInstanceMethodMapping methodMapping => new NullDelegateMethodMapping(ctx.Source, ctx.Target, methodMapping, nullFallback),

diff --git a/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs b/src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs
@@ -827,6 +827,26 @@ public static class DiagnosticDescriptors
             true
         );
 
+    public static readonly DiagnosticDescriptor NullableSourceValueToNonNullableTargetValue =
+        new(
+            "RMG089",
+            "Mapping nullable source to non-nullable target member",
+            "Mapping the nullable source property {0} of {1} to the target property {2} of {3} which is not nullable",
+            DiagnosticCategories.Mapper,
+            DiagnosticSeverity.Info,
+            true
+        );
+
+    public static readonly DiagnosticDescriptor NullableSourceTypeToNonNullableTargetType =
+        new(
+            "RMG090",
+            "Mapping nullable source type to non-nullable target type",
+            "Mapping the nullable source of type {0} to target of type {1} which is not nullable",
+            DiagnosticCategories.Mapper,
+            DiagnosticSeverity.Info,
+            true
+        );
+
     private static string BuildHelpUri(string id)
     {
 #if ENV_NEXT

diff --git a/test/Riok.Mapperly.Tests/Mapping/DictionaryTest.cs b/test/Riok.Mapperly.Tests/Mapping/DictionaryTest.cs
@@ -1,4 +1,5 @@
 using Riok.Mapperly.Abstractions;
+using Riok.Mapperly.Diagnostics;
 
 namespace Riok.Mapperly.Tests.Mapping;
 
@@ -72,8 +73,13 @@ public void DictionaryToDictionaryNullableToNonNullable()
     {
         var source = TestSourceBuilder.Mapping("Dictionary<string, int?>", "Dictionary<string, int>");
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type int? to target of type int which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::System.Collections.Generic.Dictionary<string, int>(source.Count);
@@ -98,8 +104,13 @@ TestSourceBuilderOptions.Default with
             }
         );
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type int? to target of type int which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::System.Collections.Generic.Dictionary<string, int>(source.Count);

diff --git a/test/Riok.Mapperly.Tests/Mapping/EnumTest.cs b/test/Riok.Mapperly.Tests/Mapping/EnumTest.cs
@@ -229,8 +229,13 @@ public void NullableEnumToOtherEnumShouldCastWithNullHandling()
         var source = TestSourceBuilder.Mapping("E1?", "E2", "enum E1 {A, B, C}", "enum E2 {A, B, C}");
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type E1? to target of type E2 which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 "return source == null ? throw new System.ArgumentNullException(nameof(source)) : (global::E2)source.Value;"
             );

diff --git a/test/Riok.Mapperly.Tests/Mapping/EnumerableDeepCloningTest.cs b/test/Riok.Mapperly.Tests/Mapping/EnumerableDeepCloningTest.cs
@@ -1,3 +1,5 @@
+using Riok.Mapperly.Diagnostics;
+
 namespace Riok.Mapperly.Tests.Mapping;
 
 public class EnumerableDeepCloningTest
@@ -34,8 +36,13 @@ public void ArrayOfNullablePrimitiveTypesToNonNullableArrayDeepCloning()
     {
         var source = TestSourceBuilder.Mapping("int?[]", "int[]", TestSourceBuilderOptions.WithDeepCloning);
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type int? to target of type int which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new int[source.Length];

diff --git a/test/Riok.Mapperly.Tests/Mapping/EnumerableTest.cs b/test/Riok.Mapperly.Tests/Mapping/EnumerableTest.cs
@@ -17,8 +17,13 @@ public void NullableArrayToNonNullableArray()
     {
         var source = TestSourceBuilder.Mapping("int[]?", "int[]");
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type int[]? to target of type int[] which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody("return source ?? throw new System.ArgumentNullException(nameof(source));");
     }
 
@@ -27,8 +32,13 @@ public void ArrayOfNullablePrimitiveTypesToNonNullableArray()
     {
         var source = TestSourceBuilder.Mapping("int?[]", "int[]");
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type int? to target of type int which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new int[source.Length];
@@ -72,8 +82,13 @@ public void ArrayCustomClassNullableToArrayCustomClassNonNullable()
     {
         var source = TestSourceBuilder.Mapping("B?[]", "B[]", "class B { public int Value { get; set; } }");
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type B? to target of type B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B[source.Length];

diff --git a/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs b/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs
@@ -1,3 +1,5 @@
+using Riok.Mapperly.Diagnostics;
+
 namespace Riok.Mapperly.Tests.Mapping;
 
 public class NullableTest
@@ -8,8 +10,13 @@ public void NullableToNonNullableShouldThrow()
         var source = TestSourceBuilder.Mapping("A?", "B", "class A { }", "class B { }");
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type A? to target of type B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 if (source == null)
@@ -76,8 +83,13 @@ TestSourceBuilderOptions.Default with
         );
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type A? to target of type B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 if (source == null)
@@ -116,7 +128,15 @@ TestSourceBuilderOptions.Default with
             "class A { }"
         );
 
-        TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return source == null ? \"\" : source.ToString();");
+        TestHelper
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
+            .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type A? to target of type string which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
+            .HaveSingleMethodBody("return source == null ? \"\" : source.ToString();");
     }
 
     [Fact]
@@ -131,7 +151,15 @@ TestSourceBuilderOptions.Default with
             }
         );
 
-        TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return source == null ? default : source.Value;");
+        TestHelper
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
+            .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceTypeToNonNullableTargetType,
+                "Mapping the nullable source of type System.DateTime? to target of type System.DateTime which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
+            .HaveSingleMethodBody("return source == null ? default : source.Value;");
     }
 
     [Fact]

diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectNestedPropertyTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectNestedPropertyTest.cs
@@ -246,8 +246,13 @@ public void NullableNestedPropertyWithMemberNameOfSource()
         );
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceValueToNonNullableTargetValue,
+                "Mapping the nullable source property Value.Id of A to the target property Id of B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B();

diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyConstructorResolverTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyConstructorResolverTest.cs
@@ -1,3 +1,5 @@
+using Riok.Mapperly.Diagnostics;
+
 namespace Riok.Mapperly.Tests.Mapping;
 
 public class ObjectPropertyConstructorResolverTest
@@ -345,8 +347,13 @@ public void RecordToFlattenedRecordNullablePath()
         );
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceValueToNonNullableTargetValue,
+                "Mapping the nullable source property Nested.Value of A to the target property NestedValue of B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B(
@@ -394,8 +401,13 @@ TestSourceBuilderOptions.Default with
         );
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceValueToNonNullableTargetValue,
+                "Mapping the nullable source property Nested.Value of A to the target property NestedValue of B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B(source.Nested?.Value ?? "");
@@ -426,8 +438,13 @@ public void RecordToRecordNullableToNonNullablePrimitive()
         var source = TestSourceBuilder.Mapping("A", "B", "record A(int? Value);", "record B(int Value);");
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceValueToNonNullableTargetValue,
+                "Mapping the nullable source property Value of A to the target property Value of B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B(
@@ -444,8 +461,13 @@ public void RecordToRecordNonDirectAssignmentNullHandling()
         var source = TestSourceBuilder.Mapping("A", "B", "record A(int? Value);", "record B(double Value);");
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceValueToNonNullableTargetValue,
+                "Mapping the nullable source property Value of A to the target property Value of B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B(
@@ -462,8 +484,13 @@ public void RecordToRecordFlattenedNonDirectAssignmentNullHandling()
         var source = TestSourceBuilder.Mapping("A", "B", "record A(C? Nested);", "record B(double NestedValue);", "record C(int Value);");
 
         TestHelper
-            .GenerateMapper(source)
+            .GenerateMapper(source, TestHelperOptions.AllowDiagnostics)
             .Should()
+            .HaveDiagnostic(
+                DiagnosticDescriptors.NullableSourceValueToNonNullableTargetValue,
+                "Mapping the nullable source property Nested.Value of A to the target property NestedValue of B which is not nullable"
+            )
+            .HaveAssertedAllDiagnostics()
             .HaveSingleMethodBody(
                 """
                 var target = new global::B(