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

Expand incorrectly handles nested expressions with anonymous type parameters #2

Closed
Epp-code opened this issue Mar 25, 2014 · 1 comment

Comments

@Epp-code
Copy link

Here's a test that reproduces the issue:

    public Expression<Func<T, TResult>> GetExpression<T, TResult>(
        T instanceForTypeInference,
        Expression<Func<T, TResult>> expression )
    {
        return expression;
    }

    public void AnonymousTypeParameterTest()
    {
        var anon = new
        {
            Id = 1,
        };

        var getIdExpression = GetExpression( anon, a => a.Id );
        var compareIdExpression = GetExpression( anon, a => getIdExpression.Invoke( a ) == 1 );

        var expanded = compareIdExpression.Expand();

        var areEqual = expanded.Compile().Invoke( anon );

        Assert.Equal( areEqual, true );
    }

With unmodified LinqKit code, this test throws the following exception during the Compile call, indicating that Expand returned an invalid Expression:

System.InvalidOperationException : variable 'a' of type '<>f__AnonymousType0`1[System.Int32]' referenced from scope '', but it is not defined
   at System.Linq.Expressions.Compiler.VariableBinder.Reference(ParameterExpression node, VariableStorageKind storage)
   at System.Linq.Expressions.Compiler.VariableBinder.VisitParameter(ParameterExpression node)
   at System.Linq.Expressions.ParameterExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitMember(MemberExpression node)
   at System.Linq.Expressions.MemberExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
   at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.Visit(ReadOnlyCollection`1 nodes)
   at System.Linq.Expressions.Compiler.VariableBinder.VisitLambda[T](Expression`1 node)
   at System.Linq.Expressions.Expression`1.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.Compiler.LambdaCompiler.Compile(LambdaExpression lambda, DebugInfoGenerator debugInfoGenerator)
   at System.Linq.Expressions.Expression`1.Compile()
   at LinqKit.Tests.ExpressionCombinerTest.AnonymousTypeParameterTest() in <redacted>

While researching this issue, I came across this StackOverflow posting:

http://stackoverflow.com/questions/13501437/linq-entities-build-query-at-runtime-the-parameter-is-not-in-scope-linqkit

Which recommends updating the ExpressionExpander.VisitMemberAccess method to be:

    protected override Expression VisitMemberAccess (MemberExpression m)
    {
        // Strip out any references to expressions captured by outer variables - LINQ to SQL can't handle these:
        string typeName = m.Member.DeclaringType.Name;
        bool isAnonymous = typeName.StartsWith("<>f__AnonymousType"),
        isOuter = !isAnonymous && typeName.StartsWith("<>");
        if (isOuter)
            return TransformExpr (m);

        return base.VisitMemberAccess (m);
    }

I verified that the test above does pass with this modification, but I don't understand ExpressionExpander well enough to know if this is a complete fix.

@scottksmith95
Copy link
Owner

I am not the original author of this code. This is just the official repository for it so others can benefit and contribute to it.

Your suggested change looks good to me.

If you test it more and submit a pull request with the new test and changes, we can merge it in.

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

No branches or pull requests

2 participants