rspec/rules/S4035/csharp/rule.adoc

== Why is this an issue?

When a class implements the `IEquatable<T>` interface, it enters a contract that, in effect, states "I know how to compare two instances of type T or any type derived from T for equality.". However if that class is derived, it is very unlikely that the base class will know how to make a meaningful comparison. Therefore that implicit contract is now broken.


Alternatively `IEqualityComparer<T>` provides a safer interface and is used by collections or `Equals` could be made `virtual`.


This rule raises an issue when an unsealed, `public` or `protected` class implements `IEquitable<T>` and the `Equals` is neither `virtual` nor `abstract`.


=== Noncompliant code example

[source,csharp]
----
using System;

namespace MyLibrary
{
  public class Base : IEquatable<Base> // Noncompliant
  {
    public bool Equals(Base other) 
    {
      if (other == null) { return false; }
      // do comparison of base properties
      return true;
    }

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

  class A : Base 
  {
    public bool Equals(A other) 
    {
      if (other == null) { return false; }
      // do comparison of A properties
      return base.Equals(other);
    }

    public override bool Equals(object other)  => Equals(other as A); 
  }

  class B : Base 
  {
    public bool Equals(B other) 
    {
      if (other == null) { return false; }
      // do comparison of B properties
      return base.Equals(other); 
    }

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

  internal class Program
  {
    static void Main(string[] args)
    {
        A a = new A();
        B b = new B();
         Console.WriteLine(a.Equals(b)); // This calls the WRONG equals. This causes Base.Equals(Base)
                                         // to be called which only compares the properties in Base and ignores the fact that 
                                         // a and b are different types. In the working example A.Equals(Object) would have been 
                                         // called and Equals would return false because it correctly recognizes that a and b are 
                                         // different types. If a and b have the same base properties they will be returned as equal. 
    }
  }
}
----


=== Compliant solution

[source,csharp]
----
using System;

namespace MyLibrary
{
    public sealed class Foo : IEquatable<Foo>
    {
        public bool Equals(Foo other) 
        {
            // Your code here        
        }
    }
}
----


== Resources

https://msdn.microsoft.com/en-us/library/ms132151(v=vs.110).aspx[IEqualityComparer<T> Interface]


ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Seal class "XXX" or implement "IEqualityComparer<T>" instead.


=== Highlighting

Class declaration


'''
== Comments And Links
(visible only on this page)

=== on 21 Jun 2017, 11:26:00 Amaury Levé wrote:
We decided not to recommend sealing the class when implementing ``++Equals(object)++`` as it is possible for sub-classes to change the behavior. Besides, we also added an exception to the rule to say we don't report if the ``++Equals(T)++`` is ``++virtual++``.

endif::env-github,rspecator-view[]