Skip to content

don't panic on unexported struct key in map #1816

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+61 −3
Open

don't panic on unexported struct key in map #1816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07088d8
don't panic on unexported struct key in map
HaraldNordgren Oct 25, 2025
4462478
Update assert/assertions.go
HaraldNordgren Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions assert/assertions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1928,6 +1928,23 @@ func typeAndKind(v interface{}) (reflect.Type, reflect.Kind) {
return t, k
}

func isStructWithUnexportedMapField(t reflect.Type) bool {
if t.Kind() != reflect.Struct {
return false
}

for i := 0; i < t.NumField(); i++ {
field := t.Field(i)
if !field.IsExported() &&
field.Type.Kind() == reflect.Map &&
field.Type.Key().Kind() == reflect.Array {
return true
}
}

return false
}
Comment on lines +1931 to +1946
Copy link
Collaborator

@ccoVeille ccoVeille Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the code the discussion that happened here

And based on the code of go-spew

#1816 (comment)

Copy link
Collaborator

@ccoVeille ccoVeille Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like started writting something, but I posted before putting all together.

So here, I was spotting the code here limited to unexported field that are maps with 2 elements, but go-spew code would panic when dealing with Interface and here, maybe a better fix would be to use CanInterface and check nothing else.

I'm unsure what other code examples could panic, but maybe it could be considered as a better approach than the "recover" I suggested.

Copy link
Collaborator

@ccoVeille ccoVeille Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this might not be enough.

I checked at go-spew code while working on https://github.com/stretchr/testify/pull/1824/files

I feel like a code like this won't be caught by your fix

type Bar struct {
	  m map[[1]byte]*struct{}
}

type Foo struct {
  Foo Bar
}

When using a Foo, the problem is wider that simply recursively parsing the fields, as you need to also go through the whole stack, check for the pointer, the slices, maps, and arrays

So have something that goes as deep as go-spew.


// diff returns a diff of both values as long as both are of the same type and
// are a struct, map, slice, array or string. Otherwise it returns an empty string.
func diff(expected interface{}, actual interface{}) string {
Expand All @@ -1948,13 +1965,16 @@ func diff(expected interface{}, actual interface{}) string {

var e, a string

switch et {
case reflect.TypeOf(""):
switch {
case et == reflect.TypeOf(""):
e = reflect.ValueOf(expected).String()
a = reflect.ValueOf(actual).String()
case reflect.TypeOf(time.Time{}):
case et == reflect.TypeOf(time.Time{}):
e = spewConfigStringerEnabled.Sdump(expected)
a = spewConfigStringerEnabled.Sdump(actual)
case isStructWithUnexportedMapField(et):
e = fmt.Sprintf("%+v", expected)
a = fmt.Sprintf("%+v", actual)
default:
e = spewConfig.Sdump(expected)
a = spewConfig.Sdump(actual)
Expand Down
38 changes: 38 additions & 0 deletions assert/assertions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,10 @@ func TestEqual(t *testing.T) {
mockT := new(testing.T)
var m map[string]interface{}

type unexportedKeyType struct {
m map[[1]byte]*struct{}
}

cases := []struct {
expected interface{}
actual interface{}
Expand All @@ -613,6 +617,40 @@ func TestEqual(t *testing.T) {
{m["bar"], "something", false, ""},
{myType("1"), myType("2"), false, ""},

// Test cases for struct with unexported map field containing array keys
{
unexportedKeyType{
map[[1]byte]*struct{}{
{1}: nil,
{2}: nil,
},
},
unexportedKeyType{
map[[1]byte]*struct{}{
{1}: nil,
{2}: nil,
},
},
true,
"",
},
{
unexportedKeyType{
map[[1]byte]*struct{}{
{1}: {},
{2}: {},
},
},
unexportedKeyType{
map[[1]byte]*struct{}{
{3}: {},
{4}: {},
},
},
false,
"structs with unexported map fields using array keys are not equal when keys differ",
},

// A case that might be confusing, especially with numeric literals
{10, uint(10), false, ""},
}
Expand Down