diff --git a/src/Riok.Mapperly/Configuration/IMemberPathConfiguration.cs b/src/Riok.Mapperly/Configuration/IMemberPathConfiguration.cs index e50ad72a9d..6ae0ead982 100644 --- a/src/Riok.Mapperly/Configuration/IMemberPathConfiguration.cs +++ b/src/Riok.Mapperly/Configuration/IMemberPathConfiguration.cs @@ -15,6 +15,11 @@ public interface IMemberPathConfiguration /// string FullName { get; } + /// + /// The member names of the path / each segment, e.g. [A,B,C] + /// + IEnumerable MemberNames { get; } + /// /// The number of path segments in this path. /// diff --git a/src/Riok.Mapperly/Configuration/StringMemberPath.cs b/src/Riok.Mapperly/Configuration/StringMemberPath.cs index 41d7f7a319..65d365512d 100644 --- a/src/Riok.Mapperly/Configuration/StringMemberPath.cs +++ b/src/Riok.Mapperly/Configuration/StringMemberPath.cs @@ -12,6 +12,7 @@ public StringMemberPath(IEnumerable path) public string RootName => Path[0]; public string FullName => string.Join(MemberPathConstants.MemberAccessSeparatorString, Path); public int PathCount => Path.Count; + public IEnumerable MemberNames => Path; public override string ToString() => FullName; diff --git a/src/Riok.Mapperly/Configuration/SymbolMemberPath.cs b/src/Riok.Mapperly/Configuration/SymbolMemberPath.cs index 0cc7518744..446473b205 100644 --- a/src/Riok.Mapperly/Configuration/SymbolMemberPath.cs +++ b/src/Riok.Mapperly/Configuration/SymbolMemberPath.cs @@ -12,8 +12,9 @@ public record SymbolMemberPath(ImmutableEquatableArray Path) : IMemberP private string? _fullName; public string RootName => Path[0].Name; - public string FullName => _fullName ??= string.Join(MemberPathConstants.MemberAccessSeparatorString, Path.Select(x => x.Name)); + public string FullName => _fullName ??= string.Join(MemberPathConstants.MemberAccessSeparatorString, MemberNames); public int PathCount => Path.Count; + public IEnumerable MemberNames => Path.Select(x => x.Name); public override string ToString() => FullName; diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs index e3695e689e..7e7f168dfa 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs @@ -23,6 +23,7 @@ public interface IMembersBuilderContext void SetTargetMemberMapped(IMappableMember targetMember); void ConsumeMemberConfigs(MemberMappingInfo members); + bool HasDuplicatedMemberConfig(NonEmptyMemberPath targetMemberPath); void TryAddSourceMemberAlias(string alias, IMappableMember member); diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs index 1d650e6f73..d233734f6f 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs @@ -60,6 +60,8 @@ public void ConsumeMemberConfigs(MemberMappingInfo members) } } + public bool HasDuplicatedMemberConfig(NonEmptyMemberPath targetMemberPath) => _state.HasDuplicatedMemberConfig(targetMemberPath); + public void MappingAdded() => _state.MappingAdded(); protected void MappingAdded(MemberMappingInfo info, bool ignoreTargetCasing = false) => _state.MappingAdded(info, ignoreTargetCasing); diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingState.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingState.cs index 32c9ba9f6b..065083856a 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingState.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingState.cs @@ -27,6 +27,7 @@ internal class MembersMappingState( Dictionary targetMembers, Dictionary> memberValueConfigsByRootTargetName, Dictionary> memberConfigsByRootTargetName, + Dictionary> configuredTargetMembersByRootName, HashSet ignoredSourceMemberNames ) { @@ -105,6 +106,14 @@ public void IgnoreMembers(string name) } } + public bool HasDuplicatedMemberConfig(NonEmptyMemberPath targetMemberPath) + { + if (!configuredTargetMembersByRootName.TryGetValue(targetMemberPath.RootName, out var configs)) + return false; + + return configs.Where(targetMemberPath.Equals).Skip(1).Any(); + } + public void SetTargetMemberMapped(IMappableMember targetMember) => SetTargetMemberMapped(targetMember.Name); public void SetTargetMemberMapped(string targetName, bool ignoreCase = false) diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingStateBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingStateBuilder.cs index 0618dbd9bb..ead24a70e4 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingStateBuilder.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingStateBuilder.cs @@ -12,9 +12,23 @@ internal static class MembersMappingStateBuilder public static MembersMappingState Build(MappingBuilderContext ctx, IMapping mapping) { // build configurations - var configuredTargetMembers = new HashSet(); - var memberValueConfigsByRootTargetName = BuildMemberValueConfigurations(ctx, mapping, configuredTargetMembers); - var memberConfigsByRootTargetName = BuildMemberConfigurations(ctx, mapping, configuredTargetMembers); + // duplicated configurations are filtered inside the MapValue configurations + // if a MapProperty configuration references a target member which is already configured + // via MapValue it is also filtered & reported + var valueMappingConfiguredTargetPaths = new HashSet(); + var configuredTargetMembersByRootName = new ListDictionary(); + var memberValueConfigsByRootTargetName = BuildMemberValueConfigurations( + ctx, + mapping, + valueMappingConfiguredTargetPaths, + configuredTargetMembersByRootName + ); + var memberConfigsByRootTargetName = BuildMemberConfigurations( + ctx, + mapping, + valueMappingConfiguredTargetPaths, + configuredTargetMembersByRootName + ); // build all members var unmappedSourceMemberNames = GetSourceMemberNames(ctx, mapping); @@ -47,6 +61,7 @@ public static MembersMappingState Build(MappingBuilderContext ctx, IMapping mapp targetMembers, memberValueConfigsByRootTargetName, memberConfigsByRootTargetName, + configuredTargetMembersByRootName.AsDictionary(), ignoredSourceMemberNames ); } @@ -76,55 +91,58 @@ private static Dictionary GetTargetMembers(MappingBuild private static Dictionary> BuildMemberValueConfigurations( MappingBuilderContext ctx, IMapping mapping, - HashSet configuredTargetMembers + HashSet configuredTargetPaths, + ListDictionary configuredTargetMembersByRootName ) { - return GetUniqueTargetConfigurations(ctx, mapping, configuredTargetMembers, ctx.Configuration.Members.ValueMappings, x => x.Target) - .GroupBy(x => x.Target.RootName) - .ToDictionary(x => x.Key, x => x.ToList()); + var byTargetRootName = new ListDictionary(ctx.Configuration.Members.ValueMappings.Count); + foreach (var config in ctx.Configuration.Members.ValueMappings) + { + configuredTargetMembersByRootName.Add(config.Target.RootName, config.Target); + if (configuredTargetPaths.Add(config.Target.FullName)) + { + byTargetRootName.Add(config.Target.RootName, config); + continue; + } + + ctx.ReportDiagnostic( + DiagnosticDescriptors.MultipleConfigurationsForTargetMember, + mapping.TargetType.ToDisplayString(), + config.Target.FullName + ); + } + + return byTargetRootName.AsDictionary(); } private static Dictionary> BuildMemberConfigurations( MappingBuilderContext ctx, IMapping mapping, - HashSet configuredTargetMembers + HashSet valueConfiguredTargetPaths, + ListDictionary configuredTargetMembersByRootName ) { // order by target path count as objects with less path depth should be mapped first // to prevent NREs in the generated code - return GetUniqueTargetConfigurations( - ctx, - mapping, - configuredTargetMembers, - ctx.Configuration.Members.ExplicitMappings, - x => x.Target - ) - .GroupBy(x => x.Target.RootName) - .ToDictionary(x => x.Key, x => x.OrderBy(cfg => cfg.Target.PathCount).ToList()); - } - - private static IEnumerable GetUniqueTargetConfigurations( - MappingBuilderContext ctx, - IMapping mapping, - HashSet configuredTargetMembers, - IEnumerable configs, - Func targetPathSelector - ) - { - foreach (var config in configs) + var result = new ListDictionary(); + foreach (var config in ctx.Configuration.Members.ExplicitMappings.OrderBy(cfg => cfg.Target.PathCount)) { - var targetPath = targetPathSelector(config); - if (configuredTargetMembers.Add(targetPath)) + // if MapValue is already configured for this target member + // no additional MapProperty is allowed => diagnostic duplicate config. + if (valueConfiguredTargetPaths.Contains(config.Target.FullName)) { - yield return config; + ctx.ReportDiagnostic( + DiagnosticDescriptors.MultipleConfigurationsForTargetMember, + mapping.TargetType.ToDisplayString(), + config.Target.FullName + ); continue; } - ctx.ReportDiagnostic( - DiagnosticDescriptors.MultipleConfigurationsForTargetMember, - mapping.TargetType.ToDisplayString(), - targetPath.FullName - ); + configuredTargetMembersByRootName.Add(config.Target.RootName, config.Target); + result.Add(config.Target.RootName, config); } + + return result.AsDictionary(); } } diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs index 97ffb355fa..695d19b6ca 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs @@ -107,11 +107,22 @@ public static bool ValidateMappingSpecification( var noInitOnlyPath = allowInitOnlyMember ? targetMemberPath.ObjectPath : targetMemberPath.Path; if (noInitOnlyPath.Any(p => p.IsInitOnly)) { - ctx.BuilderContext.ReportDiagnostic( - DiagnosticDescriptors.CannotMapToInitOnlyMemberPath, - memberInfo.DescribeSource(), - targetMemberPath.ToDisplayString(includeMemberType: false) - ); + if (ctx.HasDuplicatedMemberConfig(targetMemberPath)) + { + ctx.BuilderContext.ReportDiagnostic( + DiagnosticDescriptors.MultipleConfigurationsForTargetMember, + ctx.Mapping.TargetType.ToDisplayString(), + targetMemberPath.ToDisplayString(includeMemberType: false, includeRootType: false) + ); + } + else + { + ctx.BuilderContext.ReportDiagnostic( + DiagnosticDescriptors.CannotMapToInitOnlyMemberPath, + memberInfo.DescribeSource(), + targetMemberPath.ToDisplayString(includeMemberType: false) + ); + } return false; } diff --git a/src/Riok.Mapperly/Helpers/ListDictionary.cs b/src/Riok.Mapperly/Helpers/ListDictionary.cs index 20225915fa..a92b8939cc 100644 --- a/src/Riok.Mapperly/Helpers/ListDictionary.cs +++ b/src/Riok.Mapperly/Helpers/ListDictionary.cs @@ -9,8 +9,20 @@ namespace Riok.Mapperly.Helpers; public class ListDictionary where TKey : notnull { - private readonly IReadOnlyList _empty = []; - private readonly Dictionary> _data = new(); + private static readonly IReadOnlyList _empty = []; + private readonly Dictionary> _data; + + public ListDictionary() + { + _data = new Dictionary>(); + } + + public ListDictionary(int capacity) + { + _data = new Dictionary>(capacity); + } + + public Dictionary> AsDictionary() => _data; public IReadOnlyList GetOrEmpty(TKey key) => _data.GetValueOrDefault(key) ?? _empty; diff --git a/src/Riok.Mapperly/Symbols/Members/MemberPath.cs b/src/Riok.Mapperly/Symbols/Members/MemberPath.cs index 5fd1b09c09..5f8662bd3b 100644 --- a/src/Riok.Mapperly/Symbols/Members/MemberPath.cs +++ b/src/Riok.Mapperly/Symbols/Members/MemberPath.cs @@ -1,5 +1,6 @@ using System.Diagnostics; using Microsoft.CodeAnalysis; +using Riok.Mapperly.Configuration; using Riok.Mapperly.Descriptors; using Riok.Mapperly.Helpers; @@ -79,6 +80,20 @@ public static MemberPath Create(ITypeSymbol rootType, IReadOnlyList p /// public override IMappableMember Member => Path[^1]; + /// + /// Gets the name of the first path segment. + /// + public string RootName => Path[0].Name; + /// /// Gets the type of the . If any part of the path is nullable, this type will be nullable too. /// diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyTest.cs index 71aff93af7..0599979b21 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyTest.cs @@ -177,30 +177,36 @@ public void WithManualMappedProperty() } [Fact] - public void WithManualMappedPropertyDuplicated() + public void WithManualMappedPropertyDuplicatedAndNullFilter() { var source = TestSourceBuilder.MapperWithBodyAndTypes( """ - [MapProperty(nameof(A.StringValue), nameof(B.StringValue2)] - [MapProperty(nameof(A.StringValue), nameof(B.StringValue2)] + [MapProperty(nameof(A.StringValue1), nameof(B.StringValue)] + [MapProperty(nameof(A.StringValue2), nameof(B.StringValue)] partial B Map(A source); """, - "class A { public string StringValue { get; set; } }", - "class B { public string StringValue2 { get; set; } }" + TestSourceBuilderOptions.Default with + { + AllowNullPropertyAssignment = false, + }, + "class A { public string? StringValue1 { get; set; } public string? StringValue2 { get; set; } }", + "class B { public string StringValue { get; set; } }" ); TestHelper - .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) + .GenerateMapper(source) .Should() - .HaveDiagnostic( - DiagnosticDescriptors.MultipleConfigurationsForTargetMember, - "Multiple mappings are configured for the same target member B.StringValue2" - ) - .HaveAssertedAllDiagnostics() .HaveSingleMethodBody( """ var target = new global::B(); - target.StringValue2 = source.StringValue; + if (source.StringValue1 != null) + { + target.StringValue = source.StringValue1; + } + if (source.StringValue2 != null) + { + target.StringValue = source.StringValue2; + } return target; """ ); diff --git a/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs b/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs index 4de2354337..da5e476d40 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ValueTupleTest.cs @@ -624,31 +624,6 @@ public void QueryableTupleToIQueryableValueTuple() ); } - [Fact] - public void TupleToTupleWithManyMapPropertyShouldDiagnostic() - { - var source = TestSourceBuilder.MapperWithBodyAndTypes( - """ - [MapProperty("B", "A")] - [MapProperty("Item1", "A")] - partial (int A, int) Map((int, string B) source); - """ - ); - - TestHelper - .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) - .Should() - .HaveDiagnostic( - DiagnosticDescriptors.SourceMemberNotMapped, - "The member Item1 on the mapping source type (int, string B) is not mapped to any member on the mapping target type (int A, int)" - ) - .HaveDiagnostic( - DiagnosticDescriptors.MultipleConfigurationsForTargetMember, - "Multiple mappings are configured for the same target member (int A, int).A" - ) - .HaveAssertedAllDiagnostics(); - } - [Fact] public void TupleToTupleWithMapPropertyWithImplicitNameShouldDiagnostic() {