From ee1b90ad2c3cc97215c0f38f70e62396856eb0f2 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Tue, 22 Mar 2016 15:38:21 +0200 Subject: [PATCH] cmd/vet: improve detecting printf-like format argument Previously format argument was detected via scanning func type args. This didn't work when func type couldn't be determined if the func is declared in the external package. Fall back to scanning for the first string call argument in this case. Fixes #14754 Change-Id: I571cc29684cc641bc87882002ef474cf1481e9e2 Reviewed-on: https://go-review.googlesource.com/21023 Run-TryBot: Rob Pike TryBot-Result: Gobot Gobot Reviewed-by: Rob Pike --- src/cmd/vet/print.go | 94 ++++++++++++++++++++--------------- src/cmd/vet/testdata/print.go | 22 ++++++++ 2 files changed, 76 insertions(+), 40 deletions(-) diff --git a/src/cmd/vet/print.go b/src/cmd/vet/print.go index 5968141417..4e3252f2fb 100644 --- a/src/cmd/vet/print.go +++ b/src/cmd/vet/print.go @@ -78,32 +78,63 @@ 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 { +// formatString returns the format string argument and its index within +// the given printf-like call expression. +// +// The last parameter before variadic arguments is assumed to be +// a format string. +// +// The first string literal or string constant is assumed to be a format string +// if the call's signature cannot be determined. +// +// If it cannot find any format string parameter, it returns ("", -1). +func formatString(f *File, call *ast.CallExpr) (string, int) { typ := f.pkg.types[call.Fun].Type - if typ == nil { - return nil + if typ != nil { + if sig, ok := typ.(*types.Signature); ok { + if !sig.Variadic() { + // Skip checking non-variadic functions + return "", -1 + } + idx := sig.Params().Len() - 2 + if idx < 0 { + // Skip checking variadic functions without + // fixed arguments. + return "", -1 + } + s, ok := stringLiteralArg(f, call, idx) + if !ok { + // The last argument before variadic args isn't a string + return "", -1 + } + return s, idx + } } - 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 + // Cannot determine call's signature. Fallback to scanning for the first + // string argument in the call + for idx := range call.Args { + if s, ok := stringLiteralArg(f, call, idx); ok { + return s, idx } } - return idx + return "", -1 +} + +// stringLiteralArg returns call's string constant argument at the index idx. +// +// ("", false) is returned if call's argument at the index idx isn't a string +// literal. +func stringLiteralArg(f *File, call *ast.CallExpr, idx int) (string, bool) { + if idx >= len(call.Args) { + return "", false + } + arg := call.Args[idx] + lit := f.pkg.types[arg].Value + if lit != nil && lit.Kind() == constant.String { + return constant.StringVal(lit), true + } + return "", false } // checkCall triggers the print-specific checks if the call invokes a print function. @@ -173,32 +204,15 @@ 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) { - idx := formatIndex(signature(f, call)) - + format, idx := formatString(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[idx]].Value - if lit == nil { if *verbose { f.Warn(call.Pos(), "can't check non-constant format in call to", name) } return } - if lit.Kind() != constant.String { - f.Badf(call.Pos(), "constant %v not a string in call to %s", lit, name) - return - } - format := constant.StringVal(lit) + firstArg := idx + 1 // Arguments are immediately after format string. if !strings.Contains(format, "%") { if len(call.Args) > firstArg { diff --git a/src/cmd/vet/testdata/print.go b/src/cmd/vet/testdata/print.go index d0b5821e77..5c7ff35c90 100644 --- a/src/cmd/vet/testdata/print.go +++ b/src/cmd/vet/testdata/print.go @@ -11,6 +11,9 @@ import ( "math" "os" "unsafe" // just for test case printing unsafe.Pointer + + // For testing printf-like functions from external package. + "github.com/foobar/externalprintf" ) func UnsafePointerPrintfTest() { @@ -215,6 +218,19 @@ func PrintfTests() { Errorf(1, "%d", 3) // OK Errorf(1, "%d", "hi") // ERROR "arg .hi. for printf verb %d of wrong type: string" + + // Multiple string arguments before variadic args + errorf("WARNING", "foobar") // OK + errorf("INFO", "s=%s, n=%d", "foo", 1) // OK + errorf("ERROR", "%d") // ERROR "format reads arg 1, have only 0 args" + + // Printf from external package + externalprintf.Printf("%d", 42) // OK + externalprintf.Printf("foobar") // OK + level := 123 + externalprintf.Logf(level, "%d", 42) // OK + externalprintf.Errorf(level, level, "foo %q bar", "foobar") // OK + externalprintf.Logf(level, "%d") // ERROR "format reads arg 1, have only 0 args" } // A function we use as a function value; it has no other purpose. @@ -242,6 +258,12 @@ func Errorf(i int, format string, args ...interface{}) { panic("don't call - testing only") } +// errorf is used by the test for a case in which the function accepts multiple +// string parameters before variadic arguments +func errorf(level, 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") -- 2.48.1