From ea84011da2848032433bb8841abc1a9e5d5e2bb0 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 13 Nov 2018 13:19:12 -0500 Subject: [PATCH] go/analysis/passes/printf: fix some pointer false positives fmt's godoc reads: For compound objects, the elements are printed using these rules, recursively, laid out like this: struct: {field0 field1 ...} array, slice: [elem0 elem1 ...] maps: map[key1:value1 key2:value2 ...] pointer to above: &{}, &[], &map[] That is, a pointer to a struct, array, slice, or map, can be correctly printed by fmt if the type pointed to can be printed without issues. vet was only following this rule for pointers to structs, omitting arrays, slices, and maps. Fix that, and add tests for all the combinations. This change was originally made to cmd/vet in https://go-review.googlesource.com/c/147758 Updates #27672. Change-Id: I7e25ecaeed619ae8b6ada79bccacba6b67171733 Reviewed-on: https://go-review.googlesource.com/c/149318 Reviewed-by: Michael Matloob --- go/analysis/passes/printf/testdata/src/a/a.go | 26 +++++++++++++++++++ go/analysis/passes/printf/types.go | 19 ++++++++++---- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/go/analysis/passes/printf/testdata/src/a/a.go b/go/analysis/passes/printf/testdata/src/a/a.go index b73d1cad..40e8f2f7 100644 --- a/go/analysis/passes/printf/testdata/src/a/a.go +++ b/go/analysis/passes/printf/testdata/src/a/a.go @@ -654,6 +654,32 @@ func dbg(format string, args ...interface{}) { fmt.Printf(format, args...) } +func PointersToCompoundTypes() { + stringSlice := []string{"a", "b"} + fmt.Printf("%s", &stringSlice) // not an error + + intSlice := []int{3, 4} + fmt.Printf("%s", &intSlice) // want `Printf format %s has arg &intSlice of wrong type \*\[\]int` + + stringArray := [2]string{"a", "b"} + fmt.Printf("%s", &stringArray) // not an error + + intArray := [2]int{3, 4} + fmt.Printf("%s", &intArray) // want `Printf format %s has arg &intArray of wrong type \*\[2\]int` + + stringStruct := struct{ F string }{"foo"} + fmt.Printf("%s", &stringStruct) // not an error + + intStruct := struct{ F int }{3} + fmt.Printf("%s", &intStruct) // want `Printf format %s has arg &intStruct of wrong type \*struct{F int}` + + stringMap := map[string]string{"foo": "bar"} + fmt.Printf("%s", &stringMap) // not an error + + intMap := map[int]int{3: 4} + fmt.Printf("%s", &intMap) // want `Printf format %s has arg &intMap of wrong type \*map\[int\]int` +} + // Printf wrappers from external package func externalPackage() { b.Wrapf("%s", 1) // want "Wrapf format %s has arg 1 of wrong type int" diff --git a/go/analysis/passes/printf/types.go b/go/analysis/passes/printf/types.go index 701d08be..28d94e1f 100644 --- a/go/analysis/passes/printf/types.go +++ b/go/analysis/passes/printf/types.go @@ -97,12 +97,21 @@ func matchArgTypeInternal(pass *analysis.Pass, t printfArgType, typ types.Type, 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 matchStructArgType(pass, t, str, arg, inProgress) + + under := typ.Elem().Underlying() + switch under.(type) { + case *types.Struct: // see below + case *types.Array: // see below + case *types.Slice: // see below + case *types.Map: // see below + default: + // Check whether the rest can print pointers. + return t&argPointer != 0 } - // Check whether the rest can print pointers. - return t&argPointer != 0 + // If it's a pointer to a struct, array, slice, or map, that's + // equivalent in our analysis to whether we can print the type + // being pointed to. + return matchArgTypeInternal(pass, t, under, arg, inProgress) case *types.Struct: return matchStructArgType(pass, t, typ, arg, inProgress)