Skip to content

Commit

Permalink
fix: do not emit null coalesce operator in queryable mappings
Browse files Browse the repository at this point in the history
  • Loading branch information
latonz committed Feb 11, 2025
1 parent bddc413 commit a63c8b3
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 13 deletions.
10 changes: 4 additions & 6 deletions src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ namespace Riok.Mapperly.Descriptors;
public class MappingBuilderContext : SimpleMappingBuilderContext
{
private readonly FormatProviderCollection _formatProviders;
private CollectionInfos? _collectionInfos;
private DictionaryInfos? _dictionaryInfos;

public MappingBuilderContext(
SimpleMappingBuilderContext parentCtx,
Expand Down Expand Up @@ -57,9 +55,9 @@ bool ignoreDerivedTypes

public ITypeSymbol Target => MappingKey.Target;

public CollectionInfos? CollectionInfos => _collectionInfos ??= CollectionInfoBuilder.Build(Types, SymbolAccessor, Source, Target);
public CollectionInfos? CollectionInfos => field ??= CollectionInfoBuilder.Build(Types, SymbolAccessor, Source, Target);

public DictionaryInfos? DictionaryInfos => _dictionaryInfos ??= DictionaryInfoBuilder.Build(Types, CollectionInfos);
public DictionaryInfos? DictionaryInfos => field ??= DictionaryInfoBuilder.Build(Types, CollectionInfos);

public IUserMapping? UserMapping { get; }

Expand Down Expand Up @@ -162,7 +160,7 @@ bool ignoreDerivedTypes
/// <summary>
/// Finds or builds a mapping (<seealso cref="FindOrBuildMapping(Riok.Mapperly.Descriptors.TypeMappingKey,Riok.Mapperly.Descriptors.MappingBuildingOptions,Location)"/>).
/// Before a new mapping is built existing mappings are tried to be found by the following priorities:
/// 1. exact match
/// 1. user mapping with exact type match
/// 2. ignoring the nullability of the source and the target (needs to be handled by the caller of this method)
/// If no mapping can be found a new mapping is built with the source and the target as non-nullables.
/// </summary>
Expand All @@ -176,7 +174,7 @@ bool ignoreDerivedTypes
Location? diagnosticLocation = null
)
{
if (FindMapping(key) is { } mapping)
if (FindMapping(key) is INewInstanceUserMapping mapping)
return mapping;

// if a user mapping is referenced
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,17 @@ bool useNullConditionalAccess

public ExpressionSyntax Build(TypeMappingBuildContext ctx)
{
// the source type of the delegate mapping is nullable or the source path is not nullable
// build mapping with null conditional access
if (_delegateMapping.SourceType.IsNullable() || !_sourceGetter.MemberPath.IsAnyNullable())
// if the source is not nullable, return it directly.
if (!_sourceGetter.MemberPath.IsAnyNullable())
{
ctx = ctx.WithSource(_sourceGetter.BuildAccess(ctx.Source));
return _delegateMapping.Build(ctx);
}

// if null conditional is allowed and the delegate mapping allows null source types,
// use null conditional access.
// => Map(source?.Value)
if (_delegateMapping.SourceType.IsNullable() && useNullConditionalAccess)
{
ctx = ctx.WithSource(_sourceGetter.BuildAccess(ctx.Source, nullConditional: true));
return _delegateMapping.Build(ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ public class UserImplementedInlinedExpressionMapping(
ParameterSyntax sourceParameter,
IReadOnlyDictionary<SyntaxAnnotation, INewInstanceMapping> mappingInvocations,
ExpressionSyntax mappingBody
) : NewInstanceMapping(userMapping.SourceType, userMapping.TargetType), INewInstanceMapping
) : NewInstanceMapping(userMapping.SourceType, userMapping.TargetType), INewInstanceUserMapping
{
public IMethodSymbol Method => userMapping.Method;
public bool? Default => userMapping.Default;
public bool IsExternal => userMapping.IsExternal;

public override ExpressionSyntax Build(TypeMappingBuildContext ctx)
{
var body = InlineUserMappings(ctx, mappingBody);
Expand Down
2 changes: 1 addition & 1 deletion src/Riok.Mapperly/Symbols/Members/MemberPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Riok.Mapperly.Symbols.Members;
/// Represents a (possibly empty) list of members to access a certain member.
/// E.g. A.B.C
/// </summary>
[DebuggerDisplay("{ToDebugString}")]
[DebuggerDisplay("{ToDebugString()}")]
public abstract class MemberPath(ITypeSymbol rootType, IReadOnlyList<IMappableMember> path)
{
protected const string MemberAccessSeparator = ".";
Expand Down
2 changes: 0 additions & 2 deletions src/Riok.Mapperly/Symbols/Members/NonEmptyMemberPath.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
using System.Diagnostics;
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Descriptors;

namespace Riok.Mapperly.Symbols.Members;

[DebuggerDisplay("{FullName}")]
public class NonEmptyMemberPath : MemberPath
{
public NonEmptyMemberPath(ITypeSymbol rootType, IReadOnlyList<IMappableMember> path)
Expand Down
3 changes: 3 additions & 0 deletions src/Riok.Mapperly/Symbols/Members/SourceMemberPath.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
using System.Diagnostics;

namespace Riok.Mapperly.Symbols.Members;

[DebuggerDisplay("{MemberPath} ({Type})")]
public record SourceMemberPath(MemberPath MemberPath, SourceMemberType Type);
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,26 @@ public Task ClassToClassNullableSourcePathAutoFlatten()
return TestHelper.VerifyGenerator(source);
}

[Fact]
public Task NestedPropertyWithDeepCloneable()
{
// see https://github.com/riok/mapperly/issues/1710
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"""
public partial System.Linq.IQueryable<B> Map(System.Linq.IQueryable<A> source);
[MapNestedProperties("Nested")]
public partial B MapConfig(A source);
""",
TestSourceBuilderOptions.WithDeepCloning,
"class A { public C Nested { get; set; } }",
"class B { public string[] Value0 { get; set; } public string Value { get; set; } }",
"class C { public string[] Value0 { get; set; } public string Value { get; set; } }"
);

return TestHelper.VerifyGenerator(source, TestHelperOptions.DisabledNullable);
}

[Fact]
public Task ClassToClassNullableSourcePathAutoFlattenString()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//HintName: Mapper.g.cs
// <auto-generated />
#nullable enable
public partial class Mapper
{
[global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")]
public partial global::System.Linq.IQueryable<global::B?>? Map(global::System.Linq.IQueryable<global::A?>? source)
{
if (source == null)
return default;
#nullable disable
return System.Linq.Queryable.Select(
source,
x => new global::B()
{
Value0 = x.Nested != null && x.Nested.Value0 != null ? global::System.Linq.Enumerable.ToArray(
global::System.Linq.Enumerable.Select(x.Nested.Value0, x1 => x1 == null ? default : x1)
) : default,
Value = x.Nested != null && x.Nested.Value != null ? x.Nested.Value : default,
}
);
#nullable enable
}

[global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")]
public partial global::B? MapConfig(global::A? source)
{
if (source == null)
return default;
var target = new global::B();
if (source.Nested?.Value0 != null)
{
target.Value0 = MapToStringArray(source.Nested.Value0);
}
else
{
target.Value0 = null;
}
target.Value = source.Nested?.Value;
return target;
}

[global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")]
private string?[] MapToStringArray(string?[] source)
{
var target = new string?[source.Length];
for (var i = 0; i < source.Length; i++)
{
target[i] = source[i] == null ? default : source[i]!;
}
return target;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,25 @@
DefaultSeverity: Error,
IsEnabledByDefault: true
}
},
{
Location: /*

[MapProperty(nameof(A.StringValue1), nameof(B.StringValue1), Use = nameof(ModifyString)]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[MapProperty(nameof(A.StringValue2), nameof(B.StringValue2), Use = nameof(ModifyString2)]
*/
: (18,1)-(18,87),
Message: The referenced mapping name ModifyString is ambiguous, use a unique name,
Severity: Error,
Descriptor: {
Id: RMG062,
Title: The referenced mapping name is ambiguous,
MessageFormat: The referenced mapping name {0} is ambiguous, use a unique name,
Category: Mapper,
DefaultSeverity: Error,
IsEnabledByDefault: true
}
}
]
}

0 comments on commit a63c8b3

Please sign in to comment.