From 4fa4163a39a1acaa554a0df104e7ecc458986dc6 Mon Sep 17 00:00:00 2001 From: Ricky Kaare Engelharth Date: Tue, 12 Sep 2023 15:01:33 +0200 Subject: [PATCH] fix: add correct type for default null-fallback statements when using nullable value types --- .../MemberMappings/NullMemberMapping.cs | 8 +-- .../Mappings/NullDelegateMapping.cs | 2 +- .../Mappings/NullDelegateMethodMapping.cs | 2 +- .../Emit/Syntax/SyntaxFactoryHelper.Null.cs | 1 + ...jectionShouldTranslateToQuery.verified.sql | 7 +- ...ectionShouldTranslateToResult.verified.txt | 1 - ...SnapshotGeneratedSource_NET6_0.verified.cs | 2 +- ...SnapshotGeneratedSource_NET6_0.verified.cs | 2 +- test/Riok.Mapperly.Tests/Mapping/EnumTest.cs | 5 +- .../Mapping/NullableTest.cs | 2 +- .../ObjectPropertyConstructorResolverTest.cs | 64 +++++++++++++++++++ test/Riok.Mapperly.Tests/Mapping/ParseTest.cs | 2 +- ...houldIncludeNullables#Mapper.g.verified.cs | 2 +- 13 files changed, 80 insertions(+), 20 deletions(-) diff --git a/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/NullMemberMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/NullMemberMapping.cs index aba9b5ede1..ea9b6485fa 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/NullMemberMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/NullMemberMapping.cs @@ -60,7 +60,7 @@ public ExpressionSyntax Build(TypeMappingBuildContext ctx) var mapping = _delegateMapping.Build(ctx.WithSource(nullConditionalSourceAccess)); return _nullFallback == NullFallbackValue.Default && _targetType.IsNullable() ? mapping - : Coalesce(mapping, NullSubstitute(_delegateMapping.TargetType, nameofSourceAccess, _nullFallback)); + : Coalesce(mapping, NullSubstitute(_targetType, nameofSourceAccess, _nullFallback)); } var notNullCondition = _useNullConditionalAccess @@ -68,11 +68,7 @@ public ExpressionSyntax Build(TypeMappingBuildContext ctx) : SourcePath.BuildNonNullConditionWithoutConditionalAccess(ctx.Source)!; var sourceMemberAccess = SourcePath.BuildAccess(ctx.Source, true); ctx = ctx.WithSource(sourceMemberAccess); - return Conditional( - notNullCondition, - _delegateMapping.Build(ctx), - NullSubstitute(_delegateMapping.TargetType, sourceMemberAccess, _nullFallback) - ); + return Conditional(notNullCondition, _delegateMapping.Build(ctx), NullSubstitute(_targetType, sourceMemberAccess, _nullFallback)); } protected bool Equals(NullMemberMapping other) => diff --git a/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs index b2a0af2a9c..a6508aed2e 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMapping.cs @@ -69,7 +69,7 @@ public override ExpressionSyntax Build(TypeMappingBuildContext ctx) return Conditional( IsNull(ctx.Source), - NullSubstitute(TargetType.NonNullable(), ctx.Source, _nullFallbackValue), + NullSubstitute(TargetType, ctx.Source, _nullFallbackValue), _delegateMapping.Build(ctx.WithSource(sourceValue)) ); } diff --git a/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMethodMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMethodMapping.cs index 10c5e9d56c..3037c3484d 100644 --- a/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMethodMapping.cs +++ b/src/Riok.Mapperly/Descriptors/Mappings/NullDelegateMethodMapping.cs @@ -41,7 +41,7 @@ private IEnumerable AddPreNullHandling(TypeMappingBuildContext // call mapping only if source is not null. // if (source == null) // return ; - var fallbackExpression = NullSubstitute(TargetType.NonNullable(), ctx.Source, _nullFallbackValue); + var fallbackExpression = NullSubstitute(TargetType, ctx.Source, _nullFallbackValue); var ifExpression = ctx.SyntaxFactory.IfNullReturnOrThrow(ctx.Source, fallbackExpression); return body.Prepend(ifExpression); } diff --git a/src/Riok.Mapperly/Emit/Syntax/SyntaxFactoryHelper.Null.cs b/src/Riok.Mapperly/Emit/Syntax/SyntaxFactoryHelper.Null.cs index 847bc1fe5b..aa2d1b0020 100644 --- a/src/Riok.Mapperly/Emit/Syntax/SyntaxFactoryHelper.Null.cs +++ b/src/Riok.Mapperly/Emit/Syntax/SyntaxFactoryHelper.Null.cs @@ -33,6 +33,7 @@ public static ExpressionSyntax NullSubstitute(ITypeSymbol t, ExpressionSyntax ar { return nullFallbackValue switch { + NullFallbackValue.Default when t.IsNullableValueType() => DefaultExpression(FullyQualifiedIdentifier(t)), NullFallbackValue.Default => DefaultLiteral(), NullFallbackValue.EmptyString => StringLiteral(string.Empty), NullFallbackValue.CreateInstance => CreateInstance(t), diff --git a/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToQuery.verified.sql b/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToQuery.verified.sql index 67724ba251..14293c9c40 100644 --- a/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToQuery.verified.sql +++ b/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToQuery.verified.sql @@ -1,10 +1,7 @@ -SELECT "o"."CtorValue", "o"."IntValue", "o"."IntInitOnlyValue", "o"."RequiredValue", "o"."StringValue", "o"."RenamedStringValue", "i"."IdValue", CASE - WHEN "i0"."IdValue" IS NOT NULL THEN "i0"."IdValue" - ELSE 0 -END, CASE +SELECT "o"."CtorValue", "o"."IntValue", "o"."IntInitOnlyValue", "o"."RequiredValue", "o"."StringValue", "o"."RenamedStringValue", "i"."IdValue", "i0"."IdValue", CASE WHEN "t"."IntValue" IS NOT NULL THEN "t"."IntValue" ELSE 0 -END, "t"."IntValue" IS NOT NULL, "t"."IntValue", "t0"."IntValue" IS NOT NULL, "t0"."IntValue", COALESCE("o"."StringNullableTargetNotNullable", ''), "o0"."Id", "o0"."CtorValue", "o0"."DateTimeValueTargetDateOnly", "o0"."DateTimeValueTargetTimeOnly", "o0"."EnumName", "o0"."EnumRawValue", "o0"."EnumReverseStringValue", "o0"."EnumStringValue", "o0"."EnumValue", "o0"."FlatteningIdValue", "o0"."IgnoredIntValue", "o0"."IgnoredStringValue", "o0"."IntInitOnlyValue", "o0"."IntValue", "o0"."ManuallyMapped", "o0"."NestedNullableIntValue", "o0"."NestedNullableTargetNotNullableIntValue", "o0"."NullableFlatteningIdValue", "o0"."NullableUnflatteningIdValue", "o0"."RecursiveObjectId", "o0"."RenamedStringValue", "o0"."RequiredValue", "o0"."StringNullableTargetNotNullable", "o0"."StringValue", "o0"."SubObjectSubIntValue", "o0"."UnflatteningIdValue", "o"."Id", "i0"."IdValue", "i1"."SubIntValue", "t1"."IntValue", "t1"."TestObjectProjectionId", "t2"."IntValue", CAST("o"."EnumValue" AS INTEGER), CAST("o"."EnumName" AS INTEGER), CAST("o"."EnumRawValue" AS INTEGER), "o"."EnumStringValue", "o"."EnumReverseStringValue", "i1"."SubIntValue" IS NOT NULL, "i1"."BaseIntValue", "o"."DateTimeValueTargetDateOnly", "o"."DateTimeValueTargetTimeOnly", "o"."ManuallyMapped", "t3"."Id", "t3"."OtherValue", "t3"."TestObjectProjectionId", "t3"."Value" +END, "t"."IntValue" IS NOT NULL, "t"."IntValue", "t0"."IntValue" IS NOT NULL, "t0"."IntValue", COALESCE("o"."StringNullableTargetNotNullable", ''), "o0"."Id", "o0"."CtorValue", "o0"."DateTimeValueTargetDateOnly", "o0"."DateTimeValueTargetTimeOnly", "o0"."EnumName", "o0"."EnumRawValue", "o0"."EnumReverseStringValue", "o0"."EnumStringValue", "o0"."EnumValue", "o0"."FlatteningIdValue", "o0"."IgnoredIntValue", "o0"."IgnoredStringValue", "o0"."IntInitOnlyValue", "o0"."IntValue", "o0"."ManuallyMapped", "o0"."NestedNullableIntValue", "o0"."NestedNullableTargetNotNullableIntValue", "o0"."NullableFlatteningIdValue", "o0"."NullableUnflatteningIdValue", "o0"."RecursiveObjectId", "o0"."RenamedStringValue", "o0"."RequiredValue", "o0"."StringNullableTargetNotNullable", "o0"."StringValue", "o0"."SubObjectSubIntValue", "o0"."UnflatteningIdValue", "o"."Id", "i1"."SubIntValue", "t1"."IntValue", "t1"."TestObjectProjectionId", "t2"."IntValue", CAST("o"."EnumValue" AS INTEGER), CAST("o"."EnumName" AS INTEGER), CAST("o"."EnumRawValue" AS INTEGER), "o"."EnumStringValue", "o"."EnumReverseStringValue", "i1"."SubIntValue" IS NOT NULL, "i1"."BaseIntValue", "o"."DateTimeValueTargetDateOnly", "o"."DateTimeValueTargetTimeOnly", "o"."ManuallyMapped", "t3"."Id", "t3"."OtherValue", "t3"."TestObjectProjectionId", "t3"."Value" FROM "Objects" AS "o" INNER JOIN "IdObject" AS "i" ON "o"."FlatteningIdValue" = "i"."IdValue" LEFT JOIN "IdObject" AS "i0" ON "o"."NullableFlatteningIdValue" = "i0"."IdValue" diff --git a/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToResult.verified.txt b/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToResult.verified.txt index 0a44530068..bc349afdbf 100644 --- a/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToResult.verified.txt +++ b/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.ProjectionShouldTranslateToResult.verified.txt @@ -4,7 +4,6 @@ StringValue: , RenamedStringValue2: , FlatteningIdValue: 1, - NullableFlatteningIdValue: 0, NestedNullableTargetNotNullable: {}, StringNullableTargetNotNullable: , SourceTargetSameObjectType: { diff --git a/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs b/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs index 026ce3c4d8..89eb70cbbe 100644 --- a/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs +++ b/test/Riok.Mapperly.IntegrationTests/_snapshots/ProjectionMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs @@ -15,7 +15,7 @@ public static partial class ProjectionMapper StringValue = x.StringValue, RenamedStringValue2 = x.RenamedStringValue, FlatteningIdValue = x.Flattening.IdValue, - NullableFlatteningIdValue = x.NullableFlattening != null ? x.NullableFlattening.IdValue : default, + NullableFlatteningIdValue = x.NullableFlattening != null ? x.NullableFlattening.IdValue : default(int?), NestedNullableIntValue = x.NestedNullable != null ? x.NestedNullable.IntValue : default, NestedNullable = x.NestedNullable != null ? new global::Riok.Mapperly.IntegrationTests.Dto.TestObjectNestedDto() { diff --git a/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs b/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs index faf056f65a..3d77b56c3a 100644 --- a/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs +++ b/test/Riok.Mapperly.IntegrationTests/_snapshots/StaticMapperTest.SnapshotGeneratedSource_NET6_0.verified.cs @@ -11,7 +11,7 @@ public static partial int DirectInt(int value) public static partial int? DirectIntNullable(int? value) { - return value == null ? default : value.Value; + return value == null ? default(int?) : value.Value; } public static partial long ImplicitCastInt(int value) diff --git a/test/Riok.Mapperly.Tests/Mapping/EnumTest.cs b/test/Riok.Mapperly.Tests/Mapping/EnumTest.cs index ec39ee449a..43cb1795fa 100644 --- a/test/Riok.Mapperly.Tests/Mapping/EnumTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/EnumTest.cs @@ -213,7 +213,10 @@ public void NullableEnumToOtherNullableEnumShouldCast() { var source = TestSourceBuilder.Mapping("E1?", "E2?", "enum E1 {A, B, C}", "enum E2 {A, B, C}"); - TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return source == null ? default : (global::E2)source.Value;"); + TestHelper + .GenerateMapper(source) + .Should() + .HaveSingleMethodBody("return source == null ? default(global::E2? ) : (global::E2)source.Value;"); } [Fact] diff --git a/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs b/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs index 5369fb4bce..01e112c200 100644 --- a/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/NullableTest.cs @@ -44,7 +44,7 @@ public void NullablePrimitiveToOtherNullablePrimitiveShouldWork() { var source = TestSourceBuilder.Mapping("decimal?", "int?"); - TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody(@"return source == null ? default : (int)source.Value;"); + TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody(@"return source == null ? default(int? ) : (int)source.Value;"); } [Fact] diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyConstructorResolverTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyConstructorResolverTest.cs index d2968b198a..18d473f538 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyConstructorResolverTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyConstructorResolverTest.cs @@ -483,4 +483,68 @@ public void CanResolveToClassConstructorWithMapPropertyAttribute() """ ); } + + [Fact] + public void RecordToRecordNullableToNullableEnum() + { + var source = TestSourceBuilder.CSharp( + """ + using Riok.Mapperly.Abstractions; + + enum SourceEnum { Value1, Value2 } + enum TargetEnum { Value1, Value2 } + + record SourceRecord(SourceEnum? EnumValue); + record TargetRecord(TargetEnum? EnumValue); + + [Mapper(EnumMappingStrategy = EnumMappingStrategy.ByName)] + static partial class Mapper + { + public static partial TargetRecord Map(this SourceRecord record); + } + """ + ); + + TestHelper + .GenerateMapper(source) + .Should() + .HaveMapMethodBody( + """ + var target = new global::TargetRecord(record.EnumValue != null ? MapToTargetEnum(record.EnumValue.Value) : default(global::TargetEnum? )); + return target; + """ + ); + } + + [Fact] + public void RecordToRecordNullableToNullableStruct() + { + var source = TestSourceBuilder.CSharp( + """ + using Riok.Mapperly.Abstractions; + + struct SourceStruct { public int Value { get; set; } } + struct TargetStruct { public int Value { get; set; } } + + record SourceRecord(SourceStruct? StructValue); + record TargetRecord(TargetStruct? StructValue); + + [Mapper(EnumMappingStrategy = EnumMappingStrategy.ByName)] + static partial class Mapper + { + public static partial TargetRecord Map(this SourceRecord record); + } + """ + ); + + TestHelper + .GenerateMapper(source) + .Should() + .HaveMapMethodBody( + """ + var target = new global::TargetRecord(record.StructValue != null ? MapToTargetStruct(record.StructValue.Value) : default(global::TargetStruct? )); + return target; + """ + ); + } } diff --git a/test/Riok.Mapperly.Tests/Mapping/ParseTest.cs b/test/Riok.Mapperly.Tests/Mapping/ParseTest.cs index e16bd36df0..32dcca8048 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ParseTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ParseTest.cs @@ -23,7 +23,7 @@ public void ParseableBuiltInClass() public void ParseableBuiltNullableInClass() { var source = TestSourceBuilder.Mapping("string?", "int?"); - TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return source == null ? default : int.Parse(source);"); + TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return source == null ? default(int? ) : int.Parse(source);"); } [Fact] diff --git a/test/Riok.Mapperly.Tests/_snapshots/RuntimeTargetTypeMappingTest.WithNullableObjectSourceAndTargetTypeShouldIncludeNullables#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/RuntimeTargetTypeMappingTest.WithNullableObjectSourceAndTargetTypeShouldIncludeNullables#Mapper.g.verified.cs index 0a1aafa58a..20f81dc5ab 100644 --- a/test/Riok.Mapperly.Tests/_snapshots/RuntimeTargetTypeMappingTest.WithNullableObjectSourceAndTargetTypeShouldIncludeNullables#Mapper.g.verified.cs +++ b/test/Riok.Mapperly.Tests/_snapshots/RuntimeTargetTypeMappingTest.WithNullableObjectSourceAndTargetTypeShouldIncludeNullables#Mapper.g.verified.cs @@ -34,7 +34,7 @@ string x when targetType.IsAssignableFrom(typeof(int)) => MapStringToInt(x), private partial int? MapStringToInt(string? source) { - return source == null ? default : int.Parse(source); + return source == null ? default(int?) : int.Parse(source); } private partial int? MapIntToInt(int source)