Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmpopts.IgnoreFields does not work on structs with an Equal method #352

Open
TeodorPt opened this issue Feb 1, 2024 · 2 comments
Open

Comments

@TeodorPt
Copy link

TeodorPt commented Feb 1, 2024

Summary

If a struct has a method of the form (T) Equal(T) bool, cmp.Equal is using that instead of the default evaluation process, as documented in the Godoc.

If one wants to use options such as cmpopts.IgnoreFields, they are ignored, because the rule above takes precedence.

In some situations (e.g. in tests), it might be useful to be able to ignore fields even on structs that have an Equal method.

Example

package main

import (
	"fmt"

	"github.com/google/go-cmp/cmp"
	"github.com/google/go-cmp/cmp/cmpopts"
)

type Entry struct {
	ID   string
	Name string
}

func (e Entry) Equal(other Entry) bool {
	return e.ID == other.ID && e.Name == other.Name
}

func main() {
	a := Entry{
		ID:   "123",
		Name: "EntryA",
	}
	b := Entry{
		ID:   "124",
		Name: "EntryA",
	}

	// Returns false, as expected.
	fmt.Println(cmp.Equal(a, b))

	// Also returns false, though one might expect true from reading this statement.
	fmt.Println(cmp.Equal(a, b, cmpopts.IgnoreFields(Entry{}, "ID")))
}

Suggestion

An idea would be to provide an additional Option to ignore the struct's Equal method even if it exists.
Please let me know if there's already a way to bypass this that I might have missed.

Thanks!

@dsnet
Copy link
Collaborator

dsnet commented Feb 1, 2024

Hi, thanks for the issue report. It may make sense to provide an option to ignore the Equal method.

In the mean time you can do something like:

type EntryNoMethods Entry // type definitions in Go drop methods of the parent
cmp.Equal(...,
    // Transform Entry into EntryNoMethods to avoid calling the equal method.
    cmp.Transformer("", func(e Entry) EntryNoMethods { return EntryNoMethods(e) }),
    // Ignore the ID field on post-transformation EntryNoMethods type.
    cmpopts.IgnoreFields(EntryNoMethods{}, "ID"),
)

@StevenACoffman
Copy link

StevenACoffman commented Sep 28, 2024

@dsnet I ran across this suprising issue when I tried to use cmp inside an Equal method.

func (m *UserDistrictInfo) Equal(y *UserDistrictInfo) bool {
	if y == nil {
		return m == y
	}
	//type NoMethods UserDistrictInfo // type definitions in Go drop methods of the parent
	//methodRemover := func(e *UserDistrictInfo) *NoMethods { return (*NoMethods)(e) }
	return cmp.Equal(m, y,
		// Transform Entry into NoMethods to avoid calling the equal method.
		//cmp.Transformer("", methodRemover),
		cmp.AllowUnexported(UserDistrictInfo{}),
		// Ignore the fields on post-transformation NoMethods type.
		cmpopts.IgnoreFields(UserDistrictInfo{}, "DataflowExportData", "ModificationTimestamp", "CreatedAt", "UpdatedAt"),
	)
}

will hang forever but this:

func (m *UserDistrictInfo) Equal(y *UserDistrictInfo) bool {
	if y == nil {
		return m == y
	}
	type NoMethods UserDistrictInfo // type definitions in Go drop methods of the parent
	methodRemover := func(e *UserDistrictInfo) *NoMethods { return (*NoMethods)(e) }
	return cmp.Equal(m, y,
		// Transform Entry into NoMethods to avoid calling the equal method.
		cmp.Transformer("", methodRemover),
		cmp.AllowUnexported(NoMethods{}),
		// Ignore the fields on post-transformation NoMethods type.
		cmpopts.IgnoreFields(NoMethods{}, "DataflowExportData", "ModificationTimestamp", "CreatedAt", "UpdatedAt"),
	)
}

works reliably.

It would be nice to have a cmpopts.IgnoreEqual() option to help document this potential pitfall and make this more generic and succinct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants