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.

Sign up for GitHub

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

Jump to bottom

fix: correctly assign null if target is nullable instead of throwing #1191

Merged
merged 1 commit into from
Mar 25, 2024
Merged

fix: correctly assign null if target is nullable instead of throwing #1191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion ...Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersContainerBuilderContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ public void AddNullDelegateMemberAssignmentMapping(IMemberAssignmentMapping memb
{
container.AddNullMemberAssignment(SetterMemberPath.Build(BuilderContext, memberMapping.TargetPath));
}
else if (BuilderContext.Configuration.Mapper.ThrowOnPropertyMappingNullMismatch)
{
container.ThrowOnSourcePathNull();
}
}

private void AddMemberAssignmentMapping(IMemberAssignmentMappingContainer container, IMemberAssignmentMapping mapping)
Expand Down Expand Up @@ -95,7 +99,6 @@ private MemberNullDelegateAssignmentMapping GetOrCreateNullDelegateMappingForPat
mapping = new MemberNullDelegateAssignmentMapping(
GetterMemberPath.Build(BuilderContext, nullConditionSourcePath),
parentMapping,
BuilderContext.Configuration.Mapper.ThrowOnPropertyMappingNullMismatch,
needsNullSafeAccess
);
_nullDelegateMappings[nullConditionSourcePath] = mapping;
Expand Down
27 changes: 10 additions & 17 deletions src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberNullDelegateAssignmentMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@
public class MemberNullDelegateAssignmentMapping(
GetterMemberPath nullConditionalSourcePath,
IMemberAssignmentMappingContainer parent,
bool throwInsteadOfConditionalNullMapping,
bool needsNullSafeAccess
) : MemberAssignmentMappingContainer(parent)
{
private readonly GetterMemberPath _nullConditionalSourcePath = nullConditionalSourcePath;
private readonly bool _throwInsteadOfConditionalNullMapping = throwInsteadOfConditionalNullMapping;
private readonly List<SetterMemberPath> _targetsToSetNull = new();
private bool _throwOnSourcePathNull;

public void ThrowOnSourcePathNull()
{
_throwOnSourcePathNull = true;
}

public override IEnumerable<StatementSyntax> Build(TypeMappingBuildContext ctx, ExpressionSyntax targetAccess)
{
Expand Down Expand Up @@ -48,32 +52,21 @@
if (obj.GetType() != GetType())
return false;

return Equals((MemberNullDelegateAssignmentMapping)obj);
var other = (MemberNullDelegateAssignmentMapping)obj;
return _nullConditionalSourcePath.Equals(other._nullConditionalSourcePath);

Check warning on line 56 in src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberNullDelegateAssignmentMapping.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/MemberNullDelegateAssignmentMapping.cs#L55-L56 

Added lines #L55 - L56 were not covered by tests
}

public override int GetHashCode()
{
unchecked
{
return (_nullConditionalSourcePath.GetHashCode() * 397) ^ _throwInsteadOfConditionalNullMapping.GetHashCode();
}
}
public override int GetHashCode() => _nullConditionalSourcePath.GetHashCode();

public static bool operator ==(MemberNullDelegateAssignmentMapping? left, MemberNullDelegateAssignmentMapping? right) =>
Equals(left, right);

public static bool operator !=(MemberNullDelegateAssignmentMapping? left, MemberNullDelegateAssignmentMapping? right) =>
!Equals(left, right);

protected bool Equals(MemberNullDelegateAssignmentMapping other)
{
return _nullConditionalSourcePath.Equals(other._nullConditionalSourcePath)
&& _throwInsteadOfConditionalNullMapping == other._throwInsteadOfConditionalNullMapping;
}

private IEnumerable<StatementSyntax>? BuildElseClause(TypeMappingBuildContext ctx, ExpressionSyntax targetAccess)
{
if (_throwInsteadOfConditionalNullMapping)
if (_throwOnSourcePathNull)
{
// throw new ArgumentNullException
var nameofSourceAccess = _nullConditionalSourcePath.BuildAccess(ctx.Source, false, false, true);
Expand Down
6 changes: 3 additions & 3 deletions test/Riok.Mapperly.Tests/Mapping/EnumerableTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ public void ArrayOfPrimitiveTypesToNullablePrimitiveTypesArray()
[Fact]
public void ArrayCustomClassToArrayCustomClass()
{
var source = TestSourceBuilder.Mapping("B[]", "B[]", "class B { public int Value {get; set; }}");
var source = TestSourceBuilder.Mapping("B[]", "B[]", "class B { public int Value { get; set; } }");
TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return source;");
}

[Fact]
public void ArrayCustomClassNullableToArrayCustomClassNonNullable()
{
var source = TestSourceBuilder.Mapping("B?[]", "B[]", "class B { public int Value {get; set; }}");
var source = TestSourceBuilder.Mapping("B?[]", "B[]", "class B { public int Value { get; set; } }");
TestHelper
.GenerateMapper(source)
.Should()
Expand All @@ -89,7 +89,7 @@ public void ArrayCustomClassNullableToArrayCustomClassNonNullable()
[Fact]
public void ArrayCustomClassNonNullableToArrayCustomClassNullable()
{
var source = TestSourceBuilder.Mapping("B[]", "B?[]", "class B { public int Value {get; set; }}");
var source = TestSourceBuilder.Mapping("B[]", "B?[]", "class B { public int Value { get; set; } }");
TestHelper.GenerateMapper(source).Should().HaveSingleMethodBody("return (global::B?[])source;");
}

Expand Down
117 changes: 101 additions & 16 deletions test/Riok.Mapperly.Tests/Mapping/ObjectPropertyNullableTest.cs
View file
Open in desktop
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 ObjectPropertyNullableTest
Expand Down Expand Up @@ -145,8 +147,8 @@ public void NullableClassToNonNullableClassProperty()
"B",
"class A { public C? Value { get; set; } }",
"class B { public D Value { get; set; } }",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand Down Expand Up @@ -280,8 +282,8 @@ public void NonNullableClassToNullableClassProperty()
"B",
"class A { public C Value { get; set; } }",
"class B { public D? Value { get; set; } }",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand All @@ -304,8 +306,8 @@ public void NullableClassToNullableClassProperty()
"B",
"class A { public C? Value { get; set; } }",
"class B { public D? Value { get; set; } }",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand Down Expand Up @@ -339,8 +341,8 @@ TestSourceBuilderOptions.Default with
},
"class A { public C? Value { get; set; } }",
"class B { public D? Value { get; set; } }",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand Down Expand Up @@ -371,8 +373,8 @@ TestSourceBuilderOptions.Default with
},
"class A { public C? Value { get; set; } }",
"class B { public D? Value { get; set; } }",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand Down Expand Up @@ -402,8 +404,8 @@ public void DisabledNullableClassPropertyToNonNullableProperty()
"B",
"#nullable disable\n class A { public C Value { get; set; } }\n#nullable enable",
"class B { public D Value { get; set; } }",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand All @@ -429,8 +431,8 @@ public void NullableClassPropertyToDisabledNullableProperty()
"B",
"class A { public C? Value { get; set; } }",
"#nullable disable\n class B { public D Value { get; set; } }\n#nullable enable",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand Down Expand Up @@ -493,8 +495,43 @@ TestSourceBuilderOptions.Default with
},
"class A { public C? Value { get; set; } }",
"class B { public D Value { get; set; } }",
"class C { public string V {get; set; } }",
"class D { public string V {get; set; } }"
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
.GenerateMapper(source)
.Should()
.HaveMapMethodBody(
"""
var target = new global::B();
if (source.Value != null)
{
target.Value = MapToD(source.Value);
}
else
{
throw new System.ArgumentNullException(nameof(source.Value));
}
return target;
"""
);
}

[Fact]
public void NullableClassToNullableClassPropertyThrowShouldSetNull()
{
var source = TestSourceBuilder.Mapping(
"A",
"B",
TestSourceBuilderOptions.Default with
{
ThrowOnPropertyMappingNullMismatch = true
},
"class A { public C? Value { get; set; } }",
"class B { public D? Value { get; set; } }",
"class C { public string V { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
Expand All @@ -508,6 +545,54 @@ TestSourceBuilderOptions.Default with
target.Value = MapToD(source.Value);
}
else
{
target.Value = null;
}
return target;
"""
);
}

[Fact]
public void NullableClassToNullableClassFlattenedPropertyThrow()
{
// the flattened property is not nullable
// therefore if source.Value is null
// an exception should be thrown
// instead of assigning null to target.Value.
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"""
[MapProperty("Value", "Value")]
[MapProperty("Value.Flattened", "ValueFlattened")]
partial B Map(A source);
""",
TestSourceBuilderOptions.Default with
{
ThrowOnPropertyMappingNullMismatch = true
},
"class A { public C? Value { get; set; } }",
"class B { public D? Value { get; set; } public string ValueFlattened { get; set; } }",
"class C { public string V { get; set; } public string Flattened { get; set; } }",
"class D { public string V { get; set; } }"
);

TestHelper
.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics)
.Should()
.HaveDiagnostic(
DiagnosticDescriptors.SourceMemberNotMapped,
"The member Flattened on the mapping source type C is not mapped to any member on the mapping target type D"
)
.HaveAssertedAllDiagnostics()
.HaveMapMethodBody(
"""
var target = new global::B();
if (source.Value != null)
{
target.Value = MapToD(source.Value);
target.ValueFlattened = source.Value.Flattened;
}
else
{
throw new System.ArgumentNullException(nameof(source.Value));
}
Expand Down
4 changes: 2 additions & 2 deletions test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public void ClassToTuple()
var source = TestSourceBuilder.Mapping(
"A",
"(int B, string C)",
"public class A { public int B { get;set;} public int C {get;set;} }"
"public class A { public int B { get; set; } public int C { get; set; } }"
);

TestHelper
Expand Down Expand Up @@ -280,7 +280,7 @@ public void ClassToTupleWithIgnoredSource()
[MapperIgnoreSource("A")]
partial (int, int) Map(B source);
""",
"public class B { public int Item1 { get;set;} public int A {get;set;} public int Item2 {get;set;} }"
"public class B { public int Item1 { get;set;} public int A { get; set; } public int Item2 {get;set;} }"
);

TestHelper
Expand Down