From 31c219bef71c80f65ac18b04c352c3d3f4f595c1 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Tue, 4 Mar 2014 15:41:49 +1100 Subject: [PATCH] go.tools/cmd/vet: change reporting of incorrect error count in Printf The old code was misleading in saying how many args were present. Change the wording of the message to be unambiguous and change the presentation of the format to include the full directive, making it easier to correlate with the input (and fixing a silent bug). Fixes golang/go#6248. LGTM=dsymonds R=golang-codereviews, dsymonds CC=golang-codereviews https://golang.org/cl/69120044 --- cmd/vet/print.go | 14 +++++--------- cmd/vet/testdata/print.go | 9 +++++---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/cmd/vet/print.go b/cmd/vet/print.go index 52e14f8e14..ab96dcf677 100644 --- a/cmd/vet/print.go +++ b/cmd/vet/print.go @@ -9,7 +9,6 @@ package main import ( "bytes" "flag" - "fmt" "go/ast" "go/token" "strconv" @@ -65,11 +64,10 @@ func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) { // It is constructed by parsePrintfVerb. type formatState struct { verb rune // the format verb: 'd' for "%d" - format string // the full format string + format string // the full format directive from % through verb, "%.3d". name string // Printf, Sprintf etc. flags []byte // the list of # + etc. argNums []int // the successive argument numbers that are consumed, adjusted to refer to actual arg in call - nbytes int // number of bytes of the format string consumed. indexed bool // whether an indexing expression appears: %[1]d. firstArg int // Index of first argument after the format in the Printf call. // Used only during parse. @@ -77,6 +75,7 @@ type formatState struct { call *ast.CallExpr argNum int // Which argument we're expecting to format now. indexPending bool // Whether we have an indexed argument that has not resolved. + nbytes int // number of bytes of the format string consumed. } // checkPrintf checks a call to a formatted print routine such as Printf. @@ -115,7 +114,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) { if state == nil { return } - w = state.nbytes + w = len(state.format) if state.indexed { indexed = true } @@ -267,6 +266,7 @@ func (f *File) parsePrintfVerb(call *ast.CallExpr, name, format string, firstArg if verb != '%' { state.argNums = append(state.argNums, state.argNum) } + state.format = state.format[:state.nbytes] return state } @@ -433,13 +433,9 @@ func (f *File) argCanBeChecked(call *ast.CallExpr, formatArg int, isStar bool, s return true } // There are bad indexes in the format or there are fewer arguments than the format needs. - verb := fmt.Sprintf("verb %%%c", state.verb) - if isStar { - verb = "indirect *" - } // This is the argument number relative to the format: Printf("%s", "hi") will give 1 for the "hi". arg := argNum - state.firstArg + 1 // People think of arguments as 1-indexed. - f.Badf(call.Pos(), "missing argument for %s %s: need %d, have %d", state.name, verb, arg, len(call.Args)-state.firstArg) + f.Badf(call.Pos(), `missing argument for %s("%s"): format reads arg %d, have only %d args`, state.name, state.format, arg, len(call.Args)-state.firstArg) return false } diff --git a/cmd/vet/testdata/print.go b/cmd/vet/testdata/print.go index ee8d22039a..1dd58d9f69 100644 --- a/cmd/vet/testdata/print.go +++ b/cmd/vet/testdata/print.go @@ -134,7 +134,7 @@ func PrintfTests() { fmt.Printf("%08s", "woo") // correct fmt.Printf("% 8s", "woo") // correct fmt.Printf("%.*d", 3, 3) // correct - fmt.Printf("%.*d", 3, 3, 3) // ERROR "wrong number of args for format in Printf call" + fmt.Printf("%.*d", 3, 3, 3, 3) // ERROR "wrong number of args for format in Printf call.*4 args" fmt.Printf("%.*d", "hi", 3) // ERROR "arg .hi. for \* in printf format not of type int" fmt.Printf("%.*d", i, 3) // correct fmt.Printf("%.*d", s, 3) // ERROR "arg s for \* in printf format not of type int" @@ -146,7 +146,8 @@ func PrintfTests() { Printf("hi") // ok const format = "%s %s\n" Printf(format, "hi", "there") - Printf(format, "hi") // ERROR "missing argument for Printf verb %s: need 2, have 1" + Printf(format, "hi") // ERROR "missing argument for Printf..%s..: format reads arg 2, have only 1" + Printf("%s %d %.3v %q", "str", 4) // ERROR "missing argument for Printf..%.3v..: format reads arg 3, have only 2" f := new(stringer) f.Warn(0, "%s", "hello", 3) // ERROR "possible formatting directive in Warn call" f.Warnf(0, "%s", "hello", 3) // ERROR "wrong number of args for format in Warnf call" @@ -169,8 +170,8 @@ func PrintfTests() { // Bad argument reorderings. Printf("%[xd", 3) // ERROR "illegal syntax for printf argument index" Printf("%[x]d", 3) // ERROR "illegal syntax for printf argument index" - Printf("%[3]*s", "hi", 2) // ERROR "missing argument for Printf indirect \*: need 3, have 2" - fmt.Sprintf("%[3]d", 2) // ERROR "missing argument for Sprintf verb %d: need 3, have 1" + Printf("%[3]*s", "hi", 2) // ERROR "missing argument for Printf.* reads arg 3, have only 2" + fmt.Sprintf("%[3]d", 2) // ERROR "missing argument for Sprintf.* reads arg 3, have only 1" Printf("%[2]*.[1]*[3]d", 2, "hi", 4) // ERROR "arg .hi. for \* in printf format not of type int" // Something that satisfies the error interface. var e error