cmd/vet: Use function signature to find format string index.

cmd/vet's printf checker currently uses a hardcoded map of function
names to expected positions of format strings. We can be a bit more
precise than this by looking up the signature of the function, which
helps when libraries implement functions like Errorf or Logf with
extra arguments like log levels or error codes.

Specifically, the format string param is assumed to be the last string
parameter of the called function.

Fixes #12294.

Change-Id: Icf10ebb819bba91fa1c4109301417042901e34c7
Reviewed-on: https://go-review.googlesource.com/20163
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

src/cmd/vet/print.go

@@ -46,24 +46,23 @@ func initPrintFlags() {
 		}
 		name = strings.ToLower(name)
 		if name[len(name)-1] == 'f' {
-			printfList[name] = skip
+			isFormattedPrint[name] = true
 		} else {
 			printList[name] = skip
 		}
 	}
 }
 
-// printfList records the formatted-print functions. The value is the location
+// isFormattedPrint records the formatted-print functions. Names are
-// of the format parameter. Names are lower-cased so the lookup is
+// lower-cased so the lookup is case insensitive.
-// case insensitive.
+var isFormattedPrint = map[string]bool{
-var printfList = map[string]int{
+	"errorf":  true,
-	"errorf":  0,
+	"fatalf":  true,
-	"fatalf":  0,
+	"fprintf": true,
-	"fprintf": 1,
+	"logf":    true,
-	"logf":    0,
+	"panicf":  true,
-	"panicf":  0,
+	"printf":  true,
-	"printf":  0,
+	"sprintf": true,
-	"sprintf": 0,
 }
 
 // printList records the unformatted-print functions. The value is the location
@@ -79,6 +78,34 @@ var printList = map[string]int{
 	"sprint": 0, "sprintln": 0,
 }
 
+// signature returns the types.Signature of a call. If it is unable to
+// identify the call's signature, it can return nil.
+func signature(f *File, call *ast.CallExpr) *types.Signature {
+	typ := f.pkg.types[call.Fun].Type
+	if typ == nil {
+		return nil
+	}
+	sig, _ := typ.(*types.Signature)
+	return sig
+}
+
+// formatIndex returns the index of the format string parameter within
+// a signature. If it cannot find any format string parameter, it
+// returns -1.
+func formatIndex(sig *types.Signature) int {
+	if sig == nil {
+		return -1
+	}
+	idx := -1
+	for i := 0; i < sig.Params().Len(); i++ {
+		p := sig.Params().At(i)
+		if typ, ok := p.Type().(*types.Basic); ok && typ.Kind() == types.String {
+			idx = i
+		}
+	}
+	return idx
+}
+
 // checkCall triggers the print-specific checks if the call invokes a print function.
 func checkFmtPrintfCall(f *File, node ast.Node) {
 	if d, ok := node.(*ast.FuncDecl); ok && isStringer(f, d) {
@@ -109,8 +136,8 @@ func checkFmtPrintfCall(f *File, node ast.Node) {
 	}
 
 	name := strings.ToLower(Name)
-	if skip, ok := printfList[name]; ok {
+	if _, ok := isFormattedPrint[name]; ok {
-		f.checkPrintf(call, Name, skip)
+		f.checkPrintf(call, Name)
 		return
 	}
 	if skip, ok := printList[name]; ok {
@@ -147,12 +174,20 @@ type formatState struct {
 
 // checkPrintf checks a call to a formatted print routine such as Printf.
 // call.Args[formatIndex] is (well, should be) the format argument.
-func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) {
+func (f *File) checkPrintf(call *ast.CallExpr, name string) {
-	if formatIndex >= len(call.Args) {
+	idx := formatIndex(signature(f, call))
+
+	if idx < 0 {
+		f.Badf(call.Pos(), "no formatting directive in %s call", name)
+		return
+	}
+
+	if idx >= len(call.Args) {
 		f.Bad(call.Pos(), "too few arguments in call to", name)
 		return
 	}
-	lit := f.pkg.types[call.Args[formatIndex]].Value
+
+	lit := f.pkg.types[call.Args[idx]].Value
 	if lit == nil {
 		if *verbose {
 			f.Warn(call.Pos(), "can't check non-constant format in call to", name)
@@ -164,7 +199,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) {
 		return
 	}
 	format := constant.StringVal(lit)
-	firstArg := formatIndex + 1 // Arguments are immediately after format string.
+	firstArg := idx + 1 // Arguments are immediately after format string.
 	if !strings.Contains(format, "%") {
 		if len(call.Args) > firstArg {
 			f.Badf(call.Pos(), "no formatting directive in %s call", name)

src/cmd/vet/testdata/print.go

@@ -211,8 +211,10 @@ func PrintfTests() {
 	Log(3)       // OK
 	Log("%d", 3) // ERROR "possible formatting directive in Log call"
 	Logf("%d", 3)
-	Logf("%d", "hi") // ERROR "arg .hi. for printf verb %d of wrong type: untyped string"
+	Logf("%d", "hi") // ERROR "arg .hi. for printf verb %d of wrong type: string"
 
+	Errorf(1, "%d", 3)    // OK
+	Errorf(1, "%d", "hi") // ERROR "arg .hi. for printf verb %d of wrong type: string"
 }
 
 // A function we use as a function value; it has no other purpose.
@@ -224,11 +226,22 @@ func Printf(format string, args ...interface{}) {
 	panic("don't call - testing only")
 }
 
+// Logf is used by the test so we must declare it.
+func Logf(format string, args ...interface{}) {
+	panic("don't call - testing only")
+}
+
 // printf is used by the test so we must declare it.
 func printf(format string, args ...interface{}) {
 	panic("don't call - testing only")
 }
 
+// Errorf is used by the test for a case in which the first parameter
+// is not a format string.
+func Errorf(i int, format string, args ...interface{}) {
+	panic("don't call - testing only")
+}
+
 // multi is used by the test.
 func multi() []interface{} {
 	panic("don't call - testing only")