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

stringer not used for map keys #141

Closed
crawshaw opened this issue May 25, 2019 · 1 comment · Fixed by #142
Closed

stringer not used for map keys #141

crawshaw opened this issue May 25, 2019 · 1 comment · Fixed by #142

Comments

@crawshaw
Copy link
Contributor

When rendering type T, the cmp package uses the String method for slice values, but not for map keys.

package main

import (
	"fmt"

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

type T [5]byte

func (t T) String() string { return string(t[:]) }

func main() {
	var t T
	copy(t[:], "world")
	m := map[T]string{t: "val"}

	s := []T{t}

	fmt.Println(cmp.Diff(nil, m))
	fmt.Println(cmp.Diff(nil, s))
}

Outputs:

  interface{}(
+ 	map[main.T]string{{0x77, 0x6f, 0x72, 0x6c, 0x64}: "val"},
  )

  interface{}(
+ 	[]main.T{s"world"},
  )

Try it here: https://play.golang.org/p/GS8f7ABvd61

If you agree this is a bug, I can take a crack at fixing it.

@dsnet
Copy link
Collaborator

dsnet commented May 26, 2019

Not calling String on keys was probably an oversight. Feel free to fix it.

It seems that enabling this is as trivial as removing this line:

opts.AvoidStringer = true

However, I can't remember why I would have set that explicitly to true.

crawshaw added a commit to crawshaw/go-cmp that referenced this issue May 26, 2019
This reverts a change introduced in commit 2940eda where the
cmp package stopped calling the String method when printing map
keys. The motivation for the change is unclear, indeed there was
a pre-existing test that String was called on map keys that the
commit changed.

Fixes google#141
dsnet pushed a commit that referenced this issue May 27, 2019
This reverts a change introduced in commit 2940eda where cmp 
stopped calling the String method when printing map keys.

Fixes #141
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

Successfully merging a pull request may close this issue.

2 participants