PropertiesComparer doesn't work well with records

[Dreamescaper]

record C1
{
    public string Name { get; set; }
    public int[] Collection { get; set; }
}

var v1 = new C1 { Name = "1", Collection = [1, 2, 3] };
var v2 = new C1 { Name = "1", Collection = [1, 2, 3] };

Assert.That(v1, Is.EqualTo(v2).UsingPropertiesComparer());

The assertion above fails, even thouht the same code works fine if I replace "record" with "class".
I assume that the issue is that record is IEquatable, therefore comparer doesn't attempt to compare it by properties.

I would suggest to ignore IEquatable for records in case UsingPropertiesComparer is used, and still compare it property by property.

[stevenaw]

Thanks for identifying this and already proposing a fix @Dreamescaper ! This feels like a bug to me as well

[manfred-brands]

It depends. The PropertiesComparer deliberately thinks that IEquatable is a better implementation than using properties.
E.g. if your IEquatable does a case-insensitive compare.
Dropping that would break those cases.

If you are not happy with the IEquatable implementation generated for record than you should override it.

[Dreamescaper]

If you are not happy with the IEquatable implementation generated for record than you should override it.

With such approach I could ignore PropertiesComparer altogether, and implement IEquatable for whatever type I need. But I don't want to do that, since I need that for testing only, and I would expect my testing framework to help me with that.

[Dreamescaper]

Btw, I can see that FluentAssertions follows this approach with their IsEquivalent:

The default behavior is to treat every type that overrides Object.Equals as an object that was designed to have value semantics. Anonymous types, records, record structs and tuples also override this method, but because the community proved us that they use them quite often in equivalency comparisons, we decided to always compare them by their members.

[manfred-brands]

With such approach I could ignore PropertiesComparer altogether, and implement IEquatable for whatever type I need. But I don't want to do that, since I need that for testing only, and I would expect my testing framework to help me with that.

I agree not implementing IEquatable. We do so for our Dto objects where we only need to compare them in NUnit tests.
A pity we can't have "slim record" where we get the properties, but not the Equatable.
And for testing we need value semantics and not the default reference semantics.

I'm happy with your approach checking for an CompilerGenerated version and ignore that, within the comment I added to the PR.
This does allow the user to create a better version if so inclined.

[Dreamescaper]

I'm happy with your approach checking for an CompilerGenerated version and ignore that, within the comment I added to the PR.
This does allow the user to create a better version if so inclined.

Ok, so the idea is not to check whether this type is a record or not, but whether it has a compiler generated Equals method.
And if this record has a user-defined Equals method - we should use it instead of per-property comparison.

That totally makes sense. I already do that, but treated it like an implementation detail.

[Dreamescaper]

What do you think is the expected behavior for ValueTuples?
I.e. should the following test pass or fail?

var i1 = (Name: "123", Collection: new [] {1, 2, 3});
var i2 = (Name: "123", Collection: new [] {1, 2, 3});
Assert.That(i1, Is.EqualTo(i2).UsingPropertiesComparer());

[manfred-brands]

What do you think is the expected behavior for ValueTuples? I.e. should the following test pass or fail?

var i1 = (Name: "123", Collection: new [] {1, 2, 3});
var i2 = (Name: "123", Collection: new [] {1, 2, 3});
Assert.That(i1, Is.EqualTo(i2).UsingPropertiesComparer());

I would say that it should pass.
It does in the released version. Didn't test your branch.
In the released version if would go into the ValueTupleComparer which checks its properties.

[Dreamescaper]

I would say that it should pass. It does in the released version. Didn't test your branch. In the released version if would go into the ValueTupleComparer which checks its properties.

Yeah, my bad. So it works independently of PropertiesComparer.