String formatting for Path produces empty strings when it shouldn't

[kernel-panic96]

I have a use case where I compare very nested protobuf code generated structures. The behaviour is that if a structure has an Equals() method, the library stops recursing. To bypass that I convert the protobuf structures to json and back into very very nested map[string]interface{}`s. When I report a particular Diff I would like it to appear with a key - rootType.Level1Field.Level2Field.Level3Field.Leaf or something similar. I cannot do that because the String() implementation does not work with nested maps or slices. I have to resort to fmt.Sprintf("%#v", path) which for nested code generated structures is very verbose. E.g. root["period"].(map[string]interface {})["endsAt"].(map[string]interface {})["value"].(string), this is an actual output for 3 levels of nesting (which isn't that much nesting).

https://github.com/google/go-cmp/blob/master/cmp/path.go#L93

// String returns the simplified path to a node.
// The simplified path only contains struct field accesses.
//
// For example:
//
//	MyMap.MySlices.MyField
func (pa Path) String() string {
	var ss []string
	for _, s := range pa {
		if _, ok := s.(StructField); ok {
			ss = append(ss, s.String())
		}
	}
	return strings.TrimPrefix(strings.Join(ss, ""), ".")
}

There's a type assertion that only allows struct field accesses, for a reason unknown to me. If that's necessary at the very least the documentation is misleading because it suggests that it works on maps and slices - // MyMap.MySlices.MyField

The way I bypassed this is by implementing a custom stringer func

func pathStringer(p cmp.Path) string {
	var ss []string

	if len(p) > 0 { // the root node
		ss = append(ss, "{"+p[0].Type().String()+"}")
	}

	for _, step := range p {
		switch step.(type) {
		case cmp.StructField, cmp.SliceIndex, cmp.Transform, cmp.MapIndex:
			ss = append(ss, step.String())
		}
	}
	return strings.Join(ss, "")
}

Ideally I shouldn't resort to that and I would like it to just work as described in the doc string.

[kernel-panic96]

PS: I don't know what Transform is (from the snippet), and whether it's necessary

[dsnet]

This is working as intended.

Having Path.String only print the struct field accesses was perhaps a mistake, but it's clearly documented as "only [containing] struct field accesses." I don't think we can change the behavior now.

On the other hand, the Path.GoString prints a valid Go expression to get from the root value to the leaf. It may be verbose, but mainly because the Go language itself is verbose in this regard. There is a minor improvement where we can replace interface{} with any.

Writing your own Path formatter seems like the right thing to do.

[kernel-panic96]

I agree, about the documentation. I glazed over it. Please consider this as a feature request in that case. To me it's a bit incoherent that the library will happily compare and successfully find differences inside nested native collections but won't report them properly.

[dsnet]

Given that Path.String and Path.GoString already exist, I'm hesitant to add another method to convert the Path into a string.

library will happily compare and successfully find differences inside nested native collections but won't report them properly.

The GoString method does report them properly. It's String that's problematic, but that can't be changed now.

[kernel-panic96]

I was thinking more of changing the implementation of String(). I won't push further, but that most likely is a backwards compatible change. It would only extend the functionality. Close the issue at your own discretion. The use case isn't only structures consisted solely of native collections, but also structs with embedded native collections, which should be a much larger set.

[dsnet]

Closing as "working as intended".