mapperly icon indicating copy to clipboard operation
mapperly copied to clipboard

Support generic mappings

Open trejjam opened this issue 1 year ago • 4 comments

Is your feature request related to a problem? Please describe.

Mapping definition

public partial IReadOnlyCollection<long> Map(IReadOnlyCollection<int> source);

produces nice mapping using an array:

var target = new long[source.Count];
var i = 0;
foreach (var item in source)
{
    target[i] = (long)item;
    i++;
}
return target;

When you change it to generic:

public partial IReadOnlyCollection<T> MapItems<T>(IReadOnlyCollection<A> source) where T : class;
public partial T MapItem<T>(A source) where T : class;
public partial B MapToB(A source);

public record A(string Value);
public record B(string Value);

it refuses to generate array mapping.

Describe the solution you'd like

public partial IReadOnlyCollection<T> MapItems<T>(IReadOnlyCollection<A> source)
{
  var target = new T[source.Count];
  var i = 0;
  foreach (var item in source)
  {
      target[i] = MapItem(item);
      i++;
  }
  return target;
}

Additional context Failing test

Workaround: Add another (specific-non-generic) mapping, then generic arm-search finds a match. The downside is that you need to define it for all generic arguments (which makes it ugly)

public partial IReadOnlyCollection<B> MapItems(IReadOnlyCollection<A> source);

trejjam avatar May 13 '24 18:05 trejjam

If you can point me to the solution, I can implement it.

trejjam avatar May 13 '24 18:05 trejjam

Thanks for opening this. It seems to be somewhat related to https://github.com/riok/mapperly/issues/884 and https://github.com/riok/mapperly/issues/451. IMO we should try to implement support for mappings with open generics no matter whether it is a collection or not. However, this is not a trivial task as probably a lot of different parts of the codebase need to be touched.

  • Introduce new generic mappings (in Riok.Mapperly.Descriptors.Mappings)

  • Extract them while discovering user mappings (in Riok.Mapperly.Descriptors.UserMethodMappingExtractor)

  • If a mapping is needed and one cannot be found, try to match one of the discovered generic mappings (make sure the signature matches and no type constraints are validated) (in Riok.Mapperly.Descriptors.MappingCollection and callers)

  • Include the generic mappings in the currently supported generic mappings (probably not in the first iteration as this could get really complicated) (in Riok.Mapperly.Descriptors.MappingBodyBuilders.RuntimeTargetTypeMappingBodyBuilder)

    • Idea 1: resolve all possible types in the generator (as in the sample below); could get really complicated really fast, does not work with #451, could result in a LOT of switch arms
    • Idea 2: match against open generic types and type constraints at runtime; would need more reflection at runtime and therefore probably also make things slower at runtime
    • Idea 3: implement both Idea 1 + 2
  • How would this work in Expression mappings?

These are the points that currently come to my mind.

Example

public partial IReadOnlyCollection<S, T> MapItems<S, T>(IReadOnlyCollection<S> source)
{
  var target = new T[source.Count];
  var i = 0;
  foreach (var item in source)
  {
      target[i] = MapItem(item);
      i++;
  }
  return target;
}

public partial T MapItem<S, T>(S source)
{
    return source switch
    {
        A x when typeof(T).IsAssignableFrom(typeof(B)) => MapToB(x),
        IReadOnlyCollection<A> x when typeof(IReadOnlyCollection<B>).IsAssignableFrom(typeof(IReadOnlyCollection<B>)) => MapItems<A, B>(x),
        _ => throw ...,
    }
}

public partial B MapToB(A source)
{
    return new B(source.Value);
}

latonz avatar May 16 '24 01:05 latonz

Ran into an issue related to this today. I have a generic wrapper class for search results, so its generic type parameter could be practically anything in the app domain. That makes it pretty unreasonable to register every possible closed generic mapping. That being said, was curious if this is something actively being worked on, or if it's waiting on a dev resource.

Ixonal avatar Dec 06 '24 14:12 Ixonal

Not on my side; I have too full backlog now.

trejjam avatar Dec 06 '24 15:12 trejjam