From 450d7ca602a48ac39b6a2fb9567b6bdefcd4315e Mon Sep 17 00:00:00 2001 From: Lars Date: Fri, 16 Feb 2024 17:38:36 +0800 Subject: [PATCH] fix: don't diagnostic for constructor mappings when no instance can be created but none is needed anyway --- .../BuilderContext/IMembersBuilderContext.cs | 8 +++-- .../MembersMappingBuilderContext.cs | 19 +++++++++++ ...wInstanceObjectMemberMappingBodyBuilder.cs | 26 ++------------- .../NewValueTupleMappingBodyBuilder.cs | 11 +------ .../Mapping/ObjectPropertyNullableTest.cs | 32 +++++++++++++++++++ .../QueryableProjectionNullableTest.cs | 25 +++++++++++++++ ...orShouldNotDiagnostic#Mapper.g.verified.cs | 13 ++++++++ 7 files changed, 98 insertions(+), 36 deletions(-) create mode 100644 test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.ClassToRecordNoAccessibleSourceCtorShouldNotDiagnostic#Mapper.g.verified.cs diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs index 71d0743566..3c4f865617 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/IMembersBuilderContext.cs @@ -1,5 +1,7 @@ +using Microsoft.CodeAnalysis; using Riok.Mapperly.Configuration; using Riok.Mapperly.Descriptors.Mappings; +using Riok.Mapperly.Descriptors.Mappings.MemberMappings; using Riok.Mapperly.Symbols; namespace Riok.Mapperly.Descriptors.MappingBodyBuilders.BuilderContext; @@ -13,8 +15,6 @@ public interface IMembersBuilderContext { T Mapping { get; } - void AddDiagnostics(); - MappingBuilderContext BuilderContext { get; } IReadOnlyCollection IgnoredSourceMemberNames { get; } @@ -22,4 +22,8 @@ public interface IMembersBuilderContext Dictionary TargetMembers { get; } Dictionary> MemberConfigsByRootTargetName { get; } + + void AddDiagnostics(); + + NullMemberMapping BuildNullMemberMapping(MemberPath sourcePath, INewInstanceMapping delegateMapping, ITypeSymbol targetMemberType); } diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs index fad95d8426..199cb0c1f1 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/BuilderContext/MembersMappingBuilderContext.cs @@ -1,6 +1,8 @@ +using Microsoft.CodeAnalysis; using Riok.Mapperly.Abstractions; using Riok.Mapperly.Configuration; using Riok.Mapperly.Descriptors.Mappings; +using Riok.Mapperly.Descriptors.Mappings.MemberMappings; using Riok.Mapperly.Diagnostics; using Riok.Mapperly.Helpers; using Riok.Mapperly.Symbols; @@ -72,6 +74,23 @@ protected MembersMappingBuilderContext(MappingBuilderContext builderContext, T m public Dictionary> MemberConfigsByRootTargetName { get; } + public NullMemberMapping BuildNullMemberMapping( + MemberPath sourcePath, + INewInstanceMapping delegateMapping, + ITypeSymbol targetMemberType + ) + { + var getterSourcePath = GetterMemberPath.Build(BuilderContext, sourcePath); + + var nullFallback = NullFallbackValue.Default; + if (!delegateMapping.SourceType.IsNullable() && sourcePath.IsAnyNullable()) + { + nullFallback = BuilderContext.GetNullFallbackValue(targetMemberType); + } + + return new NullMemberMapping(delegateMapping, getterSourcePath, targetMemberType, nullFallback, !BuilderContext.IsExpression); + } + public void AddDiagnostics() { AddUnmatchedIgnoredTargetMembersDiagnostics(); diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewInstanceObjectMemberMappingBodyBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewInstanceObjectMemberMappingBodyBuilder.cs index 54e11ca8a0..782c464e73 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewInstanceObjectMemberMappingBodyBuilder.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewInstanceObjectMemberMappingBodyBuilder.cs @@ -173,22 +173,8 @@ private static void BuildInitMemberMapping( return; } - var nullFallback = NullFallbackValue.Default; - if (!delegateMapping.SourceType.IsNullable() && sourcePath.IsAnyNullable()) - { - nullFallback = ctx.BuilderContext.GetNullFallbackValue(targetMember.Type); - } - - var getterSourcePath = GetterMemberPath.Build(ctx.BuilderContext, sourcePath); var setterTargetPath = SetterMemberPath.Build(ctx.BuilderContext, targetPath); - - var memberMapping = new NullMemberMapping( - delegateMapping, - getterSourcePath, - targetMember.Type, - nullFallback, - !ctx.BuilderContext.IsExpression - ); + var memberMapping = ctx.BuildNullMemberMapping(sourcePath, delegateMapping, targetMember.Type); var memberAssignmentMapping = new MemberAssignmentMapping(setterTargetPath, memberMapping); ctx.AddInitMemberMapping(memberAssignmentMapping); } @@ -298,15 +284,7 @@ private static bool TryBuildConstructorMapping( return false; } - var getterSourcePath = GetterMemberPath.Build(ctx.BuilderContext, sourcePath); - - var memberMapping = new NullMemberMapping( - delegateMapping, - getterSourcePath, - parameterType, - ctx.BuilderContext.GetNullFallbackValue(parameterType), - !ctx.BuilderContext.IsExpression - ); + var memberMapping = ctx.BuildNullMemberMapping(sourcePath, delegateMapping, parameterType); var ctorMapping = new ConstructorParameterMapping(parameter, memberMapping, skippedOptionalParam); constructorParameterMappings.Add(ctorMapping); mappedTargetMemberNames.Add(parameter.Name); diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs index 1a28052c3e..b602960851 100644 --- a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs +++ b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs @@ -113,16 +113,7 @@ out HashSet mappedTargetMemberNames return false; } - var getterSourcePath = GetterMemberPath.Build(ctx.BuilderContext, sourcePath); - - var memberMapping = new NullMemberMapping( - delegateMapping, - getterSourcePath, - targetMemberType, - ctx.BuilderContext.GetNullFallbackValue(targetMemberType), - !ctx.BuilderContext.IsExpression - ); - + var memberMapping = ctx.BuildNullMemberMapping(sourcePath, delegateMapping, targetMemberType); var ctorMapping = new ValueTupleConstructorParameterMapping(targetMember, memberMapping); constructorParameterMappings.Add(ctorMapping); mappedTargetMemberNames.Add(targetMember.Name); diff --git a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyNullableTest.cs b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyNullableTest.cs index 47c71e143b..c7c9c44c3b 100644 --- a/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyNullableTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/ObjectPropertyNullableTest.cs @@ -748,4 +748,36 @@ public Task NullableToNullablePropertyWithAnotherNullableToNonNullableMappingSho return TestHelper.VerifyGenerator(source); } + + [Fact] + public void ClassToRecordNoAccessibleSourceCtorShouldNotDiagnostic() + { + var source = TestSourceBuilder.Mapping( + "A", + "B", + """ + public class A + { + private A(C value) + { + Value = value; + } + + public C Value { get; } + } + """, + "public record B(C Value);", + "public record C(string StringValue);" + ); + + TestHelper + .GenerateMapper(source) + .Should() + .HaveMapMethodBody( + """ + var target = new global::B(source.Value); + return target; + """ + ); + } } diff --git a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionNullableTest.cs b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionNullableTest.cs index 31cb78210d..1f9437d385 100644 --- a/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionNullableTest.cs +++ b/test/Riok.Mapperly.Tests/Mapping/QueryableProjectionNullableTest.cs @@ -143,4 +143,29 @@ public Task ClassToClassNullableSourcePathManuallyFlatten() return TestHelper.VerifyGenerator(source); } + + [Fact] + public Task ClassToRecordNoAccessibleSourceCtorShouldNotDiagnostic() + { + // see https://github.com/riok/mapperly/issues/972 + var source = TestSourceBuilder.Mapping( + "System.Linq.IQueryable", + "System.Linq.IQueryable", + """ + public class A + { + private A(C value) + { + Value = value; + } + + public C Value { get; } + } + """, + "public record B(C Value);", + "public record C(string StringValue);" + ); + + return TestHelper.VerifyGenerator(source); + } } diff --git a/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.ClassToRecordNoAccessibleSourceCtorShouldNotDiagnostic#Mapper.g.verified.cs b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.ClassToRecordNoAccessibleSourceCtorShouldNotDiagnostic#Mapper.g.verified.cs new file mode 100644 index 0000000000..0b6e689bc4 --- /dev/null +++ b/test/Riok.Mapperly.Tests/_snapshots/QueryableProjectionNullableTest.ClassToRecordNoAccessibleSourceCtorShouldNotDiagnostic#Mapper.g.verified.cs @@ -0,0 +1,13 @@ +//HintName: Mapper.g.cs +// +#nullable enable +public partial class Mapper +{ + [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")] + private partial global::System.Linq.IQueryable Map(global::System.Linq.IQueryable source) + { +#nullable disable + return System.Linq.Queryable.Select(source, x => new global::B(x.Value)); +#nullable enable + } +} \ No newline at end of file