Search code examples
c#expression-treesroslyn

Equality in Expression Trees doesn't use the correct operator overload


I got a something odd issue with expression trees and operator overloading (in particular with == and != operators).

I'm using the MemberwiseComparer from one of Marc Gravell's answers, more or less the one

public static class MemberComparer
{
    public static bool Equal<T>(T x, T y)
    {
        return EqualComparerCache<T>.Compare(x, y);
    }

    static class EqualComparerCache<T>
    {
        internal static readonly Func<T, T, bool> Compare = (a, b) => true;

        static EqualComparerCache()
        {
            var members = typeof(T).GetTypeInfo().DeclaredProperties.Cast<MemberInfo>()
                .Concat(typeof(T).GetTypeInfo().DeclaredFields.Where(p => !p.IsStatic && p.IsPublic).Cast<MemberInfo>());
            var x = Expression.Parameter(typeof(T), "x");
            var y = Expression.Parameter(typeof(T), "y");

            Expression body = null;
            foreach (var member in members)
            {
                Expression memberEqual;
                if (member is FieldInfo)
                {
                    memberEqual = Expression.Equal(
                        Expression.Field(x, (FieldInfo)member),
                        Expression.Field(y, (FieldInfo)member));
                }
                else if (member is PropertyInfo)
                {
                    memberEqual = Expression.Equal(
                        Expression.Property(x, (PropertyInfo)member),
                        Expression.Property(y, (PropertyInfo)member));
                }
                else
                {
                    throw new NotSupportedException(member.GetType().GetTypeInfo().Name);
                }

                body = body == null ? memberEqual : Expression.AndAlso(body, memberEqual);
            }

            if (body != null)
            {
                var lambda = Expression.Lambda<Func<T, T, bool>>(body, x, y);
                Compare = lambda.Compile();
            }
        }
    }
}

And a base class ValueObject<T> that serves as base class for value objects.

public class ValueObject<T> : IEquatable<T> where T : ValueObject<T>
{
    public virtual bool Equals(T other)
    {
        if (ReferenceEquals(this, other))
            return true;

        return MemberComparer.Equal<T>((T)this, other);
    }

    public override bool Equals(object obj)
    {
        return Equals(obj as T);
    }

    public override int GetHashCode()
    {
        return MemberComparer.GetHashCode((T)this);
    }

    public static bool operator ==(ValueObject<T> left, ValueObject<T> right)
    {
        // If both are null, or both are same instance, return true.
        if (ReferenceEquals(left, right))
        {
            return true;
        }

        // If one is null, but not both, return false.
        if (((object)left == null) || ((object)right == null))
        {
            return false;
        }

        return left.Equals(right);
    }

    public static bool operator !=(ValueObject<T> left, ValueObject<T> right)
    {
        return !(left == right);
    }
}

In general this works fine for classes which implement IEquatable<T> or scalar types and/or strings. However, when the class contains properties which are classes that implement ValueObject<T>, the comparison fails.

public class Test : ValueObject<Test>
{
    public string Value { get; set; }
}

public class Test2 : ValueObject<Test2>
{
    public Test Test { get; set; }
}

When comparing Test with Test it works fine.

var test1 = new Test { Value = "TestValue"; }
var test2 = new Test { Value = "TestValue"; }

Assert.True(test1==test2); // true
Assert.Equals(test1, test2); // true

But when comparing Test2 it fails:

var nestedTest1 = new Test2 { Test = new Test { Value = "TestValue"; } }
var nestedTest2 = new Test2 { Test = new Test { Value = "TestValue"; } }

Assert.True(nestedTest1==nestedTest2 ); // false
Assert.Equals(nestedTest1, nestedTest2 ); // false

// Second Test with referenced Test object
var test = new Test { Value = "TestValue"; }
var nestedTest1 = new Test2 { Test = test }
var nestedTest2 = new Test2 { Test = test }

Assert.True(nestedTest1==nestedTest2 ); // true
Assert.Equals(nestedTest1, nestedTest2 ); // true

The == operator override is called for the Test2 class, but not for Test class. When nestedTest1 and nestedTest2 reference the same Test object, it works. So the == overload is not called when the expression is built and compiled.

I couldn't find a reason why it would ignore it. Is this some change to Roslyn no one noticed or is there something wrong with the expression tree generation?

Of course I could rewrite the Expression Tree generation to call .Equals method instead, but this would add more complexity (and additional null checks). But the actual question is, why doesn't the compiled Expression Tree use the == overload and how to make it work?


Solution

  • I end up implementing a method which searches for the op_Equality operator override method and pass it to Expression.Equal as the 4th parameter.

    MethodInfo equalsOperator = FindMethod(memberType, "op_Equality", false);
    
    equalityExpression = Expression.Equal(
        Expression.Property(left, memberInfo),
        Expression.Property(right, memberInfo),
        false,
        equalsOperator);
    
    ... 
    private static MethodInfo FindMethod(Type type, string methodName, bool throwIfNotFound = true)
    {
        TypeInfo typeInfo = type.GetTypeInfo();
    
        // TODO: Improve to search methods with a specific signature and parameters
        while (typeInfo != null)
        {
            IEnumerable<MethodInfo> methodInfo = typeInfo.GetDeclaredMethods(methodName);
            if (methodInfo.Any())
                return methodInfo.First();
    
            typeInfo = typeInfo.BaseType?.GetTypeInfo();
        }
    
        if (!throwIfNotFound)
            return null;
    
        throw new InvalidOperationException($"Type '{type.GetTypeInfo().FullName}' has no '{methodName}' method.");
    }
    

    In my simple scenario it's sufficient (for now) to use the first op_Equality found there should be not more than one in the ValueObject<T> class and I made sure the MemberComparer.Equal<T>((T)this, other) is only called, when both objects are of the same type.