Proposal: value matcher interface in is.Equal

[breml]

I sometimes have cases, where I would like to alter or extend the way is.Equal is deciding, if the values are considered equal. After trying several approaches, I came up with the following solution, which does not extend the current API surface, but adds a maximum of flexibility and freedom to the user in regards to how is.Equal decides if two inputs are equal.

The proposal is to value a newly defined matcher interface, which is defined as follows (the matcher interface does not need to become part of the public API of github.com/matryer/is, in fact it can just be defined inline where needed):

type matcher interface{
	Match(interface{}) bool
}

The actual change, that I am proposing is, to extend the existing areEqual function like this (lines 9-11):

// areEqual gets whether a equals b or not.
func areEqual(a, b interface{}) bool {
	if isNil(a) && isNil(b) {
		return true
	}
	if isNil(a) || isNil(b) {
		return false
	}
	if matcher, ok := a.(interface{ Match(interface{}) bool }); ok {
		return matcher.Match(b)
	}
	if reflect.DeepEqual(a, b) {
		return true
	}
	aValue := reflect.ValueOf(a)
	bValue := reflect.ValueOf(b)
	return aValue == bValue
}

In theory, this change does alter the working of is.Equal. In practice, I assume the chances of this change having negative side effects for users of this package to be extremely low. Only users, that use this package to compare types, that implement this exact interface would be affected. I rate the profit of this change to be way higher than the risk of negative side effects.
If you think, that the current signature (Match(interface{}) bool) is too likely to cause problems, the name of the method can easily be altered such that the chance of a collision become negligible (e.g. MatRyerIsMatch(interface{}) bool 😜).

If I find acceptance for this proposal, I am happy to provide the necessary PR to update the code, the tests and the documentation.

---

As a side note, the areEqual function could be minimally simplified by replacing:

	if isNil(a) && isNil(b) {
		return true
	}
	if isNil(a) || isNil(b) {
		return false
	}

with

	if isNil(a) || isNil(b) {
		return isNil(a) && isNil(b)
	}

[matryer]

This is an interesting idea. Perhaps IsEqual would be a hint that it's for testing? do I wonder though if it's easier to just have people prepare the values beforehand in the test code,

complexType1 := getComplexType(1)
complexType2 := getComplexType(2)
complexType1TestStr := fmt.Sprintf("%s: %s", complexType1.ImportantField1, complexType1.ImportantField2)
complexType2TestStr := fmt.Sprintf("%s: %s", complexType2.ImportantField1, complexType2.ImportantField2)
is.Equal(complexType1TestStr, complexType2TestStr)

This makes the code more verbose, but very easy to see what's going on. Otherwise, you'll have to know that there's a special interface, and it might look like magic just passing these types directly into is.Equal.

What do you think of that?

[breml]

Hi @matryer
Thanks for having a look at this.

I can see, that in a lot of cases, the "prepare the values" approach can work. But there are cases, where this falls short, e.g. if the comparison depends on private fields, since these fields would then not necessarily be accessible.

Additionally, I personally don't like if I have to "massage" the returned values of the function under test before I can compare it with the expected value. I prefer to have the following structure

// Initialization (e.g. mocks)

// Call function under test

// Assert the returned values with the expected values from the test table

If I need to post-process the returned values after the call to the "function under test" it makes reading the test harder and it adds more possibilities for subtle errors. My goal is to keep the test logic as simple as possible (e.g. having an if-clause in the tests I consider a code-smell).

If there is a Match, Equal or IsEqual method on the type that I want to assert for, I can then again write regular tests to make sure, that this method works as intended. Whereas if this logic is buried in the test it self, I will not write a test to test the test.

With all of this said, I would greatly prefer to have is supporting such a matcher, equaler or isEqualer interface. In regards to the actual name of the interface and the method, I do not have a clear favorite, the functionality, that is provided is more important to me.