-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
feat: add support for nested properties #587
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
namespace Riok.Mapperly.Abstractions; | ||
|
||
/// <summary> | ||
/// Maps all properties from a nested path to the root destination. | ||
/// </summary> | ||
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)] | ||
public class MapperNestedPropertiesAttribute : Attribute | ||
{ | ||
private const string PropertyAccessSeparatorStr = "."; | ||
private const char PropertyAccessSeparator = '.'; | ||
|
||
/// <summary> | ||
/// Maps a specified source property to the specified target property. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment isn't correct, is it? The same for the other ctor. |
||
/// </summary> | ||
/// <param name="source">The name of the source property. The use of `nameof()` is encouraged. A path can be specified by joining property names with a '.'.</param> | ||
public MapperNestedPropertiesAttribute(string source) | ||
: this(source.Split(PropertyAccessSeparator)) { } | ||
|
||
/// <summary> | ||
/// Maps a specified source property to the specified target property. | ||
/// </summary> | ||
/// <param name="source">The path of the source property. The use of `nameof()` is encouraged.</param> | ||
public MapperNestedPropertiesAttribute(string[] source) | ||
{ | ||
Source = source; | ||
} | ||
|
||
/// <summary> | ||
/// Gets the name of the source property. | ||
/// </summary> | ||
public IReadOnlyCollection<string> Source { get; } | ||
|
||
/// <summary> | ||
/// Gets the full name of the source property path. | ||
/// </summary> | ||
public string SourceFullName => string.Join(PropertyAccessSeparatorStr, Source); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
namespace Riok.Mapperly.Configuration; | ||
|
||
public record NestedPropertyMappingConfiguration(StringMemberPath Source); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rename to |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,8 +42,6 @@ protected MembersMappingBuilderContext(MappingBuilderContext builderContext, T m | |
|
||
_unmappedSourceMemberNames.ExceptWith(IgnoredSourceMemberNames); | ||
|
||
MemberConfigsByRootTargetName = GetMemberConfigurations(); | ||
|
||
// remove explicitly mapped ignored targets from ignoredTargetMemberNames | ||
// then remove all ignored targets from TargetMembers, leaving unignored and explicitly mapped ignored members | ||
ignoredTargetMemberNames.ExceptWith(MemberConfigsByRootTargetName.Keys); | ||
|
@@ -117,9 +115,25 @@ private Dictionary<string, IMappableMember> GetTargetMembers() | |
|
||
private Dictionary<string, List<PropertyMappingConfiguration>> GetMemberConfigurations() | ||
{ | ||
return BuilderContext.Configuration.Properties.ExplicitMappings | ||
.GroupBy(x => x.Target.Path.First()) | ||
.ToDictionary(x => x.Key, x => x.ToList()); | ||
var simpleMappings = BuilderContext.Configuration.Properties.ExplicitMappings.GroupBy(x => x.Target.Path.First()); | ||
|
||
var wildcardMappings = BuilderContext.Configuration.Properties.NestedMappings | ||
.SelectMany(x => | ||
{ | ||
return BuilderContext.SymbolAccessor | ||
.GetAllAccessibleMappableMembers(Mapping.SourceType) | ||
.Where(i => i.Name == x.Source.Path.Last()) | ||
.SelectMany(i => BuilderContext.SymbolAccessor.GetAllAccessibleMappableMembers(i.Type)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The members included via |
||
.Select(i => | ||
{ | ||
var list = x.Source.Path.ToList(); | ||
list.Add(i.Name); | ||
return new PropertyMappingConfiguration(new StringMemberPath(list), new StringMemberPath(new[] { i.Name })); | ||
}); | ||
Comment on lines
+123
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think mixing these with the manually configured mappings won't fully work... |
||
}) | ||
.GroupBy(x => x.Target.Path.Last()); | ||
|
||
return simpleMappings.Union(wildcardMappings).ToDictionary(x => x.Key, x => x.ToList()); | ||
} | ||
|
||
private void AddUnmatchedIgnoredTargetMembersDiagnostics() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -208,6 +208,19 @@ public Task WithPropertyNameMappingStrategyCaseSensitive() | |
return TestHelper.VerifyGenerator(source); | ||
} | ||
|
||
[Fact] | ||
public Task WithPropertyNameMappingStrategyCaseSensitiveAndManualMappedProperty() | ||
{ | ||
var source = TestSourceBuilder.MapperWithBodyAndTypes( | ||
"[MapProperty(nameof(A.stringvalue), nameof(B.StringValue2)] partial B Map(A source);", | ||
new TestSourceBuilderOptions { PropertyNameMappingStrategy = PropertyNameMappingStrategy.CaseSensitive }, | ||
"class A { public string stringvalue { get; set; } }", | ||
"class B { public string StringValue2 { get; set; } }" | ||
); | ||
|
||
return TestHelper.VerifyGenerator(source); | ||
} | ||
|
||
[Fact] | ||
public Task WithManualMappedNotFoundTargetPropertyShouldDiagnostic() | ||
{ | ||
|
@@ -443,4 +456,33 @@ public void ShouldIgnoreStaticConstructorAndDiagnostic() | |
.HaveDiagnostic(DiagnosticDescriptors.CouldNotCreateMapping) | ||
.HaveAssertedAllDiagnostics(); | ||
} | ||
|
||
[Fact] | ||
public void ShouldFlattedNestedToTargetRootMapping() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you move these tests to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is a complete new feature, an integration test should be added to ensure this works with all supported Roslyn versions. |
||
{ | ||
var mapperBody = """ | ||
[MapperNestedProperties(nameof(A.NestedValue)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you also add a test for deep nested properties. Eg. |
||
public partial C Map(A source); | ||
"""; | ||
|
||
var source = TestSourceBuilder.MapperWithBodyAndTypes( | ||
mapperBody, | ||
"class A { public B NestedValue { get; set; } public string DummyValue { get; set; } }", | ||
"class B { public string GoodProp1 { get; set; } public string GoodProp2 { get; set; } }", | ||
"class C { public string GoodProp1 { get; set; } public string GoodProp2 { get; set; } public string DummyValue { get; set; } }" | ||
); | ||
|
||
TestHelper | ||
.GenerateMapper(source) | ||
.Should() | ||
.HaveMapMethodBody( | ||
""" | ||
var target = new global::C(); | ||
target.GoodProp1 = source.NestedValue.GoodProp1; | ||
target.GoodProp2 = source.NestedValue.GoodProp2; | ||
target.DummyValue = source.DummyValue; | ||
return target; | ||
""" | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
//HintName: Mapper.g.cs | ||
// <auto-generated /> | ||
#nullable enable | ||
public partial class Mapper | ||
{ | ||
private partial global::B Map(global::A source) | ||
{ | ||
var target = new global::B(); | ||
target.StringValue2 = source.stringvalue; | ||
return target; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rename according to the issue and other mapping related attributes to
MapNestedPropertiesAttribute
.