From 797431e1a314e09984500e2425acb8e4fd4854f2 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Fri, 2 Aug 2013 22:52:38 +1000 Subject: [PATCH] go.tools/cmd/vet: fix printf analysis for structs. The old code only got it right for Stringers (etc.) and a few other simple cases. But the rule used by fmt.Printf for non-Stringers is that pointers to structs print as pointers, the rest must satisfy the format verb element-wise. Thus for example struct {a int, b []byte} prints with %d and %q (sic) but not %g. R=golang-dev, dsymonds CC=golang-dev https://golang.org/cl/12340043 --- cmd/vet/testdata/print.go | 38 +++++++++++++++++++++++++++++++++++++- cmd/vet/types.go | 29 ++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/cmd/vet/testdata/print.go b/cmd/vet/testdata/print.go index a703825d..38d1f0ec 100644 --- a/cmd/vet/testdata/print.go +++ b/cmd/vet/testdata/print.go @@ -150,6 +150,13 @@ func PrintfTests() { f.Warnf(0, "%r", "hello") // ERROR "unrecognized printf verb" f.Warnf(0, "%#s", "hello") // ERROR "unrecognized printf flag" Printf("d%", 2) // ERROR "missing verb at end of format string in Printf call" + Printf("%d", percentDV) + Printf("%d", &percentDV) + Printf("%d", notPercentDV) // ERROR "arg notPercentDV for printf verb %d of wrong type" + Printf("%d", ¬PercentDV) // ERROR "arg ¬PercentDV for printf verb %d of wrong type" + Printf("%p", ¬PercentDV) // Works regardless: we print it as a pointer. + Printf("%s", percentSV) + Printf("%s", &percentSV) // Good argument reorderings. Printf("%[1]d", 3) Printf("%[1]*d", 3, 1) @@ -214,7 +221,9 @@ func (*stringer) Warnf(int, string, ...interface{}) string { return "warnf" } -type notstringer struct{} +type notstringer struct { + f float64 +} var notstringerv notstringer @@ -233,3 +242,30 @@ var notstringerarrayv notstringerarray var nonemptyinterface = interface { f() }(nil) + +// A data type we can print with "%d". +type percentDStruct struct { + a int + b []byte + c *float64 +} + +var percentDV percentDStruct + +// A data type we cannot print correctly with "%d". +type notPercentDStruct struct { + a int + b []byte + c bool +} + +var notPercentDV notPercentDStruct + +// A data type we can print with "%s". +type percentSStruct struct { + a string + b []byte + c stringerarray +} + +var percentSV percentSStruct diff --git a/cmd/vet/types.go b/cmd/vet/types.go index 624f71af..d017e1df 100644 --- a/cmd/vet/types.go +++ b/cmd/vet/types.go @@ -111,7 +111,7 @@ func (f *File) matchArgType(t printfArgType, typ types.Type, arg ast.Expr) bool return true // %s matches []byte } // Recur: []int matches %d. - return t&argPointer != 0 || f.matchArgType(t, typ.Elem(), arg) + return t&argPointer != 0 || f.matchArgType(t, typ.Elem().Underlying(), arg) case *types.Slice: // Same as array. @@ -119,7 +119,7 @@ func (f *File) matchArgType(t printfArgType, typ types.Type, arg ast.Expr) bool return true // %s matches []byte } // Recur: []int matches %d. - return t&argPointer != 0 || f.matchArgType(t, typ.Elem(), arg) + return t&argPointer != 0 || f.matchArgType(t, typ.Elem().Underlying(), arg) case *types.Pointer: // Ugly, but dealing with an edge case: a known pointer to an invalid type, @@ -130,7 +130,19 @@ func (f *File) matchArgType(t printfArgType, typ types.Type, arg ast.Expr) bool } return true // special case } - return t&argPointer != 0 + // If it's actually a pointer with %p, it prints as one. + if t == argPointer { + return true + } + // If it's pointer to struct, that's equivalent in our analysis to whether we can print the struct. + if str, ok := typ.Elem().Underlying().(*types.Struct); ok { + return f.matchStructArgType(t, str, arg) + } + // The rest can print with %p as pointers, or as integers with %x etc. + return t&(argInt|argPointer) != 0 + + case *types.Struct: + return f.matchStructArgType(t, typ, arg) case *types.Interface: // If the static type of the argument is empty interface, there's little we can do. @@ -196,6 +208,17 @@ func (f *File) matchArgType(t printfArgType, typ types.Type, arg ast.Expr) bool return false } +// matchStructArgType reports whether all the elements of the struct match the expected +// type. For instance, with "%d" all the elements must be printable with the "%d" format. +func (f *File) matchStructArgType(t printfArgType, typ *types.Struct, arg ast.Expr) bool { + for i := 0; i < typ.NumFields(); i++ { + if !f.matchArgType(t, typ.Field(i).Type(), arg) { + return false + } + } + return true +} + // numArgsInSignature tells how many formal arguments the function type // being called has. func (f *File) numArgsInSignature(call *ast.CallExpr) int {