Skip to content

Commit

Permalink
fix: resolve user mappings correctly when reference handling is enabled
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
latonz committed Mar 11, 2024
1 parent b4d5e74 commit 0ad1699
Show file tree
Hide file tree
Showing 21 changed files with 178 additions and 159 deletions.
Expand Up @@ -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,
Expand Down
Expand Up @@ -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;
}
Expand Down
14 changes: 4 additions & 10 deletions src/Riok.Mapperly/Descriptors/MappingCollection.cs
Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
Expand Up @@ -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);
}
Expand Up @@ -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;
}
8 changes: 0 additions & 8 deletions src/Riok.Mapperly/Descriptors/Mappings/ITypeMapping.cs
Expand Up @@ -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; }
}

This file was deleted.

6 changes: 1 addition & 5 deletions src/Riok.Mapperly/Descriptors/Mappings/MethodMapping.cs
Expand Up @@ -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; }

Expand All @@ -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));

Expand Down
5 changes: 0 additions & 5 deletions src/Riok.Mapperly/Descriptors/Mappings/NewInstanceMapping.cs
Expand Up @@ -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;

Expand Down

This file was deleted.

Expand Up @@ -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)
{

Check warning on line 43 in src/Riok.Mapperly/Descriptors/Mappings/UserMappings/UserDefinedExistingTargetMethodMapping.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L43 was not covered by tests
return ctx.SyntaxFactory.SingleStatement(
Invocation(
MethodName,
SourceParameter.WithArgument(ctx.Source),
TargetParameter.WithArgument(target),
ReferenceHandlerParameter?.WithArgument(ctx.ReferenceHandler)
)
);
}

Check warning on line 52 in src/Riok.Mapperly/Descriptors/Mappings/UserMappings/UserDefinedExistingTargetMethodMapping.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/Mappings/UserMappings/UserDefinedExistingTargetMethodMapping.cs#L45-L52

Added lines #L45 - L52 were not covered by tests

public override IEnumerable<StatementSyntax> BuildBody(TypeMappingBuildContext ctx)
{
Expand All @@ -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);
Expand Down
Expand Up @@ -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>();
Expand All @@ -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
Expand Down
Expand Up @@ -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);
Expand Down
18 changes: 0 additions & 18 deletions src/Riok.Mapperly/Helpers/PriorityQueueExtensions.cs

This file was deleted.

17 changes: 17 additions & 0 deletions 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;
}
}
}

0 comments on commit 0ad1699

Please sign in to comment.