Inherited classes Equals ignored when called from the base class

[christianrondeau]

First of all, great library, thanks for making it!

Problem

When calling .Equals() or System.Collections.Generic.EqualityComparer on a base class that uses [Equatable], the generated Equals implementation will not call inherited Equals implementations.

• When the base class doesn't use [Equatable], reference equality is used (which is not what we want)
• When the base class uses [Equatable], the base class only checks it's own fields and ignores inherited types fields.

Reproduction

public class GeneratorBaseClassBugTests
{
  [Equatable]
  public partial class MyContainer
  {
	  public MyBase Content { get; set; }
  }
  
  [Equatable]
  public abstract partial class MyBase
  {
  }
  
  [Equatable]
  public partial class MyImpl : MyBase
  {
	  public int Value { get; set; }
  }

  [Test]
  public void Test()
  {
    var v1 = new MyContainer { Content = new MyImpl { Value = 1 } };
    var v2 = new MyContainer { Content = new MyImpl { Value = 2 } };

    Assert.That(v1, Is.Not.EqualTo(v2));
  }
}

Solution

This code is generated in the container:

global::System.Collections.Generic.EqualityComparer<global::MyNamespace.MyBase>.Default.Equals(Content!, other.Content!)

This shorts-circuits the concrete type by directly checking the base type. This would work:

global::System.Object.Equals(Content!, other.Content!)

Version

.NET 6.0, Generator.Equals 2.6.0

Thanks!

[diegofrata]

This is definitely a bug, thanks for raising it.

[diegofrata]

I think we still want to keep the EqualityComparer for value types as to avoid unnecessary boxing. So use Object.Equals only for reference types.

I might have some time this week to look at this, unless you'd like to have a go at it yourself and send a PR!

Let me know!

[christianrondeau]

Yes, that sounds right!

I just started playing with generators yesterday (they are great, I should have used them earlier), and I'm swamped right now so I wouldn't be able to make a PR in the next two weeks. However if the issue is still open early next year, I'll definitely try my hand at it :)

Thanks for responding!

[diegofrata]

I looked into this issue this morning, I think the fix is a bit more involved. Basically, all the comparers use EqualityComparer.Default for the element comparison -- these need also be fixed, my view is that we create another comparer, let's say VirtualEqualityComparer<T> (probably needs better naming).

In this comparison, at the static ctor, we check whether the type is a reference type && not sealed, if so, then on Equals/Hashcode operations we call the Object methods, else, we call the EqualityComparer methods.

@ssttgg does it sound good to you?

[diegofrata]

@christianrondeau this should be fixed now in 2.7.2.

[christianrondeau]

Wow that's great, before the end of the holidays even! Hope you are having a great time, and thanks!