From 44c277ff255a0ce63d8b3cbaddb96a210fc98402 Mon Sep 17 00:00:00 2001 From: clegoz <10047369+clegoz@users.noreply.github.com> Date: Thu, 20 Feb 2025 22:22:02 +0300 Subject: [PATCH] feat: add RMG072 and RMG073 diagnostics for named existing target mappings --- .../ObjectMemberMappingBodyBuilder.cs | 150 +++++++++++++++--- .../CompositeMemberExistingTargetMapping.cs | 34 ++++ .../ObjectPropertyUseNamedMappingTest.cs | 82 ++++++++++ ...thDifferentSourceType#Mapper.g.verified.cs | 19 +++ ...appingWithDifferentSourceType.verified.txt | 26 +++ ...thDifferentTargetType#Mapper.g.verified.cs | 19 +++ ...appingWithDifferentTargetType.verified.txt | 26 +++ 7 files changed, 338 insertions(+), 18 deletions(-) create mode 100644 src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/CompositeMemberExistingTargetMapping.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType.verified.txt create mode 100644 test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType#Mapper.g.verified.cs create mode 100644 test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType.verified.txt diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs index 62a4f16bca..ba2c6641f2 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs @@ -1,4 +1,5 @@ using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis; using Riok.Mapperly.Descriptors.MappingBodyBuilders.BuilderContext; using Riok.Mapperly.Descriptors.Mappings; using Riok.Mapperly.Descriptors.Mappings.ExistingTarget; @@ -187,6 +188,9 @@ MemberMappingInfo memberMappingInfo // even if a mapping validation fails ctx.ConsumeMemberConfigs(memberMappingInfo); + if (TryAddNamedExistingTargetMapping(ctx, memberMappingInfo)) + return; + if (TryAddExistingTargetMapping(ctx, memberMappingInfo)) return; @@ -206,40 +210,150 @@ MemberMappingInfo memberMappingInfo } } - private static bool TryAddExistingTargetMapping( + private static bool TryAddNamedExistingTargetMapping( IMembersContainerBuilderContext ctx, MemberMappingInfo memberMappingInfo ) { // can only map with an existing target from a source member - if (memberMappingInfo.SourceMember == null) + if (memberMappingInfo.SourceMember is null) return false; - var sourceMemberPath = memberMappingInfo.SourceMember; + if (memberMappingInfo.Configuration?.Use is null) + return false; + + var use = memberMappingInfo.Configuration.Use; + var existingTargetMapping = ctx.BuilderContext.FindExistingTargetNamedMapping(use); + if (existingTargetMapping is null) + return false; + + var sourceMemberPath = memberMappingInfo.SourceMember.MemberPath; var targetMemberPath = memberMappingInfo.TargetMember; - IExistingTargetMapping? existingTargetMapping; - if (memberMappingInfo.Configuration?.Use is not null) + var differentSourceType = !SymbolEqualityComparer.IncludeNullability.Equals( + sourceMemberPath.MemberType, + existingTargetMapping.SourceType + ); + var differentTargetType = !SymbolEqualityComparer.IncludeNullability.Equals( + targetMemberPath.MemberType, + existingTargetMapping.TargetType + ); + + if (!differentSourceType && !differentTargetType) { - existingTargetMapping = ctx.BuilderContext.FindExistingTargetNamedMapping(memberMappingInfo.Configuration.Use); + var sourceMemberGetter = sourceMemberPath.BuildGetter(ctx.BuilderContext); + var targetMemberGetter = targetMemberPath.BuildGetter(ctx.BuilderContext); + + var memberMapping = new MemberExistingTargetMapping( + existingTargetMapping, + sourceMemberGetter, + targetMemberGetter, + memberMappingInfo + ); + ctx.AddMemberAssignmentMapping(memberMapping); + return true; } - else + + if (differentSourceType) + { + ctx.BuilderContext.ReportDiagnostic( + DiagnosticDescriptors.ReferencedMappingSourceTypeMismatch, + use, + existingTargetMapping.SourceType.ToDisplayString(), + sourceMemberPath.MemberType.ToDisplayString() + ); + } + + if (differentTargetType) + { + ctx.BuilderContext.ReportDiagnostic( + DiagnosticDescriptors.ReferencedMappingTargetTypeMismatch, + use, + existingTargetMapping.TargetType.ToDisplayString(), + targetMemberPath.MemberType.ToDisplayString() + ); + } + + return TryAddCompositeExistingTargetMapping(ctx, existingTargetMapping, memberMappingInfo); + } + + private static bool TryAddCompositeExistingTargetMapping( + IMembersContainerBuilderContext ctx, + IExistingTargetMapping existingTargetMapping, + MemberMappingInfo memberMappingInfo + ) + { + if (memberMappingInfo.SourceMember is null) + return false; + + var sourceMemberPath = memberMappingInfo.SourceMember.MemberPath; + var targetMemberPath = memberMappingInfo.TargetMember; + + var sourceMemberGetter = sourceMemberPath.BuildGetter(ctx.BuilderContext); + var targetMemberGetter = targetMemberPath.BuildGetter(ctx.BuilderContext); + + var sourceMapping = ctx.BuilderContext.FindOrBuildMapping(sourceMemberPath.MemberType, existingTargetMapping.SourceType); + if (sourceMapping == null) + { + ctx.BuilderContext.ReportDiagnostic( + DiagnosticDescriptors.CouldNotCreateMapping, + sourceMemberPath.MemberType, + existingTargetMapping.SourceType + ); + + return true; + } + + var targetMapping = ctx.BuilderContext.FindOrBuildMapping(targetMemberPath.MemberType, existingTargetMapping.TargetType); + if (targetMapping == null) { - // if the member is readonly - // and the target and source path is readable, - // we try to create an existing target mapping - if ( - targetMemberPath.Member is { CanSet: true, IsInitOnly: false } - || !targetMemberPath.Path.All(op => op.CanGet) - || !sourceMemberPath.MemberPath.Path.All(op => op.CanGet) + ctx.BuilderContext.ReportDiagnostic( + DiagnosticDescriptors.CouldNotCreateMapping, + targetMemberPath.MemberType.ToDisplayString(), + existingTargetMapping.TargetType.ToDisplayString() + ); + + return true; + } + + ctx.AddMemberAssignmentMapping( + new CompositeMemberExistingTargetMapping( + existingTargetMapping, + sourceMapping, + sourceMemberGetter, + targetMapping, + targetMemberGetter, + memberMappingInfo ) - { - return false; - } + ); + return true; + } + + private static bool TryAddExistingTargetMapping( + IMembersContainerBuilderContext ctx, + MemberMappingInfo memberMappingInfo + ) + { + // can only map with an existing target from a source member + if (memberMappingInfo.SourceMember == null) + return false; + + var sourceMemberPath = memberMappingInfo.SourceMember; + var targetMemberPath = memberMappingInfo.TargetMember; - existingTargetMapping = ctx.BuilderContext.FindOrBuildExistingTargetMapping(memberMappingInfo.ToTypeMappingKey()); + // if the member is readonly + // and the target and source path is readable, + // we try to create an existing target mapping + if ( + targetMemberPath.Member is { CanSet: true, IsInitOnly: false } + || !targetMemberPath.Path.All(op => op.CanGet) + || !sourceMemberPath.MemberPath.Path.All(op => op.CanGet) + ) + { + return false; } + var existingTargetMapping = ctx.BuilderContext.FindOrBuildExistingTargetMapping(memberMappingInfo.ToTypeMappingKey()); if (existingTargetMapping == null) return false; diff --git a/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/CompositeMemberExistingTargetMapping.cs b/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/CompositeMemberExistingTargetMapping.cs new file mode 100644 index 0000000000..14978b635b --- /dev/null +++ b/src/Riok.Mapperly/Descriptors/Mappings/MemberMappings/CompositeMemberExistingTargetMapping.cs @@ -0,0 +1,34 @@ +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Riok.Mapperly.Descriptors.Mappings.ExistingTarget; +using Riok.Mapperly.Symbols.Members; + +namespace Riok.Mapperly.Descriptors.Mappings.MemberMappings; + +/// +/// Creates a composite mapping for existing target +/// +/// OutputMapping(SourceMapping(source), TargetMapping(target)); +/// +/// +public class CompositeMemberExistingTargetMapping( + IExistingTargetMapping delegateMapping, + INewInstanceMapping sourceMapping, + MemberPathGetter sourcePath, + INewInstanceMapping targetMapping, + MemberPathGetter targetPath, + MemberMappingInfo memberInfo +) : IMemberAssignmentMapping +{ + public MemberMappingInfo MemberInfo { get; } = memberInfo; + + public IEnumerable Build(TypeMappingBuildContext ctx, ExpressionSyntax targetAccess) + { + var sourcePathAccess = sourcePath.BuildAccess(ctx.Source); + var source = sourceMapping.Build(ctx.WithSource(sourcePathAccess)); + + var targetPathAccess = targetPath.BuildAccess(targetAccess); + var target = targetMapping.Build(ctx.WithSource(targetPathAccess)); + + return delegateMapping.Build(ctx.WithSource(source), target); + } +} diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyUseNamedMappingTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyUseNamedMappingTest.cs index b96af04b1d..1f09091231 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyUseNamedMappingTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyUseNamedMappingTest.cs @@ -474,6 +474,88 @@ public class B return TestHelper.VerifyGenerator(source); } + [Fact] + public Task UserImplementedExistingTargetMappingWithDifferentSourceType() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + [MapProperty(nameof(A.Value), nameof(B.Value), Use = "MapValues")] + public partial void Map(A source, B target); + private void MapValues(F source, D target) {} + """, + """ + public class A + { + public C Value { get; set; } + } + """, + """ + public class C + { + public List Values { get; set; } + } + """, + """ + public class B + { + public D Value { get; set; } + } + public class F + { + public List Values { get; set; } + } + """, + """ + public class D + { + public List Values { get; set; } + } + """ + ); + return TestHelper.VerifyGenerator(source); + } + + [Fact] + public Task UserImplementedExistingTargetMappingWithDifferentTargetType() + { + var source = TestSourceBuilder.MapperWithBodyAndTypes( + """ + [MapProperty(nameof(A.Value), nameof(B.Value), Use = "MapValues")] + public partial void Map(A source, B target); + private void MapValues(C source, F target) {} + """, + """ + public class A + { + public C Value { get; set; } + } + """, + """ + public class C + { + public List Values { get; set; } + } + """, + """ + public class B + { + public D Value { get; set; } + } + public class F + { + public List Values { get; set; } + } + """, + """ + public class D + { + public List Values { get; set; } + } + """ + ); + return TestHelper.VerifyGenerator(source); + } + [Fact] public Task ShouldUseReferencedMappingOnSelectedPropertiesWithExistingInstance() { diff --git a/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType#Mapper.g.verified.cs new file mode 100644 index 0000000000..774fd88ccb --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType#Mapper.g.verified.cs @@ -0,0 +1,19 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + public partial void Map(global::A source, global::B target) + { + MapValues(MapToF(source.Value), target.Value); + } + + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private global::F MapToF(global::C source) + { + var target = new global::F(); + target.Values = source.Values; + return target; + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType.verified.txt b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType.verified.txt new file mode 100644 index 0000000000..66800df3ec --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentSourceType.verified.txt @@ -0,0 +1,26 @@ +{ + Diagnostics: [ + { + Location: /* +{ + [MapProperty(nameof(A.Value), nameof(B.Value), Use = "MapValues")] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +public partial void Map(A source, B target); +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +private void MapValues(F source, D target) {} +*/ + : (11,4)-(12,44), + Message: The source type F of the referenced mapping MapValues does not match the expected type C, + Severity: Warning, + WarningLevel: 1, + Descriptor: { + Id: RMG072, + Title: The source type of the referenced mapping does not match, + MessageFormat: The source type {1} of the referenced mapping {0} does not match the expected type {2}, + Category: Mapper, + DefaultSeverity: Warning, + IsEnabledByDefault: true + } + } + ] +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType#Mapper.g.verified.cs new file mode 100644 index 0000000000..680c7a1c80 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType#Mapper.g.verified.cs @@ -0,0 +1,19 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + public partial void Map(global::A source, global::B target) + { + MapValues(source.Value, MapToF(target.Value)); + } + + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private global::F MapToF(global::D source) + { + var target = new global::F(); + target.Values = source.Values; + return target; + } +} \ No newline at end of file diff --git a/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType.verified.txt b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType.verified.txt new file mode 100644 index 0000000000..8429995fb3 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/ObjectPropertyUseNamedMappingTest.UserImplementedExistingTargetMappingWithDifferentTargetType.verified.txt @@ -0,0 +1,26 @@ +{ + Diagnostics: [ + { + Location: /* +{ + [MapProperty(nameof(A.Value), nameof(B.Value), Use = "MapValues")] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +public partial void Map(A source, B target); +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +private void MapValues(C source, F target) {} +*/ + : (11,4)-(12,44), + Message: The target type F of the referenced mapping MapValues does not match the expected type D, + Severity: Warning, + WarningLevel: 1, + Descriptor: { + Id: RMG073, + Title: The target type of the referenced mapping does not match, + MessageFormat: The target type {1} of the referenced mapping {0} does not match the expected type {2}, + Category: Mapper, + DefaultSeverity: Warning, + IsEnabledByDefault: true + } + } + ] +} \ No newline at end of file