From c7f7fa13811f37c34d350a30e0974c5af5f517fe Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Tue, 28 May 2013 15:37:34 -0400 Subject: [PATCH] cmd/vet: simplify format checker Simplify the code a bit, get it working after recent go/types changes, and handle "%*%" just in case. Preparation for handling argument indexing. R=gri CC=golang-dev https://golang.org/cl/9747045 --- cmd/vet/print.go | 71 +++++++++++++++++++++------------------ cmd/vet/testdata/print.go | 37 ++++++++++---------- cmd/vet/types.go | 2 +- 3 files changed, 58 insertions(+), 52 deletions(-) diff --git a/cmd/vet/print.go b/cmd/vet/print.go index debfbf0bfb..91a983cdc1 100644 --- a/cmd/vet/print.go +++ b/cmd/vet/print.go @@ -139,9 +139,6 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) { if format[i] == '%' { verb, flags, nbytes, nargs := f.parsePrintfVerb(call, format[i:]) w = nbytes - if verb == '%' { // "%%" does nothing interesting. - continue - } // If we've run out of args, print after loop will pick that up. if argNum+nargs <= len(call.Args) { f.checkPrintfArg(call, verb, flags, argNum, nargs) @@ -228,6 +225,7 @@ type printVerb struct { // Common flag sets for printf verbs. const ( + noFlag = "" numFlag = " -+.0" sharpNumFlag = " -+.0#" allFlags = " -+.0#" @@ -242,6 +240,7 @@ var printVerbs = []printVerb{ // '+' is required sign for numbers, Go format for %v. // '#' is alternate format for several verbs. // ' ' is spacer for numbers + {'%', noFlag, 0}, {'b', numFlag, argInt | argFloat}, {'c', "-", argRune | argInt}, {'d', numFlag, argInt}, @@ -263,42 +262,48 @@ var printVerbs = []printVerb{ {'X', sharpNumFlag, argRune | argInt | argString}, } -const printfVerbs = "bcdeEfFgGopqstTvxUX" - func (f *File) checkPrintfArg(call *ast.CallExpr, verb rune, flags []byte, argNum, nargs int) { + var v printVerb + found := false // Linear scan is fast enough for a small list. - for _, v := range printVerbs { + for _, v = range printVerbs { if v.verb == verb { - for _, flag := range flags { - if !strings.ContainsRune(v.flags, rune(flag)) { - f.Badf(call.Pos(), "unrecognized printf flag for verb %q: %q", verb, flag) - return - } - } - // Verb is good. If nargs>1, we have something like %.*s and all but the final - // arg must be integer. - for i := 0; i < nargs-1; i++ { - if !f.matchArgType(argInt, call.Args[argNum+i]) { - f.Badf(call.Pos(), "arg %s for * in printf format not of type int", f.gofmt(call.Args[argNum+i])) - } - } - for _, v := range printVerbs { - if v.verb == verb { - arg := call.Args[argNum+nargs-1] - if !f.matchArgType(v.typ, arg) { - typeString := "" - if typ := f.pkg.types[arg]; typ != nil { - typeString = typ.String() - } - f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: %s", f.gofmt(arg), verb, typeString) - } - break - } - } + found = true + break + } + } + if !found { + f.Badf(call.Pos(), "unrecognized printf verb %q", verb) + return + } + for _, flag := range flags { + if !strings.ContainsRune(v.flags, rune(flag)) { + f.Badf(call.Pos(), "unrecognized printf flag for verb %q: %q", verb, flag) return } } - f.Badf(call.Pos(), "unrecognized printf verb %q", verb) + // Verb is good. If nargs>trueArgs, we have something like %.*s and all but the final + // arg must be integer. + trueArgs := 1 + if verb == '%' { + trueArgs = 0 + } + for i := 0; i < nargs-trueArgs; i++ { + if !f.matchArgType(argInt, call.Args[argNum+i]) { + f.Badf(call.Pos(), "arg %s for * in printf format not of type int", f.gofmt(call.Args[argNum+i])) + } + } + if verb == '%' { + return + } + arg := call.Args[argNum+nargs-trueArgs] + if !f.matchArgType(v.typ, arg) { + typeString := "" + if typ := f.pkg.types[arg]; typ != nil { + typeString = typ.String() + } + f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: %s", f.gofmt(arg), verb, typeString) + } } // checkPrint checks a call to an unformatted print routine such as Println. diff --git a/cmd/vet/testdata/print.go b/cmd/vet/testdata/print.go index 72587feebf..32d921fe9f 100644 --- a/cmd/vet/testdata/print.go +++ b/cmd/vet/testdata/print.go @@ -74,6 +74,7 @@ func PrintfTests() { fmt.Printf("%x %x %x %x", 3, i, "hi", s) fmt.Printf("%X %X %X %X", 3, i, "hi", s) fmt.Printf("%.*s %d %g", 3, "hi", 23, 2.3) + fmt.Printf("%*%", 2) // Ridiculous but allowed. // Some bad format/argTypes fmt.Printf("%b", "hi") // ERROR "arg .hi. for printf verb %b of wrong type" fmt.Printf("%c", 2.3) // ERROR "arg 2.3 for printf verb %c of wrong type" @@ -93,24 +94,24 @@ func PrintfTests() { fmt.Printf("%x", nil) // ERROR "arg nil for printf verb %x of wrong type" fmt.Printf("%X", 2.3) // ERROR "arg 2.3 for printf verb %X of wrong type" fmt.Printf("%.*s %d %g", 3, "hi", 23, 'x') // ERROR "arg 'x' for printf verb %g of wrong type" - // TODO - fmt.Println() // not an error - fmt.Println("%s", "hi") // ERROR "possible formatting directive in Println call" - fmt.Printf("%s", "hi", 3) // ERROR "wrong number of args for format in Printf call" - fmt.Printf("%"+("s"), "hi", 3) // ERROR "wrong number of args for format in Printf call" - fmt.Printf("%s%%%d", "hi", 3) // correct - 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", "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" - fmt.Printf("%q %q", multi()...) // ok - fmt.Printf("%#q", `blah`) // ok - printf("now is the time", "buddy") // ERROR "no formatting directive" - Printf("now is the time", "buddy") // ERROR "no formatting directive" - Printf("hi") // ok + fmt.Println() // not an error + fmt.Println("%s", "hi") // ERROR "possible formatting directive in Println call" + fmt.Printf("%s", "hi", 3) // ERROR "wrong number of args for format in Printf call" + fmt.Printf("%"+("s"), "hi", 3) // ERROR "wrong number of args for format in Printf call" + fmt.Printf("%s%%%d", "hi", 3) // correct + 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", "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" + fmt.Printf("%*%", 0.22) // ERROR "arg 0.22 for \* in printf format not of type int" + fmt.Printf("%q %q", multi()...) // ok + fmt.Printf("%#q", `blah`) // ok + printf("now is the time", "buddy") // ERROR "no formatting directive" + Printf("now is the time", "buddy") // ERROR "no formatting directive" + Printf("hi") // ok const format = "%s %s\n" Printf(format, "hi", "there") Printf(format, "hi") // ERROR "wrong number of args for format in Printf call" diff --git a/cmd/vet/types.go b/cmd/vet/types.go index d42a1d55b9..2fa7f0272b 100644 --- a/cmd/vet/types.go +++ b/cmd/vet/types.go @@ -62,7 +62,7 @@ func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool { if typ == nil { return true } - basic, ok := typ.(*types.Basic) + basic, ok := typ.Underlying().(*types.Basic) if !ok { return true }