Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diagnostic for mapping null mismatch #1612

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Riok.Mapperly/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public static bool TryBuild(

var sourceMember = memberMappingInfo.SourceMember!;
var targetMember = memberMappingInfo.TargetMember;

if (delegateMapping == null)
{
ctx.BuilderContext.ReportDiagnostic(
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -12,6 +13,7 @@ public static class NullableMappingBuilder
return null;

var delegateMapping = ctx.BuildMapping(mappingKey, MappingBuildingOptions.KeepUserSymbol);

return delegateMapping == null ? null : BuildNullDelegateMapping(ctx, delegateMapping);
}

Expand All @@ -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),
Expand Down
20 changes: 20 additions & 0 deletions src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 13 additions & 2 deletions test/Riok.Mapperly.Tests/Mapping/DictionaryTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Diagnostics;

namespace Riok.Mapperly.Tests.Mapping;

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion test/Riok.Mapperly.Tests/Mapping/EnumTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;"
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Riok.Mapperly.Diagnostics;

namespace Riok.Mapperly.Tests.Mapping;

public class EnumerableDeepCloningTest
Expand Down Expand Up @@ -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];
Expand Down
21 changes: 18 additions & 3 deletions test/Riok.Mapperly.Tests/Mapping/EnumerableTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));");
}

Expand All @@ -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];
Expand Down Expand Up @@ -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];
Expand Down
36 changes: 32 additions & 4 deletions test/Riok.Mapperly.Tests/Mapping/NullableTest.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Riok.Mapperly.Diagnostics;

namespace Riok.Mapperly.Tests.Mapping;

public class NullableTest
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand Down
7 changes: 6 additions & 1 deletion test/Riok.Mapperly.Tests/Mapping/ObjectNestedPropertyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Riok.Mapperly.Diagnostics;

namespace Riok.Mapperly.Tests.Mapping;

public class ObjectPropertyConstructorResolverTest
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 ?? "");
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
Loading