S4035 accepts when Equals is made virtual even though it does not fix the issue

(David Kantrowitz) #1

As documented in https://rules.sonarsource.com/csharp/RSPEC-4035, making Equals virtual in the base class removes the code smell S4035. However, this does not solve the problem S4035 is intended to address. In the following code, based on the sample code shown in S4035, the virtual keyword has been added, yet the base class implementation of Equals(Base other) is still being called, not the derived implementation of Equals(A other). So the code incorrectly declares the two objects to be equal.

public class Base : IEquatable<Base> // Noncompliant
{
    public int BaseClassProperty { get; set; }

    public virtual bool Equals(Base other)
    {
        if (other == null) { return false; }

        return BaseClassProperty == other.BaseClassProperty;
    }

    public override bool Equals(object obj) => Equals(obj as Base);
}

public class A : Base, IEquatable<A>
{
    public int ClassAProperty { get; set; }

    public virtual bool Equals(A other)
    {
        if (other == null) { return false; }
        // do comparison of A properties
        return base.Equals(other) && ClassAProperty == other.ClassAProperty;
    }

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

public class B : Base, IEquatable<B>
{
    public int ClassBProperty { get; set; }
    public virtual bool Equals(B other)
    {
        if (other == null) { return false; }
        // do comparison of B properties
        return base.Equals(other) && ClassBProperty == other.ClassBProperty;
    }

    public override bool Equals(object obj) => Equals(obj as B);
}

static class Program
{
    static void Main(string[] args)
    {
        A a = new A() { BaseClassProperty = 1, ClassAProperty = 10 };
        B b = new B() { BaseClassProperty = 1, ClassBProperty = 20 };
        Console.WriteLine(a.Equals(b));
    }
}