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 · 94d2d93 · 94d2d93
1 parent 96fa2c0
commit 94d2d93
Show file tree

Hide file tree

Showing 95 changed files with 2,942 additions and 1,153 deletions.
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -1,7 +1,7 @@
 <Project>
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
     <DebugSymbols>true</DebugSymbols>
     <DebugType>embedded</DebugType>
     <ArtifactsPath>$(MSBuildThisFileDirectory)artifacts</ArtifactsPath>

diff --git a/benchmarks/Riok.Mapperly.Benchmarks/Riok.Mapperly.Benchmarks.csproj b/benchmarks/Riok.Mapperly.Benchmarks/Riok.Mapperly.Benchmarks.csproj
@@ -18,7 +18,8 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
     </PackageReference>
     <PackageReference Include="Microsoft.Build.Locator" Version="1.7.8" />
-    <PackageReference Include="NuGet.Frameworks" Version="6.11.0" />
+    <PackageReference Include="NuGet.Frameworks" Version="6.12.1" />
+    <PackageReference Include="System.Formats.Asn1" Version="9.0.0" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/docs/docs/configuration/generated-source.mdx b/docs/docs/configuration/generated-source.mdx
@@ -35,7 +35,7 @@ dotnet build /p:EmitCompilerGeneratedFiles=true
 </Tabs>
 
 By default the files are written to `{BaseIntermediateOutpath}/generated/{Assembly}/Riok.Mapperly/{GeneratedFile}`.
-With `BaseIntermediateOutpath` for example being `obj/Debug/net8.0`.
+With `BaseIntermediateOutpath` for example being `obj/Debug/net9.0`.
 By convention, generated C# source code files will have the file extension `.g.cs`.
 
 The output path can be customized via the `CompilerGeneratedFilesOutputPath` property.

diff --git a/docs/package-lock.json b/docs/package-lock.json
diff --git a/docs/package.json b/docs/package.json
@@ -18,9 +18,9 @@
     "lint": "prettier --ignore-path .gitignore --check . && tsc && eslint --ignore-path .gitignore src && stylelint 'src/**/*.css'"
   },
   "dependencies": {
-    "@docusaurus/core": "3.6.0",
-    "@docusaurus/plugin-ideal-image": "3.6.0",
-    "@docusaurus/preset-classic": "3.6.0",
+    "@docusaurus/core": "3.6.1",
+    "@docusaurus/plugin-ideal-image": "3.6.1",
+    "@docusaurus/preset-classic": "3.6.1",
     "@easyops-cn/docusaurus-search-local": "0.45.0",
     "@mdx-js/react": "3.0.1",
     "clsx": "2.1.0",
@@ -32,9 +32,9 @@
     "path-to-regexp": "8.2.0"
   },
   "devDependencies": {
-    "@docusaurus/eslint-plugin": "3.6.0",
-    "@docusaurus/module-type-aliases": "3.6.0",
-    "@docusaurus/tsconfig": "3.6.0",
+    "@docusaurus/eslint-plugin": "3.6.1",
+    "@docusaurus/module-type-aliases": "3.6.1",
+    "@docusaurus/tsconfig": "3.6.1",
     "@types/react": "18.2.79",
     "@typescript-eslint/eslint-plugin": "7.16.1",
     "hast-util-to-html": "9.0.2",

diff --git a/global.json b/global.json
@@ -1,6 +1,6 @@
 {
   "sdk": {
-    "version": "8.0.401",
+    "version": "9.0.100",
     "rollForward": "latestMinor"
   }
 }
diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -16,11 +16,11 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Meziantou.Analyzer" Version="2.0.176">
+    <PackageReference Include="Meziantou.Analyzer" Version="2.0.180">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Meziantou.Polyfill" Version="1.0.40">
+    <PackageReference Include="Meziantou.Polyfill" Version="1.0.42">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
     </PackageReference>

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/Directory.Build.props b/test/Directory.Build.props
@@ -11,10 +11,10 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.10.0"/>
-    <PackageReference Include="FluentAssertions" Version="6.12.0"/>
-    <PackageReference Include="xunit" Version="2.8.0"/>
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.0">
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1"/>
+    <PackageReference Include="FluentAssertions" Version="6.12.2"/>
+    <PackageReference Include="xunit" Version="2.9.2"/>
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>
@@ -26,11 +26,11 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>
-    <PackageReference Include="Meziantou.Xunit.ParallelTestFramework" Version="2.2.0"/>
-    <PackageReference Include="Verify.XUnit" Version="24.2.0" />
-    <PackageReference Include="Verify.DiffPlex" Version="3.0.0" />
-    <PackageReference Include="JunitXml.TestLogger" Version="4.0.254" />
-    <PackageReference Include="NSubstitute" Version="5.1.0" />
+    <PackageReference Include="Meziantou.Xunit.ParallelTestFramework" Version="2.3.0"/>
+    <PackageReference Include="Verify.XUnit" Version="28.3.2" />
+    <PackageReference Include="Verify.DiffPlex" Version="3.1.2" />
+    <PackageReference Include="JunitXml.TestLogger" Version="4.1.0" />
+    <PackageReference Include="NSubstitute" Version="5.3.0" />
     <PackageReference Include="NSubstitute.Analyzers.CSharp" Version="1.0.17">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>

diff --git a/test/Riok.Mapperly.IntegrationTests/Riok.Mapperly.IntegrationTests.csproj b/test/Riok.Mapperly.IntegrationTests/Riok.Mapperly.IntegrationTests.csproj
@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <!-- setting the TargetFramework directly from the cli via -p:TargetFramework=netA.B does not work using a custom msbuild property seems to work -->
-    <MapperlyIntegrationTestsTargetFramework Condition="'$(MapperlyIntegrationTestsTargetFramework)' == ''">net8.0</MapperlyIntegrationTestsTargetFramework>
+    <MapperlyIntegrationTestsTargetFramework Condition="'$(MapperlyIntegrationTestsTargetFramework)' == ''">net9.0</MapperlyIntegrationTestsTargetFramework>
     <TargetFramework>$(MapperlyIntegrationTestsTargetFramework)</TargetFramework>
 
     <EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
@@ -36,18 +36,18 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="7.0.19" Condition="'$(TargetFramework)' == 'net7.0'" />
     <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="8.0.8" Condition="'$(TargetFramework)' == 'net8.0'" />
-    <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="9.0.0-rc.2.24474.1" Condition="'$(TargetFramework)' == 'net9.0'" />
+    <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="9.0.0" Condition="'$(TargetFramework)' == 'net9.0'" />
   </ItemGroup>
 
-  <!-- diffplex is only compatible with net7.0 up to 2.x-->
+  <!-- diffplex v3 is only compatible with net7.0 up to 2.x-->
   <ItemGroup Condition="'$(TargetFramework)' == 'net7.0' OR '$(TargetFramework)' == 'net6.0'">
     <PackageReference Update="Verify.DiffPlex" Version="2.2.1" />
   </ItemGroup>
 
   <!-- cannot use source generated polyfills since they require language version 11 -->
   <ItemGroup Condition="'$(TargetFramework)' == 'net48'">
-    <PackageReference Include="System.Collections.Immutable" Version="8.0.0" />
-    <PackageReference Include="Portable.System.DateTimeOnly" Version="8.0.0" />
+    <PackageReference Include="System.Collections.Immutable" Version="9.0.0" />
+    <PackageReference Include="Portable.System.DateTimeOnly" Version="8.0.2" />
     <PackageReference Include="IsExternalInit" Version="1.0.3">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>

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];