Skip to content

Commit

Permalink
Catch panics when calling GoString like fmt %#v does (#87)
Browse files Browse the repository at this point in the history
This handles a few cases (similar to how fmt %#v does):

- A GoString method on a value receiver, called with a nil pointer
- A GoString method on a pointer receiver that doesn't check for nil
- A GoString method that panics in some other way

Because Go 1.17 added a method Time.GoString with value receiver, this
broke structs that had *time.Time fields with nil values (which is
common!).

Also added a bunch of tests for these cases.

Fixes #77

Co-authored-by: Jordan Barrett <jordan.barrett@canonical.com>
  • Loading branch information
benhoyt and barrettj12 committed Aug 25, 2022
1 parent d928460 commit d8c7eb1
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 1 deletion.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -2,3 +2,4 @@
_go*
_test*
_obj
/.idea
19 changes: 19 additions & 0 deletions formatter.go
Expand Up @@ -92,6 +92,24 @@ type visit struct {
typ reflect.Type
}

func (p *printer) catchPanic(v reflect.Value, method string) {
if r := recover(); r != nil {
if v.Kind() == reflect.Ptr && v.IsNil() {
writeByte(p, '(')
io.WriteString(p, v.Type().String())
io.WriteString(p, ")(nil)")
return
}
writeByte(p, '(')
io.WriteString(p, v.Type().String())
io.WriteString(p, ")(PANIC=calling method ")
io.WriteString(p, strconv.Quote(method))
io.WriteString(p, ": ")
fmt.Fprint(p, r)
writeByte(p, ')')
}
}

func (p *printer) printValue(v reflect.Value, showType, quote bool) {
if p.depth > 10 {
io.WriteString(p, "!%v(DEPTH EXCEEDED)")
Expand All @@ -101,6 +119,7 @@ func (p *printer) printValue(v reflect.Value, showType, quote bool) {
if v.IsValid() && v.CanInterface() {
i := v.Interface()
if goStringer, ok := i.(fmt.GoStringer); ok {
defer p.catchPanic(v, "GoString")
io.WriteString(p, goStringer.GoString())
return
}
Expand Down
38 changes: 37 additions & 1 deletion formatter_test.go
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"strings"
"testing"
"time"
"unsafe"
)

Expand Down Expand Up @@ -97,7 +98,7 @@ var gosyntax = []test{
`[]string{"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"}`,
},
{F(5), "pretty.F(5)"},
{ NewStructWithPrivateFields("foo"), `NewStructWithPrivateFields("foo")`},
{NewStructWithPrivateFields("foo"), `NewStructWithPrivateFields("foo")`},
{
SA{&T{1, 2}, T{3, 4}},
`pretty.SA{
Expand Down Expand Up @@ -176,6 +177,41 @@ var gosyntax = []test{
},
}`,
},
{(*time.Time)(nil), "(*time.Time)(nil)"},
{&ValueGoString{"vgs"}, `VGS vgs`},
{(*ValueGoString)(nil), `(*pretty.ValueGoString)(nil)`},
{(*VGSWrapper)(nil), `(*pretty.VGSWrapper)(nil)`},
{&PointerGoString{"pgs"}, `PGS pgs`},
{(*PointerGoString)(nil), "(*pretty.PointerGoString)(nil)"},
{&PanicGoString{"oops!"}, `(*pretty.PanicGoString)(PANIC=calling method "GoString": oops!)`},
}

type ValueGoString struct {
s string
}

func (g ValueGoString) GoString() string {
return "VGS " + g.s
}

type VGSWrapper struct {
ValueGoString
}

type PointerGoString struct {
s string
}

func (g *PointerGoString) GoString() string {
return "PGS " + g.s
}

type PanicGoString struct {
s string
}

func (g *PanicGoString) GoString() string {
panic(g.s)
}

func TestGoSyntax(t *testing.T) {
Expand Down

0 comments on commit d8c7eb1

Please sign in to comment.