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

Ignore doesn't always work for map keys #73

Closed
rogpeppe opened this issue Feb 20, 2018 · 3 comments
Closed

Ignore doesn't always work for map keys #73

rogpeppe opened this issue Feb 20, 2018 · 3 comments

Comments

@rogpeppe
Copy link
Contributor

If there's an Ignore rule that matches a key in a map, I'd expect that key
to be ignored even if it doesn't exist in the map being compared to.

For example, the following code says that the two values aren't equal
even though I've specified that the "x" key should be ignored:

package main

import (
	"fmt"

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

func main() {
	a := map[string]int{
		"x": 1,
		"y": 2,
	}
	b := map[string]int{
		"y": 2,
	}
	fmt.Println(cmp.Diff(a, b, cmp.FilterPath(isKeyX, cmp.Ignore())))
}

func ignorePath(pat string) cmp.Option {
	return cmp.FilterPath(isKeyX, cmp.Ignore())
}

func isKeyX(p cmp.Path) bool {
	step, ok := p[len(p)-1].(cmp.MapIndex)
	return ok && step.Key().String() == "x"
}

It prints:

{map[string]int}["x"]:
	-: 1
	+: <non-existent>

If I add an "x" key (with any value) to b, then they compare equal as expected.

@dsnet
Copy link
Collaborator

dsnet commented Feb 22, 2018

The current behavior is documented on cmp.Equal:

Map keys are equal according to the == operator. To use custom comparisons for map keys, consider using cmpopts.SortMaps.

However, I believe the more general issue at hand is that there is no easy way for ignore to conceptually "remove" elements from a slice or map. #28 was trying to solve the application where the zero value of a map element should be removed.

I don't have a good answer for this :)

@dsnet
Copy link
Collaborator

dsnet commented Feb 22, 2018

See #28 (comment) which contains some ideas I explored and why I believe there were all problematic in some way.

@dsnet
Copy link
Collaborator

dsnet commented Feb 27, 2019

Fixed by #121

@dsnet dsnet closed this as completed Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants