Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queryable projections: Inline user-implemented mapping expressions from different file #1406

Open
hartmair opened this issue Jul 21, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@hartmair
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When having mappings defined in different classes/files, I get RMG068: Cannot inline user implemented queryable expression mapping. In contrast, when I move the two mapper definitions into one class, the mapping gets propertly inlined.

Describe the solution you'd like
A clear and concise description of what you want to happen including a proposal on the API surface and the matching generated code.

public class Car
{
    public string Model { get; set; }
}
public class CarDto
{
    public string ModelName { get; set; }
}

public class B
{
    public Car? Value { get; set; }
}
public class BDto
{
    public CarDto? Value { get; set; }
}

[Mapper]
public static partial class CarMapper
{
    [MapProperty(nameof(Car.Model), nameof(CarDto.ModelName))]
    public static partial CarDto MapCar(Car src);

    private static partial BDto Map(B src);
    public static partial IQueryable<BDto> ThisWorks(this IQueryable<B> src);
        [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "3.6.0.0")]
        public static partial global::System.Linq.IQueryable<global::Vexillum.WebApp.AutoMapper.BDto> ThisWorks(this global::System.Linq.IQueryable<global::Vexillum.WebApp.AutoMapper.B> src)
        {
#nullable disable
            return System.Linq.Queryable.Select(src, x => new global::Vexillum.WebApp.AutoMapper.BDto()
            {
                Value = new global::Vexillum.WebApp.AutoMapper.CarDto()
                {
                    ModelName = x.Value.Model,
                },
            });
#nullable enable
        }
}

[Mapper]
[UseStaticMapper(typeof(CarMapper))]
public static partial class BMapper
{
    // RMG068: Cannot inline user implemented queryable expression mapping
    private static partial BDto Map(B src);
    public static partial IQueryable<BDto> ThisShouldWork(this IQueryable<B> src);
        [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "3.6.0.0")]
        public static partial global::System.Linq.IQueryable<global::Vexillum.WebApp.AutoMapper.BDto> ThisShouldWork(this global::System.Linq.IQueryable<global::Vexillum.WebApp.AutoMapper.B> src)
        {
#nullable disable
            return System.Linq.Queryable.Select(src, x => new global::Vexillum.WebApp.AutoMapper.BDto()
            {
                Value = global::Vexillum.WebApp.AutoMapper.CarMapper.MapCar(x.Value),
            });
#nullable enable
        }

Describe alternatives you've considered
Copying all dependend configurations into one class works as ugly workaround.

Additional context
Mapperly v3.6.0

@hartmair hartmair added the enhancement New feature or request label Jul 21, 2024
@latonz
Copy link
Contributor

latonz commented Jul 22, 2024

We decided to not support this in the first iteration of inlining, since the source syntax of external methods is not always available (e.g. if it is part of another assembly). However, this could be improved and inlining could always happen if the source syntax is available.

@hartmair
Copy link
Contributor Author

I have tried to provide a PR for this, but struggle as PartialImplementationPart is always null. Furthermore, even having the PartialImplementationPart would not be enough as Mapperly uses multi-statement bodies by default. Do you have any guidance how to get around this?

Failing Unit-Test:

namespace Riok.Mapperly.Tests.Mapping;

public class QueryableProjectionExternalTest
{
    [Fact]
    public Task UseStaticMapperInlinedExpression()
    {
        var source = TestSourceBuilder.CSharp(
            $$"""
            using System;
            using System.Linq;
            using System.Collections.Generic;
            using Riok.Mapperly.Abstractions;
            using Riok.Mapperly.Abstractions.ReferenceHandling;

            [Mapper]
            [UseStaticMapper(typeof(OtherMapper))]
            public partial class Mapper
            {
                private partial IQueryable<B> Map(IQueryable<A> source);
            }
            [Mapper]
            static partial class OtherMapper
            {
                public static partial D MapToD(C v);
            }
            class A { public string StringValue { get; set; } public C NestedValue { get; set; } }
            class B { public string StringValue { get; set; } public D NestedValue { get; set; } }
            class C { public string Value { get; set; } }
            class D { public string Value { get; set; } }
            """
        );

        return TestHelper.VerifyGenerator(source);
    }
}

P.S. While debugging Mapperly #1422 happened as a side-effect.

@latonz
Copy link
Contributor

latonz commented Aug 3, 2024

Thanks for your efforts!
PartialImplementationPart is probably null because it is compiled in another assembly / compilation reference or it is another Mapperly generated method body (which is not yet available in the current compilation as it's only being generated) and therefore not visible to the current compilation. This is where it gets complicated and this is also the reason why we didn't implement it in the first place. For some cases it may work with #1418 (compilation references)...
One approach for Mapperly generated methods in the same assembly would be to re-generate the body based on the configuration and inline it directly (this could be explored, not sure how complex this would be...). Other than that I can't really think of a reasonable approach to implement this. But maybe someone out there has a brilliant idea on how to implement this, would be very interested in alternative approaches 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants