3

Check this fiddle for the error: https://dotnetfiddle.net/tlz4Qg

I have two classes like this:

public class ParentType{
    private ParentType(){}

    public int Id { get; protected set; }
    public SubType Sub { get; protected set; }
}

public class SubType{
    private SubType(){}

    public int Id { get; protected set; }
}

I am going to transform a multilevel anonymous expression to a multilevel non-anonymous expression. To achieve this I have an expression like the below-mentioned one:

x => new
{
   x.Id,
   Sub = new
   {
      x.Sub.Id
   }
}

To achieve that goal, I have transformed it to an expression like this:

x => new ParentType()
{
   Id = x.Id,
   Sub = new SubType()
   {
      Id = x.Sub.Id
   },
 }

But when I call Compile() method, I get the following error:

Variable 'x.Sub' of type 'SubType' referenced from scope '' but it is not defined

Here is my visitor class:

public class ReturnTypeVisitor<TIn, TOut> : ExpressionVisitor
{
    private readonly Type funcToReplace;
    private ParameterExpression currentParameter;
    private ParameterExpression defaultParameter;
    private Type currentType;

    public ReturnTypeVisitor() => funcToReplace = typeof(Func<,>).MakeGenericType(typeof(TIn), typeof(object));

    protected override Expression VisitNew(NewExpression node)
    {
        if (!node.Type.IsAnonymousType())
            return base.VisitNew(node);

        if (currentType == null)
            currentType = typeof(TOut);

        var ctor = currentType.GetPrivateConstructor();
        if (ctor == null)
            return base.VisitNew(node);

        NewExpression expr = Expression.New(ctor);
        IEnumerable<MemberBinding> bindings = node.Members.Select(x =>
        {
            var mi = currentType.GetProperty(x.Name);

 //if the type is anonymous then I need to transform its body
                if (((PropertyInfo)x).PropertyType.IsAnonymousType())
                {
 //This section is became unnecessary complex!
 //
                    var property = (PropertyInfo)x;

                    var parentType = currentType;
                    var parentParameter = currentParameter;

                    currentType = currentType.GetProperty(property.Name).PropertyType;

                    currentParameter = Expression.Parameter(currentType, currentParameter.Name + "." + property.Name);

 //I pass the inner anonymous expression to VisitNew and make the non-anonymous expression from it
                    var xOriginal = VisitNew(node.Arguments.FirstOrDefault(a => a.Type == property.PropertyType) as NewExpression);

                    currentType = parentType;
                    currentParameter = parentParameter;

                    return (MemberBinding)Expression.Bind(mi, xOriginal);
                }
                else//if type is not anonymous then simple find the property and make the memberbinding
                {
                    var xOriginal = Expression.PropertyOrField(currentParameter, x.Name);
                    return (MemberBinding)Expression.Bind(mi, xOriginal);
                }
        });

        return Expression.MemberInit(expr, bindings);
    }

    protected override Expression VisitLambda<T>(Expression<T> node)
    {
        if (typeof(T) != funcToReplace)
            return base.VisitLambda(node);

        defaultParameter = node.Parameters.First();

        currentParameter = defaultParameter;
        var body = Visit(node.Body);

        return Expression.Lambda<Func<TIn, TOut>>(body, currentParameter);
    }
}

And use it like this:

public static Expression<Func<Tin, Tout>> Transform<Tin, Tout>(this Expression<Func<Tin, object>> source)
    {
        var visitor = new ReturnTypeVisitor<Tin, Tout>();
        var result = (Expression<Func<Tin, Tout>>)visitor.Visit(source);
        return result;// result.Compile() throw the aforementioned error
    }

Here is the extension methods used inside my Visitor class:

public static ConstructorInfo GetPrivateConstructor(this Type type) =>
            type.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, Type.EmptyTypes, null);

// this hack taken from https://stackoverflow.com/a/2483054/4685428
// and https://stackoverflow.com/a/1650895/4685428
public static bool IsAnonymousType(this Type type)
{
 var markedWithAttribute = type.GetCustomAttributes(typeof(CompilerGeneratedAttribute), inherit: false).Any();
 var typeName = type.Name;

 return markedWithAttribute
               && (typeName.StartsWith("<>") || type.Name.StartsWith("VB$"))
               && typeName.Contains("AnonymousType");
}

Update

Here is the .Net Fiddle link for the problem: https://dotnetfiddle.net/tlz4Qg

Update

I have removed the extra codes that seems to be out of the problem scope.

Vahid Farahmandian
  • 6,081
  • 7
  • 42
  • 62
  • Can't really see what's wrong without context. Can you create a **[mcve]** that we can run, showing a simple input and expected output? Forget about your visitor class - just data and a method. – Reinstate Monica Cellio Apr 17 '19 at 15:18
  • @Archer I have updated the question and added some details to easily reproduce the issue. Thank you – Vahid Farahmandian Apr 17 '19 at 15:29
  • 2
    @VahidFarahmandian Please include extensions methods like `GetPrivateConstructor` and `IsAnonymousType`. Also check lambda used in `node.Members.Select`: it should not compile because not all code path have a return. – Aleks Andreev Apr 17 '19 at 15:45
  • @AleksAndreev I ahve updated the question and added the extension methods and correct the compile error – Vahid Farahmandian Apr 17 '19 at 15:52
  • @Archer I have added the fiddle link for problem. Thank you – Vahid Farahmandian Apr 20 '19 at 11:26
  • @AleksAndreev I have added the fiddle link for problem. Thank you – Vahid Farahmandian Apr 20 '19 at 11:26
  • @Nkosi I have corrected the question, that was miss-typed error. Thank you for comment, however it was not the cause of error. Please check the fiddle link too. – Vahid Farahmandian Apr 20 '19 at 12:00
  • 1
    @VahidFarahmandian I knew that. I was just bringing it to your attention to clarify the typo – Nkosi Apr 20 '19 at 12:13
  • 2
    It's not really clear to me how you expect your current code to work - but I'm pretty sure you shouldn't be using `Expression.Parameter`, as you're not really trying to create a new parameter. You want the parameter expression in the result to look the same as the parameter expression in the original, right? So I think you're going about things the wrong way there. – Jon Skeet Apr 20 '19 at 12:13
  • @JonSkeet In fact what I am trying to do is, to transform a multilevel anonymous expression to a multilevel non-anonymouse expression. I am agree with you that I have confused myself with the problem in Visitor class, but I do not know how to get rid of it – Vahid Farahmandian Apr 20 '19 at 13:13
  • 3
    Without understanding how your current code tries to work, it's very hard to help - I'd basically start from scratch. If you can work out why you were creating parameter expressions at all, and add comments to your code, that would help explain things. – Jon Skeet Apr 20 '19 at 13:24
  • @JonSkeet thanks for your support, I think I need to find a simple and better way to describe the requirement and problem. I am going to edit the question and make it as simple as possible and make it easy to understand. I will inform you, once I update the question. – Vahid Farahmandian Apr 20 '19 at 19:47

1 Answers1

4

The cause of the problem in question is the line

currentParameter = Expression.Parameter(currentType, currentParameter.Name + "." + property.Name);

inside VisitNew method.

With your sample, it creates a new parameter called "x.Sub", so if we mark the parameters with {}, the actual result is

Sub = new SubType()
{
    Id = {x.Sub}.Id
}, 

rather than expected

Sub = new SubType()
{
    Id = {x}.Sub.Id
},

In general you should not create new ParameterExpressions except when remapping lambda expressions. And all newly created parameters should be passed to Expression.Lambda call, otherwise they will be considered "not defined".

Also please note that the visitor code has some assumptions which doesn't hold in general. For instance

var xOriginal = Expression.PropertyOrField(currentParameter, x.Name);

won't work inside nested new, because there you need access to a member of the x parameter like x.Sub.Id rather than x.Id. Which is basically the corersonding expression from NewExpression.Arguments.

Processing nested lambda expressions or collection type members and LINQ methods with expression visitors requires much more state control. While converting simple nested anonymous new expression like in the sample does not even need a ExpressionVisitor, because it could easily be achieved with simple recursive method like this:

public static Expression<Func<Tin, Tout>> Transform<Tin, Tout>(this Expression<Func<Tin, object>> source)
{
    return Expression.Lambda<Func<Tin, Tout>>(
        Transform(source.Body, typeof(Tout)),
        source.Parameters);
}

static Expression Transform(Expression source, Type type)
{
    if (source.Type != type && source is NewExpression newExpr && newExpr.Members.Count > 0)
    {
        return Expression.MemberInit(Expression.New(type), newExpr.Members
            .Select(m => type.GetProperty(m.Name))
            .Zip(newExpr.Arguments, (m, e) => Expression.Bind(m, Transform(e, m.PropertyType))));
    }
    return source;
}
Ivan Stoev
  • 195,425
  • 15
  • 312
  • 343
  • Thats really amazing and works perfectly, however I have some challenges with collection type members, which I am working on it to overcome this challenge. – Vahid Farahmandian Apr 22 '19 at 10:27
  • Do you have any idea about transforming this too?: Tests = x.SubType.Tests.Select(u => new {Y = new {u.Id}) – Vahid Farahmandian Apr 23 '19 at 09:47
  • 1
    If it's a simple `Select`, may be not that big deal. But if it could be virtually everything, e.g. other LINQ operators before and/or after `Select`, than it's much more complicated because you have to change the lambdas return or input types, LINQ methods generic type arguments (binding to different method definitions) etc. - all that with proper state machine. Basically all AutoMapper and similar are doing with predefined mappings. – Ivan Stoev Apr 23 '19 at 09:54
  • No, it is just a simple SELECT as commented in aforementioned comment. – Vahid Farahmandian Apr 23 '19 at 10:05
  • 1
    Can you please post another (follow up) question with the new scenario because the comment space is limited, and it definitely needs some code :) – Ivan Stoev Apr 23 '19 at 10:35