Skip to content

Commit

Permalink
feat: remove unneeded null conditional initializer
Browse files Browse the repository at this point in the history
  • Loading branch information
TimothyMakkison committed Sep 14, 2023
1 parent b54b9a5 commit 757dcd0
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ private void AddNullMemberInitializers(IMemberAssignmentMappingContainer contain
if (!nullablePath.Member.CanSet)
continue;

// if member is initialised with a non null value then skip
if (
container.TryGetMemberMapping(nullablePath) is MemberAssignmentMapping
{
Mapping: MemberMapping { NullConditionalAccess: false }
}
)
{
continue;
}

if (!BuilderContext.SymbolAccessor.HasAccessibleParameterlessConstructor(type))
{
BuilderContext.ReportDiagnostic(DiagnosticDescriptors.NoParameterlessConstructorFound, type);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Descriptors.Mappings.MemberMappings;

Expand All @@ -7,6 +8,8 @@ namespace Riok.Mapperly.Descriptors.Mappings.MemberMappings;
/// </summary>
public interface IMemberAssignmentMappingContainer
{
IMemberAssignmentMapping? TryGetMemberMapping(MemberPath sourceMemberPath);

bool HasMemberMapping(IMemberAssignmentMapping mapping);

void AddMemberMapping(IMemberAssignmentMapping mapping);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ namespace Riok.Mapperly.Descriptors.Mappings.MemberMappings;
[DebuggerDisplay("MemberAssignmentMapping({SourcePath.FullName} => {TargetPath.FullName})")]
public class MemberAssignmentMapping : IMemberAssignmentMapping
{
private readonly IMemberMapping _mapping;

public MemberAssignmentMapping(MemberPath targetPath, IMemberMapping mapping)
{
TargetPath = targetPath;
_mapping = mapping;
Mapping = mapping;
}

public MemberPath SourcePath => _mapping.SourcePath;
public MemberPath SourcePath => Mapping.SourcePath;

public IMemberMapping Mapping { get; }

public MemberPath TargetPath { get; }

Expand All @@ -30,7 +30,7 @@ public IEnumerable<StatementSyntax> Build(TypeMappingBuildContext ctx, Expressio
public ExpressionSyntax BuildExpression(TypeMappingBuildContext ctx, ExpressionSyntax? targetAccess)
{
var targetMemberAccess = TargetPath.BuildAccess(targetAccess);
var mappedValue = _mapping.Build(ctx);
var mappedValue = Mapping.Build(ctx);

// target.Member = mappedValue;
return Assignment(targetMemberAccess, mappedValue);
Expand All @@ -54,7 +54,7 @@ public override int GetHashCode()
{
unchecked
{
var hashCode = _mapping.GetHashCode();
var hashCode = Mapping.GetHashCode();
hashCode = (hashCode * 397) ^ SourcePath.GetHashCode();
hashCode = (hashCode * 397) ^ TargetPath.GetHashCode();
return hashCode;
Expand All @@ -67,6 +67,6 @@ public override int GetHashCode()

protected bool Equals(MemberAssignmentMapping other)
{
return _mapping.Equals(other._mapping) && SourcePath.Equals(other.SourcePath) && TargetPath.Equals(other.TargetPath);
return Mapping.Equals(other.Mapping) && SourcePath.Equals(other.SourcePath) && TargetPath.Equals(other.TargetPath);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Descriptors.Mappings.MemberMappings;

Expand Down Expand Up @@ -42,6 +43,9 @@ public void AddMemberMapping(IMemberAssignmentMapping mapping)
}
}

public IMemberAssignmentMapping? TryGetMemberMapping(MemberPath sourceMemberPath) =>
_delegateMappings.FirstOrDefault(x => x.TargetPath == sourceMemberPath) ?? _parent?.TryGetMemberMapping(sourceMemberPath);

public bool HasMemberMapping(IMemberAssignmentMapping mapping) =>
_delegateMappings.Contains(mapping) || _parent?.HasMemberMapping(mapping) == true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace Riok.Mapperly.Descriptors.Mappings.MemberMappings;
public class MemberMapping : IMemberMapping
{
private readonly INewInstanceMapping _delegateMapping;
private readonly bool _nullConditionalAccess;
private readonly bool _addValuePropertyOnNullable;

public MemberMapping(
Expand All @@ -22,15 +21,17 @@ bool addValuePropertyOnNullable
{
_delegateMapping = delegateMapping;
SourcePath = sourcePath;
_nullConditionalAccess = nullConditionalAccess;
NullConditionalAccess = nullConditionalAccess;
_addValuePropertyOnNullable = addValuePropertyOnNullable;
}

public bool NullConditionalAccess { get; }

public MemberPath SourcePath { get; }

public ExpressionSyntax Build(TypeMappingBuildContext ctx)
{
ctx = ctx.WithSource(SourcePath.BuildAccess(ctx.Source, _addValuePropertyOnNullable, _nullConditionalAccess));
ctx = ctx.WithSource(SourcePath.BuildAccess(ctx.Source, _addValuePropertyOnNullable, NullConditionalAccess));
return _delegateMapping.Build(ctx);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Descriptors.Mappings.ExistingTarget;
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Descriptors.Mappings;

Expand All @@ -19,6 +20,8 @@ protected ObjectMemberMethodMapping(ITypeSymbol sourceType, ITypeSymbol targetTy
_mapping = new ObjectMemberExistingTargetMapping(sourceType, targetType);
}

public IMemberAssignmentMapping? TryGetMemberMapping(MemberPath sourceMemberPath) => _mapping.TryGetMemberMapping(sourceMemberPath);

public bool HasMemberMapping(IMemberAssignmentMapping mapping) => _mapping.HasMemberMapping(mapping);

public void AddMemberMapping(IMemberAssignmentMapping mapping) => _mapping.AddMemberMapping(mapping);
Expand Down
75 changes: 75 additions & 0 deletions test/Riok.Mapperly.Tests/Mapping/ObjectPropertyFlatteningTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,81 @@ public void ManualUnflattenedPropertyReadOnlyNullablePath()
);
}

[Fact]
public void ManualUnflattenedPropertyNullablePathShouldNotNullInitialize()
{
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"[MapProperty($\"MyValueId\", \"Value.Id\"), MapProperty($\"MyValueId2\", \"Value.Id2\"), MapProperty($\"Value\", \"Value\")] partial B Map(A source);",
"class A { public string MyValueId { get; set; } public string MyValueId2 { get; set; } public C Value { get; set; } }",
"class B { public C? Value { get; set; } }",
"class C { public string Id { get; set; } public string Id2 { get; set; } }"
);

TestHelper
.GenerateMapper(source)
.Should()
.HaveSingleMethodBody(
"""
var target = new global::B();
target.Value = source.Value;
target.Value.Id = source.MyValueId;
target.Value.Id2 = source.MyValueId2;
return target;
"""
);
}

[Fact]
public void ManualUnflattenedPropertyNullablePathShouldNullInitialize()
{
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"[MapProperty($\"MyValueId\", \"Value.Id\"), MapProperty($\"MyValueId2\", \"Value.Id2\"), MapProperty($\"Value\", \"Value\")] partial B Map(A source);",
"class A { public string MyValueId { get; set; } public string MyValueId2 { get; set; } public C? Value { get; set; } }",
"class B { public C? Value { get; set; } }",
"class C { public string Id { get; set; } public string Id2 { get; set; } }"
);

TestHelper
.GenerateMapper(source)
.Should()
.HaveSingleMethodBody(
"""
var target = new global::B();
target.Value ??= new();
target.Value = source.Value;
target.Value.Id = source.MyValueId;
target.Value.Id2 = source.MyValueId2;
return target;
"""
);
}

[Fact]
public void ManualUnflattenedPropertyDeepNullablePathShouldNotNullInitialize()
{
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"[MapProperty($\"MyValueId\", \"My.Value.Id\"), MapProperty($\"MyValueId2\", \"My.Value.Id2\"), MapProperty($\"My\", \"My\"), MapProperty($\"Value\", \"My.Value\")] partial B Map(A source);",
"class A { public string MyValueId { get; set; } public string MyValueId2 { get; set; } public C My { get; set; } public D Value { get; set; } }",
"class B { public C? My { get; set; } }",
"class C { public D? Value { get; set; } }",
"class D { public string Id { get; set; } public string Id2 { get; set; } }"
);

TestHelper
.GenerateMapper(source)
.Should()
.HaveSingleMethodBody(
"""
var target = new global::B();
target.My = source.My;
target.My.Value = source.Value;
target.My.Value.Id = source.MyValueId;
target.My.Value.Id2 = source.MyValueId2;
return target;
"""
);
}

[Fact]
public void ManualUnflattenedPropertyDeepNullablePath()
{
Expand Down

0 comments on commit 757dcd0

Please sign in to comment.