fix: resolve user mappings correctly when reference handling is enabl…

…ed (#1162)

* Removes the BuildPriority of a Mapping, all mappings are built with the same priority.
  This is not needed anymore as user defined mappings with reference handling enabled but no reference handler parameter
  just delegate to the a generated version of the mapping with a reference handler parameter.
* Removes CallableByOthers of a Mapping, all mappings are now considered callable by others
  For runtime target type mappings there is no type match anyway.
  And user defined mappings with reference handling enabled but no reference handler parameter
  just delegate to the version with a reference handler parameter now.

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/RuntimeTargetTypeMappingBodyBuilder.cs

@@ -50,21 +50,8 @@ public static void BuildMappingBody(MappingBuilderContext ctx, UserDefinedNewIns
         BuildMappingBody(ctx, mapping, mappings);
     }
 
-    private static IEnumerable<ITypeMapping> GetUserMappingCandidates(MappingBuilderContext ctx)
-    {
-        foreach (var userMapping in ctx.UserMappings)
-        {
-            // exclude runtime target type
-            if (userMapping is UserDefinedNewInstanceRuntimeTargetTypeMapping)
-                continue;
-
-            if (userMapping.CallableByOtherMappings)
-                yield return userMapping;
-
-            if (userMapping is IDelegateUserMapping { DelegateMapping.CallableByOtherMappings: true } delegateUserMapping)
-                yield return delegateUserMapping.DelegateMapping;
-        }
-    }
+    private static IEnumerable<ITypeMapping> GetUserMappingCandidates(MappingBuilderContext ctx) =>
+        ctx.UserMappings.Where(x => x is not UserDefinedNewInstanceRuntimeTargetTypeMapping);
 
     private static void BuildMappingBody(
         MappingBuilderContext ctx,

src/Riok.Mapperly/Descriptors/MappingBodyBuilders/UserMethodMappingBodyBuilder.cs

@@ -28,10 +28,10 @@ public static void BuildMappingBody(MappingBuilderContext ctx, UserDefinedNewIns
     {
         var options = MappingBuildingOptions.KeepUserSymbol;
 
-        // this this mapping is not callable by others
-        // the delegate mapping is probably callable by others
-        // and therefore reusable
-        if (!mapping.CallableByOtherMappings)
+        // the delegate mapping is not embedded
+        // and is therefore reusable
+        // if embedded, only the original mapping is callable by others
+        if (mapping.InternalReferenceHandlingEnabled)
         {
             options |= MappingBuildingOptions.MarkAsReusable;
         }

src/Riok.Mapperly/Descriptors/MappingCollection.cs

@@ -29,7 +29,7 @@ public class MappingCollection
     /// <summary>
     /// Queue of mappings which don't have the body built yet
     /// </summary>
-    private readonly PriorityQueue<(IMapping, MappingBuilderContext), MappingBodyBuildingPriority> _mappingsToBuildBody = new();
+    private readonly Queue<(IMapping, MappingBuilderContext)> _mappingsToBuildBody = new();
 
     /// <summary>
     /// All new instance mappings
@@ -62,8 +62,7 @@ public class MappingCollection
 
     public IEnumerable<(IMapping, MappingBuilderContext)> DequeueMappingsToBuildBody() => _mappingsToBuildBody.DequeueAll();
 
-    public void EnqueueToBuildBody(ITypeMapping mapping, MappingBuilderContext ctx) =>
-        _mappingsToBuildBody.Enqueue((mapping, ctx), mapping.BodyBuildingPriority);
+    public void EnqueueToBuildBody(ITypeMapping mapping, MappingBuilderContext ctx) => _mappingsToBuildBody.Enqueue((mapping, ctx));
 
     public MappingCollectionAddResult AddUserMapping(IUserMapping userMapping, bool ignoreDuplicates, string? name)
     {
@@ -138,7 +137,8 @@ private class MappingCollectionInstance<T, TUserMapping>
         where TUserMapping : T, IUserMapping
     {
         /// <summary>
-        /// Callable mapping of each type pair + config.
+        /// Default mappings of each type pair + config.
+        /// A default mappings is the mapping Mapperly should use to convert from one type to another.
         /// Contains mappings to build and already built mappings.
         /// </summary>
         private readonly Dictionary<TypeMappingKey, T> _defaultMappings = new();
@@ -228,9 +228,6 @@ public void AddNamedUserMapping(string? name, TUserMapping mapping)
 
         public MappingCollectionAddResult TryAddAsDefault(T mapping, TypeMappingConfiguration config)
         {
-            if (!mapping.CallableByOtherMappings)
-                return MappingCollectionAddResult.NotAddedIgnored;
-
             var mappingKey = new TypeMappingKey(mapping, config);
             if (_defaultMappings.ContainsKey(mappingKey))
                 return MappingCollectionAddResult.NotAddedDuplicated;
@@ -241,9 +238,6 @@ public MappingCollectionAddResult TryAddAsDefault(T mapping, TypeMappingConfigur
 
         public MappingCollectionAddResult AddUserMapping(TUserMapping mapping, bool ignoreDuplicates, bool? isDefault, string? name)
         {
-            if (!mapping.CallableByOtherMappings)
-                return MappingCollectionAddResult.NotAddedIgnored;
-
             AddNamedUserMapping(name, mapping);
 
             return isDefault switch

src/Riok.Mapperly/Descriptors/Mappings/ExistingTarget/ExistingTargetMapping.cs

@@ -24,11 +24,7 @@ protected ExistingTargetMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
 
     public ITypeSymbol TargetType { get; }
 
-    public virtual bool CallableByOtherMappings => true;
-
     public virtual bool IsSynthetic => false;
 
-    public MappingBodyBuildingPriority BodyBuildingPriority => MappingBodyBuildingPriority.Default;
-
     public abstract IEnumerable<StatementSyntax> Build(TypeMappingBuildContext ctx, ExpressionSyntax target);
 }

src/Riok.Mapperly/Descriptors/Mappings/ExistingTarget/ObjectMemberExistingTargetMapping.cs

@@ -18,9 +18,5 @@ public class ObjectMemberExistingTargetMapping(ITypeSymbol sourceType, ITypeSymb
 
     public ITypeSymbol TargetType { get; } = targetType;
 
-    public bool CallableByOtherMappings => true;
-
     public bool IsSynthetic => false;
-
-    public MappingBodyBuildingPriority BodyBuildingPriority => MappingBodyBuildingPriority.Default;
 }

src/Riok.Mapperly/Descriptors/Mappings/ITypeMapping.cs

@@ -5,16 +5,8 @@ namespace Riok.Mapperly.Descriptors.Mappings;
 /// </summary>
 public interface ITypeMapping : IMapping
 {
-    /// <summary>
-    /// Gets a value indicating if this mapping can be called / built by another mapping.
-    /// This should be <c>true</c> for most mappings.
-    /// </summary>
-    bool CallableByOtherMappings { get; }
-
     /// <summary>
     /// Gets a value indicating whether this mapping produces any code or can be omitted completely (eg. direct assignments or delegate mappings).
     /// </summary>
     bool IsSynthetic { get; }
-
-    MappingBodyBuildingPriority BodyBuildingPriority { get; }
 }

src/Riok.Mapperly/Descriptors/Mappings/MethodMapping.cs

@@ -61,7 +61,7 @@ ITypeSymbol targetType
 
     protected bool IsExtensionMethod { get; }
 
-    private string MethodName => _methodName ?? throw new InvalidOperationException();
+    protected string MethodName => _methodName ?? throw new InvalidOperationException();
 
     protected MethodParameter SourceParameter { get; }
 
@@ -71,12 +71,8 @@ ITypeSymbol targetType
 
     public ITypeSymbol TargetType { get; }
 
-    public virtual bool CallableByOtherMappings => true;
-
     public bool IsSynthetic => false;
 
-    public virtual MappingBodyBuildingPriority BodyBuildingPriority => MappingBodyBuildingPriority.Default;
-
     public virtual ExpressionSyntax Build(TypeMappingBuildContext ctx) =>
         Invocation(MethodName, SourceParameter.WithArgument(ctx.Source), ReferenceHandlerParameter?.WithArgument(ctx.ReferenceHandler));
 

src/Riok.Mapperly/Descriptors/Mappings/NewInstanceMapping.cs

@@ -22,11 +22,6 @@ protected NewInstanceMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
 
     public ITypeSymbol TargetType { get; }
 
-    public virtual MappingBodyBuildingPriority BodyBuildingPriority => MappingBodyBuildingPriority.Default;
-
-    /// <inheritdoc cref="INewInstanceMapping.CallableByOtherMappings"/>
-    public virtual bool CallableByOtherMappings => true;
-
     /// <inheritdoc cref="INewInstanceMapping.IsSynthetic"/>
     public virtual bool IsSynthetic => false;
 

src/Riok.Mapperly/Descriptors/Mappings/UserMappings/UserDefinedExistingTargetMethodMapping.cs

@@ -18,29 +18,38 @@ public class UserDefinedExistingTargetMethodMapping(
     MethodParameter targetParameter,
     MethodParameter? referenceHandlerParameter,
     bool enableReferenceHandling
-) : NewInstanceMethodMapping(method, sourceParameter, referenceHandlerParameter, targetParameter.Type), IExistingTargetUserMapping
+) : MethodMapping(method, sourceParameter, referenceHandlerParameter, targetParameter.Type), IExistingTargetUserMapping
 {
     private IExistingTargetMapping? _delegateMapping;
 
     public IMethodSymbol Method { get; } = method;
 
-    /// <summary>
-    /// Always false, since <see cref="CallableByOtherMappings"/> is false
-    /// this can never be the default.
-    /// </summary>
     public bool? Default => false;
 
     private MethodParameter TargetParameter { get; } = targetParameter;
 
-    public override bool CallableByOtherMappings => false;
+    /// <summary>
+    /// The reference handling is enabled but is only internal to this method.
+    /// No reference handler parameter is passed.
+    /// </summary>
+    private bool InternalReferenceHandlingEnabled => enableReferenceHandling && ReferenceHandlerParameter == null;
 
     public void SetDelegateMapping(IExistingTargetMapping delegateMapping) => _delegateMapping = delegateMapping;
 
     public override ExpressionSyntax Build(TypeMappingBuildContext ctx) =>
         throw new InvalidOperationException($"{nameof(UserDefinedExistingTargetMethodMapping)} does not support {nameof(Build)}");
 
-    public IEnumerable<StatementSyntax> Build(TypeMappingBuildContext ctx, ExpressionSyntax target) =>
-        throw new InvalidOperationException($"{nameof(UserDefinedExistingTargetMethodMapping)} does not support {nameof(Build)}");
+    public IEnumerable<StatementSyntax> Build(TypeMappingBuildContext ctx, ExpressionSyntax target)
+    {
+        return ctx.SyntaxFactory.SingleStatement(
+            Invocation(
+                MethodName,
+                SourceParameter.WithArgument(ctx.Source),
+                TargetParameter.WithArgument(target),
+                ReferenceHandlerParameter?.WithArgument(ctx.ReferenceHandler)
+            )
+        );
+    }
 
     public override IEnumerable<StatementSyntax> BuildBody(TypeMappingBuildContext ctx)
     {
@@ -60,7 +69,7 @@ public override IEnumerable<StatementSyntax> BuildBody(TypeMappingBuildContext c
 
         // if reference handling is enabled and no reference handler parameter is declared
         // a new reference handler is instantiated and used.
-        if (enableReferenceHandling && ReferenceHandlerParameter == null)
+        if (InternalReferenceHandlingEnabled)
         {
             // var refHandler = new RefHandler();
             var referenceHandlerName = ctx.NameBuilder.New(DefaultReferenceHandlerParameterName);

src/Riok.Mapperly/Descriptors/Mappings/UserMappings/UserDefinedNewInstanceMethodMapping.cs

@@ -15,27 +15,37 @@ public class UserDefinedNewInstanceMethodMapping(
     MethodParameter? referenceHandlerParameter,
     ITypeSymbol targetType,
     bool enableReferenceHandling
-) : NewInstanceMethodMapping(method, sourceParameter, referenceHandlerParameter, targetType), IDelegateUserMapping, INewInstanceUserMapping
+) : NewInstanceMethodMapping(method, sourceParameter, referenceHandlerParameter, targetType), INewInstanceUserMapping
 {
     public IMethodSymbol Method { get; } = method;
 
     public bool? Default { get; } = isDefault;
 
     public INewInstanceMapping? DelegateMapping { get; private set; }
 
+    /// <summary>
+    /// The reference handling is enabled but is only internal to this method.
+    /// No reference handler parameter is passed.
+    /// </summary>
+    public bool InternalReferenceHandlingEnabled => enableReferenceHandling && ReferenceHandlerParameter == null;
+
     public void SetDelegateMapping(INewInstanceMapping mapping) => DelegateMapping = mapping;
 
+    public override ExpressionSyntax Build(TypeMappingBuildContext ctx)
+    {
+        return InternalReferenceHandlingEnabled ? DelegateMapping?.Build(ctx) ?? base.Build(ctx) : base.Build(ctx);
+    }
+
     public override IEnumerable<StatementSyntax> BuildBody(TypeMappingBuildContext ctx)
     {
         if (DelegateMapping == null)
         {
-            return new[] { ctx.SyntaxFactory.ExpressionStatement(ctx.SyntaxFactory.ThrowMappingNotImplementedExceptionStatement()), };
+            return new[] { ctx.SyntaxFactory.ExpressionStatement(ctx.SyntaxFactory.ThrowMappingNotImplementedExceptionStatement()) };
         }
 
-        // if reference handling is enabled and no reference handler parameter is declared
         // the generated mapping method is called with a new reference handler instance
         // otherwise the generated method is embedded
-        if (enableReferenceHandling && ReferenceHandlerParameter == null)
+        if (InternalReferenceHandlingEnabled)
         {
             // new RefHandler();
             var createRefHandler = ctx.SyntaxFactory.CreateInstance<PreserveReferenceHandler>();
@@ -49,12 +59,6 @@ public override IEnumerable<StatementSyntax> BuildBody(TypeMappingBuildContext c
         return new[] { ctx.SyntaxFactory.Return(DelegateMapping.Build(ctx)) };
     }
 
-    /// <summary>
-    /// A <see cref="UserDefinedNewInstanceMethodMapping"/> is callable by other mappings
-    /// if either reference handling is not activated, or the user defined a reference handler parameter.
-    /// </summary>
-    public override bool CallableByOtherMappings => !enableReferenceHandling || ReferenceHandlerParameter != null;
-
     internal override void EnableReferenceHandling(INamedTypeSymbol iReferenceHandlerType)
     {
         // the parameters of user defined methods should not be manipulated

src/Riok.Mapperly/Descriptors/Mappings/UserMappings/UserDefinedNewInstanceRuntimeTargetTypeMapping.cs

@@ -28,30 +28,27 @@ ITypeSymbol objectType
 
     private readonly List<RuntimeTargetTypeMapping> _mappings = new();
 
-    // requires the user mapping bodies
-    // as the delegate mapping of user mappings is only set after bodies are built
-    // and if reference handling is enabled,
-    // but the user mapping does not have a reference handling parameter,
-    // only the delegate mapping is callable by other mappings.
-    public override MappingBodyBuildingPriority BodyBuildingPriority => MappingBodyBuildingPriority.AfterUserMappings;
-
     public IMethodSymbol Method { get; } = method;
 
     /// <summary>
-    /// Always false, since <see cref="CallableByOtherMappings"/> is false
+    /// Always false, as this cannot be called by other mappings,
     /// this can never be the default.
     /// </summary>
     public bool? Default => false;
 
-    public override bool CallableByOtherMappings => false;
+    /// <summary>
+    /// The reference handling is enabled but is only internal to this method.
+    /// No reference handler parameter is passed.
+    /// </summary>
+    private bool InternalReferenceHandlingEnabled => enableReferenceHandling && ReferenceHandlerParameter == null;
 
     public void AddMappings(IEnumerable<RuntimeTargetTypeMapping> mappings) => _mappings.AddRange(mappings);
 
     public override IEnumerable<StatementSyntax> BuildBody(TypeMappingBuildContext ctx)
     {
         // if reference handling is enabled and no reference handler parameter is declared
         // a new reference handler is instantiated and used.
-        if (enableReferenceHandling && ReferenceHandlerParameter == null)
+        if (InternalReferenceHandlingEnabled)
         {
             // var refHandler = new RefHandler();
             var referenceHandlerName = ctx.NameBuilder.New(DefaultReferenceHandlerParameterName);

src/Riok.Mapperly/Helpers/QueueExtensions.cs

@@ -0,0 +1,17 @@
+namespace Riok.Mapperly.Helpers;
+
+internal static class QueueExtensions
+{
+    /// <summary>
+    /// Dequeues all nodes.
+    /// Items added while this operation is in progress are also considered.
+    /// </summary>
+    /// <returns>An enumerable with all items.</returns>
+    public static IEnumerable<TElement> DequeueAll<TElement>(this Queue<TElement> queue)
+    {
+        while (queue.TryDequeue(out var element))
+        {
+            yield return element;
+        }
+    }
+}