From 3a0c4deef17a05d90ac789de162b10649b6dcf6d Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Sun, 14 Oct 2018 10:34:06 -0400 Subject: [PATCH] go/analysis/passes/printf: changes for analysis API Guide to changes: - The -printfuncs flag is renamed -printf.funcs. It no longer supports "pkg.T.f" form, requiring instead "(dir/pkg.T).f" or "(*dir/pkg.T).f". The legacy ":%d" suffix is no longer supported. - (*testing.T).Errorf and friends are removed from the isPrint map because they are discovered by induction while analyzing package "testing". - localPrintfLike map operations are replaced by the Fact mechanism. - The go/types representation is used instead of strings/ast.Nodes in various places. For example: pkgpath, name string -> *types.Func (to identify a function) format, args *ast.Field -> *types.Var (to identify format/args params) This was required to fix a latent bug in maybePrintfWrapper's handling of format string parameters` declared using "factored" syntax, such as: func f(foo, format string, args...interface{}). See L253 of the original testdata file for a testcase that ensured the buggy (?) behavior. - func printfLike is removed as it was deadcode. - isFormatter is rewritten to avoid a global variable. - "if verbose { warn }" is replaced by "if false { report }" for now. - recursive stringer is rewritten more simply in term of go/types. Change-Id: Ia6ee827117b611c686e38207916a21fe1fc296e2 Reviewed-on: https://go-review.googlesource.com/c/142239 Reviewed-by: Michael Matloob --- go/analysis/passes/printf/printf.go | 658 ++++++++---------- go/analysis/passes/printf/printf_test.go | 14 + go/analysis/passes/printf/testdata/src/a/a.go | 290 ++++---- go/analysis/passes/printf/testdata/src/b/b.go | 33 + go/analysis/passes/printf/types.go | 188 +---- go/types/typeutil/callee.go | 21 +- 6 files changed, 529 insertions(+), 675 deletions(-) create mode 100644 go/analysis/passes/printf/printf_test.go create mode 100644 go/analysis/passes/printf/testdata/src/b/b.go diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go index f980dcb5..23f634fd 100644 --- a/go/analysis/passes/printf/printf.go +++ b/go/analysis/passes/printf/printf.go @@ -1,17 +1,13 @@ -// +build ignore - // Copyright 2010 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. // This file contains the printf-checker. -package main +package printf import ( "bytes" - "encoding/gob" - "flag" "fmt" "go/ast" "go/constant" @@ -22,58 +18,78 @@ import ( "strconv" "strings" "unicode/utf8" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/analysis/passes/internal/analysisutil" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/go/types/typeutil" ) -var printfuncs = flag.String("printfuncs", "", "comma-separated list of print function names to check") - func init() { - register("printf", - "check printf-like invocations", - checkFmtPrintfCall, - funcDecl, callExpr) - registerPkgCheck("printf", findPrintfLike) - registerExport("printf", exportPrintfLike) - gob.Register([]printfExport(nil)) + Analyzer.Flags.Var(isPrint, "funcs", "comma-separated list of print function names to check") } -func initPrintFlags() { - if *printfuncs == "" { - return +var Analyzer = &analysis.Analyzer{ + Name: "printf", + Doc: "check printf-like invocations", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + FactTypes: []analysis.Fact{new(isWrapper)}, +} + +const doc = `check consistency of Printf format strings and arguments + +The check applies to known functions (for example, those in package fmt) +as well as any detected wrappers of known functions. + +A function that wants to avail itself of printf checking but does not +get found by this analyzer's heuristics (for example, due to use of +dynamic calls) can insert a bogus call: + + if false { + fmt.Sprintf(format, args...) // enable printf checking } - for _, name := range strings.Split(*printfuncs, ",") { - if len(name) == 0 { - flag.Usage() - } - // Backwards compatibility: skip optional first argument - // index after the colon. - if colon := strings.LastIndex(name, ":"); colon > 0 { - name = name[:colon] - } +The -funcs flag specifies a comma-separated list of names of additional +known formatting functions or methods. If the name contains a period, +it must denote a specific function using one of the following forms: - if !strings.Contains(name, ".") { - name = strings.ToLower(name) - } - isPrint[name] = true + dir/pkg.Function + dir/pkg.Type.Method + (*dir/pkg.Type).Method + +Otherwise the name is interpreted as a case-insensitive unqualified +identifier such as "errorf". Either way, if a listed name ends in f, the +function is assumed to be Printf-like, taking a format string before the +argument list. Otherwise it is assumed to be Print-like, taking a list +of arguments with no format string. +` + +// isWrapper is a fact indicating that a function is a print or printf wrapper. +type isWrapper struct{ Printf bool } + +func (f *isWrapper) AFact() {} + +func (f *isWrapper) String() string { + if f.Printf { + return "printfWrapper" + } else { + return "printWrapper" } } -var localPrintfLike = make(map[string]int) - -type printfExport struct { - Name string - Kind int +func run(pass *analysis.Pass) (interface{}, error) { + findPrintfLike(pass) + checkCall(pass) + return nil, nil } -// printfImported maps from package name to the printf vet data -// exported by that package. -var printfImported = make(map[string]map[string]int) - type printfWrapper struct { - name string - fn *ast.FuncDecl - format *ast.Field - args *ast.Field + obj *types.Func + fdecl *ast.FuncDecl + format *types.Var + args *types.Var callers []printfCaller failed bool // if true, not a printf wrapper } @@ -90,78 +106,56 @@ type printfCaller struct { // A function may be a Printf or Print wrapper if its last argument is ...interface{}. // If the next-to-last argument is a string, then this may be a Printf wrapper. // Otherwise it may be a Print wrapper. -func maybePrintfWrapper(decl ast.Decl) *printfWrapper { +func maybePrintfWrapper(info *types.Info, decl ast.Decl) *printfWrapper { // Look for functions with final argument type ...interface{}. - fn, ok := decl.(*ast.FuncDecl) - if !ok || fn.Body == nil { + fdecl, ok := decl.(*ast.FuncDecl) + if !ok || fdecl.Body == nil { return nil } - name := fn.Name.Name - if fn.Recv != nil { - // For (*T).Name or T.name, use "T.name". - rcvr := fn.Recv.List[0].Type - if ptr, ok := rcvr.(*ast.StarExpr); ok { - rcvr = ptr.X - } - id, ok := rcvr.(*ast.Ident) - if !ok { - return nil - } - name = id.Name + "." + name + fn := info.Defs[fdecl.Name].(*types.Func) + + sig := fn.Type().(*types.Signature) + if !sig.Variadic() { + return nil // not variadic } - params := fn.Type.Params.List - if len(params) == 0 { - return nil + + params := sig.Params() + nparams := params.Len() // variadic => nonzero + + args := params.At(nparams - 1) + iface, ok := args.Type().(*types.Slice).Elem().(*types.Interface) + if !ok || !iface.Empty() { + return nil // final (args) param is not ...interface{} } - args := params[len(params)-1] - if len(args.Names) != 1 { - return nil - } - ddd, ok := args.Type.(*ast.Ellipsis) - if !ok { - return nil - } - iface, ok := ddd.Elt.(*ast.InterfaceType) - if !ok || len(iface.Methods.List) > 0 { - return nil - } - var format *ast.Field - if len(params) >= 2 { - p := params[len(params)-2] - if len(p.Names) == 1 { - if id, ok := p.Type.(*ast.Ident); ok && id.Name == "string" { - format = p - } + + // Is second last param 'format string'? + var format *types.Var + if nparams >= 2 { + if p := params.At(nparams - 2); p.Type() == types.Typ[types.String] { + format = p } } return &printfWrapper{ - name: name, - fn: fn, + obj: fn, + fdecl: fdecl, format: format, args: args, } } // findPrintfLike scans the entire package to find printf-like functions. -func findPrintfLike(pkg *Package) { - if vcfg.ImportPath == "" { // no type or vetx information; don't bother - return - } - - // Gather potential wrappesr and call graph between them. - byName := make(map[string]*printfWrapper) +func findPrintfLike(pass *analysis.Pass) (interface{}, error) { + // Gather potential wrappers and call graph between them. + byObj := make(map[*types.Func]*printfWrapper) var wrappers []*printfWrapper - for _, file := range pkg.files { - if file.file == nil { - continue - } - for _, decl := range file.file.Decls { - w := maybePrintfWrapper(decl) + for _, file := range pass.Files { + for _, decl := range file.Decls { + w := maybePrintfWrapper(pass.TypesInfo, decl) if w == nil { continue } - byName[w.name] = w + byObj[w.obj] = w wrappers = append(wrappers, w) } } @@ -169,7 +163,7 @@ func findPrintfLike(pkg *Package) { // Walk the graph to figure out which are really printf wrappers. for _, w := range wrappers { // Scan function for calls that could be to other printf-like functions. - ast.Inspect(w.fn.Body, func(n ast.Node) bool { + ast.Inspect(w.fdecl.Body, func(n ast.Node) bool { if w.failed { return false } @@ -177,7 +171,8 @@ func findPrintfLike(pkg *Package) { // TODO: Relax these checks; issue 26555. if assign, ok := n.(*ast.AssignStmt); ok { for _, lhs := range assign.Lhs { - if match(lhs, w.format) || match(lhs, w.args) { + if match(pass.TypesInfo, lhs, w.format) || + match(pass.TypesInfo, lhs, w.args) { // Modifies the format // string or args in // some way, so not a @@ -188,7 +183,8 @@ func findPrintfLike(pkg *Package) { } } if un, ok := n.(*ast.UnaryExpr); ok && un.Op == token.AND { - if match(un.X, w.format) || match(un.X, w.args) { + if match(pass.TypesInfo, un.X, w.format) || + match(pass.TypesInfo, un.X, w.args) { // Taking the address of the // format string or args, // so not a simple wrapper. @@ -198,32 +194,33 @@ func findPrintfLike(pkg *Package) { } call, ok := n.(*ast.CallExpr) - if !ok || len(call.Args) == 0 || !match(call.Args[len(call.Args)-1], w.args) { + if !ok || len(call.Args) == 0 || !match(pass.TypesInfo, call.Args[len(call.Args)-1], w.args) { return true } - pkgpath, name, kind := printfNameAndKind(pkg, call.Fun) + fn, kind := printfNameAndKind(pass, call) if kind != 0 { - checkPrintfFwd(pkg, w, call, kind) + checkPrintfFwd(pass, w, call, kind) return true } // If the call is to another function in this package, // maybe we will find out it is printf-like later. // Remember this call for later checking. - if pkgpath == "" && byName[name] != nil { - callee := byName[name] + if fn != nil && fn.Pkg() == pass.Pkg && byObj[fn] != nil { + callee := byObj[fn] callee.callers = append(callee.callers, printfCaller{w, call}) } return true }) } + return nil, nil } -func match(arg ast.Expr, param *ast.Field) bool { +func match(info *types.Info, arg ast.Expr, param *types.Var) bool { id, ok := arg.(*ast.Ident) - return ok && id.Obj != nil && id.Obj.Decl == param + return ok && info.ObjectOf(id) == param } const ( @@ -231,37 +228,17 @@ const ( kindPrint = 2 ) -// printfLike reports whether a call to fn should be considered a call to a printf-like function. -// It returns 0 (indicating not a printf-like function), kindPrintf, or kindPrint. -func printfLike(pkg *Package, fn ast.Expr, byName map[string]*printfWrapper) int { - if id, ok := fn.(*ast.Ident); ok && id.Obj != nil { - if w := byName[id.Name]; w != nil && id.Obj.Decl == w.fn { - // Found call to function in same package. - return localPrintfLike[id.Name] - } - } - if sel, ok := fn.(*ast.SelectorExpr); ok { - if id, ok := sel.X.(*ast.Ident); ok && id.Name == "fmt" && strings.Contains(sel.Sel.Name, "rint") { - if strings.HasSuffix(sel.Sel.Name, "f") { - return kindPrintf - } - return kindPrint - } - } - return 0 -} - // checkPrintfFwd checks that a printf-forwarding wrapper is forwarding correctly. // It diagnoses writing fmt.Printf(format, args) instead of fmt.Printf(format, args...). -func checkPrintfFwd(pkg *Package, w *printfWrapper, call *ast.CallExpr, kind int) { +func checkPrintfFwd(pass *analysis.Pass, w *printfWrapper, call *ast.CallExpr, kind int) { matched := kind == kindPrint || - kind == kindPrintf && len(call.Args) >= 2 && match(call.Args[len(call.Args)-2], w.format) + kind == kindPrintf && len(call.Args) >= 2 && match(pass.TypesInfo, call.Args[len(call.Args)-2], w.format) if !matched { return } if !call.Ellipsis.IsValid() { - typ, ok := pkg.types[call.Fun].Type.(*types.Signature) + typ, ok := pass.TypesInfo.Types[call.Fun].Type.(*types.Signature) if !ok { return } @@ -275,80 +252,53 @@ func checkPrintfFwd(pkg *Package, w *printfWrapper, call *ast.CallExpr, kind int // } return } - if !vcfg.VetxOnly { - desc := "printf" - if kind == kindPrint { - desc = "print" - } - pkg.files[0].Badf(call.Pos(), "missing ... in args forwarded to %s-like function", desc) + desc := "printf" + if kind == kindPrint { + desc = "print" } + pass.Reportf(call.Pos(), "missing ... in args forwarded to %s-like function", desc) return } - name := w.name - if localPrintfLike[name] == 0 { - localPrintfLike[name] = kind + fn := w.obj + var fact isWrapper + if !pass.ImportObjectFact(fn, &fact) { + fact.Printf = kind == kindPrintf + pass.ExportObjectFact(fn, &fact) for _, caller := range w.callers { - checkPrintfFwd(pkg, caller.w, caller.call, kind) + checkPrintfFwd(pass, caller.w, caller.call, kind) } } } -func exportPrintfLike() interface{} { - out := make([]printfExport, 0, len(localPrintfLike)) - for name, kind := range localPrintfLike { - out = append(out, printfExport{ - Name: name, - Kind: kind, - }) - } - sort.Slice(out, func(i, j int) bool { - return out[i].Name < out[j].Name - }) - return out -} - // isPrint records the print functions. // If a key ends in 'f' then it is assumed to be a formatted print. -var isPrint = map[string]bool{ +// +// Keys are either values returned by (*types.Func).FullName, +// or case-insensitive identifiers such as "errorf". +// +// The -funcs flag adds to this set. +var isPrint = stringSet{ "fmt.Errorf": true, "fmt.Fprint": true, "fmt.Fprintf": true, "fmt.Fprintln": true, - "fmt.Print": true, - "fmt.Printf": true, - "fmt.Println": true, + "fmt.Print": true, // technically these three + "fmt.Printf": true, // are redundant because they + "fmt.Println": true, // forward to Fprint{,f,ln} "fmt.Sprint": true, "fmt.Sprintf": true, "fmt.Sprintln": true, - // testing.B, testing.T not auto-detected - // because the methods are picked up by embedding. - "testing.B.Error": true, - "testing.B.Errorf": true, - "testing.B.Fatal": true, - "testing.B.Fatalf": true, - "testing.B.Log": true, - "testing.B.Logf": true, - "testing.B.Skip": true, - "testing.B.Skipf": true, - "testing.T.Error": true, - "testing.T.Errorf": true, - "testing.T.Fatal": true, - "testing.T.Fatalf": true, - "testing.T.Log": true, - "testing.T.Logf": true, - "testing.T.Skip": true, - "testing.T.Skipf": true, - - // testing.TB is an interface, so can't detect wrapping. - "testing.TB.Error": true, - "testing.TB.Errorf": true, - "testing.TB.Fatal": true, - "testing.TB.Fatalf": true, - "testing.TB.Log": true, - "testing.TB.Logf": true, - "testing.TB.Skip": true, - "testing.TB.Skipf": true, + // *testing.T and B are detected by induction, but testing.TB is + // an interface and the inference can't follow dynamic calls. + "(testing.TB).Error": true, + "(testing.TB).Errorf": true, + "(testing.TB).Fatal": true, + "(testing.TB).Fatalf": true, + "(testing.TB).Log": true, + "(testing.TB).Logf": true, + "(testing.TB).Skip": true, + "(testing.TB).Skipf": true, } // formatString returns the format string argument and its index within @@ -361,8 +311,8 @@ var isPrint = map[string]bool{ // 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) (format string, idx int) { - typ := f.pkg.types[call.Fun].Type +func formatString(pass *analysis.Pass, call *ast.CallExpr) (format string, idx int) { + typ := pass.TypesInfo.Types[call.Fun].Type if typ != nil { if sig, ok := typ.(*types.Signature); ok { if !sig.Variadic() { @@ -375,7 +325,7 @@ func formatString(f *File, call *ast.CallExpr) (format string, idx int) { // fixed arguments. return "", -1 } - s, ok := stringConstantArg(f, call, idx) + s, ok := stringConstantArg(pass, call, idx) if !ok { // The last argument before variadic args isn't a string. return "", -1 @@ -387,10 +337,10 @@ func formatString(f *File, call *ast.CallExpr) (format string, idx int) { // Cannot determine call's signature. Fall back to scanning for the first // string constant in the call. for idx := range call.Args { - if s, ok := stringConstantArg(f, call, idx); ok { + if s, ok := stringConstantArg(pass, call, idx); ok { return s, idx } - if f.pkg.types[call.Args[idx]].Type == types.Typ[types.String] { + if pass.TypesInfo.Types[call.Args[idx]].Type == types.Typ[types.String] { // Skip checking a call with a non-constant format // string argument, since its contents are unavailable // for validation. @@ -404,12 +354,12 @@ func formatString(f *File, call *ast.CallExpr) (format string, idx int) { // // ("", false) is returned if call's argument at the index idx isn't a string // constant. -func stringConstantArg(f *File, call *ast.CallExpr, idx int) (string, bool) { +func stringConstantArg(pass *analysis.Pass, call *ast.CallExpr, idx int) (string, bool) { if idx >= len(call.Args) { return "", false } arg := call.Args[idx] - lit := f.pkg.types[arg].Value + lit := pass.TypesInfo.Types[arg].Value if lit != nil && lit.Kind() == constant.String { return constant.StringVal(lit), true } @@ -417,134 +367,63 @@ func stringConstantArg(f *File, call *ast.CallExpr, idx int) (string, bool) { } // checkCall triggers the print-specific checks if the call invokes a print function. -func checkFmtPrintfCall(f *File, node ast.Node) { - if f.pkg.typesPkg == nil { - // This check now requires type information. - return +func checkCall(pass *analysis.Pass) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + call := n.(*ast.CallExpr) + fn, kind := printfNameAndKind(pass, call) + switch kind { + case kindPrintf: + checkPrintf(pass, call, fn) + case kindPrint: + checkPrint(pass, call, fn) + } + }) +} + +func printfNameAndKind(pass *analysis.Pass, call *ast.CallExpr) (fn *types.Func, kind int) { + fn, _ = typeutil.Callee(pass.TypesInfo, call).(*types.Func) + if fn == nil { + return nil, 0 } - if d, ok := node.(*ast.FuncDecl); ok && isStringer(f, d) { - // Remember we saw this. - if f.stringerPtrs == nil { - f.stringerPtrs = make(map[*ast.Object]bool) + var fact isWrapper + if pass.ImportObjectFact(fn, &fact) { + if fact.Printf { + return fn, kindPrintf + } else { + return fn, kindPrint } - if l := d.Recv.List; len(l) == 1 { - if n := l[0].Names; len(n) == 1 { - typ := f.pkg.types[l[0].Type] - _, ptrRecv := typ.Type.(*types.Pointer) - f.stringerPtrs[n[0].Obj] = ptrRecv - } - } - return } - call, ok := node.(*ast.CallExpr) + _, ok := isPrint[fn.FullName()] if !ok { - return + // Next look up just "printf", for use with -printf.funcs. + _, ok = isPrint[strings.ToLower(fn.Name())] } - - // Construct name like pkg.Printf or pkg.Type.Printf for lookup. - _, name, kind := printfNameAndKind(f.pkg, call.Fun) - if kind == kindPrintf { - f.checkPrintf(call, name) - } - if kind == kindPrint { - f.checkPrint(call, name) - } -} - -func printfName(pkg *Package, called ast.Expr) (pkgpath, name string) { - switch x := called.(type) { - case *ast.Ident: - if fn, ok := pkg.uses[x].(*types.Func); ok { - if fn.Pkg() == nil || fn.Pkg() == pkg.typesPkg { - pkgpath = "" - } else { - pkgpath = fn.Pkg().Path() - } - return pkgpath, x.Name - } - - case *ast.SelectorExpr: - // Check for "fmt.Printf". - if id, ok := x.X.(*ast.Ident); ok { - if pkgName, ok := pkg.uses[id].(*types.PkgName); ok { - return pkgName.Imported().Path(), x.Sel.Name - } - } - - // Check for t.Logf where t is a *testing.T. - if sel := pkg.selectors[x]; sel != nil { - recv := sel.Recv() - if p, ok := recv.(*types.Pointer); ok { - recv = p.Elem() - } - if named, ok := recv.(*types.Named); ok { - obj := named.Obj() - if obj.Pkg() == nil || obj.Pkg() == pkg.typesPkg { - pkgpath = "" - } else { - pkgpath = obj.Pkg().Path() - } - return pkgpath, obj.Name() + "." + x.Sel.Name - } + if ok { + if strings.HasSuffix(fn.Name(), "f") { + kind = kindPrintf + } else { + kind = kindPrint } } - return "", "" -} - -func printfNameAndKind(pkg *Package, called ast.Expr) (pkgpath, name string, kind int) { - pkgpath, name = printfName(pkg, called) - if name == "" { - return pkgpath, name, 0 - } - - if pkgpath == "" { - kind = localPrintfLike[name] - } else if m, ok := printfImported[pkgpath]; ok { - kind = m[name] - } else { - var m map[string]int - if out, ok := readVetx(pkgpath, "printf").([]printfExport); ok { - m = make(map[string]int) - for _, x := range out { - m[x.Name] = x.Kind - } - } - printfImported[pkgpath] = m - kind = m[name] - } - - if kind == 0 { - _, ok := isPrint[pkgpath+"."+name] - if !ok { - // Next look up just "printf", for use with -printfuncs. - short := name[strings.LastIndex(name, ".")+1:] - _, ok = isPrint[strings.ToLower(short)] - } - if ok { - if strings.HasSuffix(name, "f") { - kind = kindPrintf - } else { - kind = kindPrint - } - } - } - return pkgpath, name, kind -} - -// isStringer returns true if the provided declaration is a "String() string" -// method, an implementation of fmt.Stringer. -func isStringer(f *File, d *ast.FuncDecl) bool { - return d.Recv != nil && d.Name.Name == "String" && d.Type.Results != nil && - len(d.Type.Params.List) == 0 && len(d.Type.Results.List) == 1 && - f.pkg.types[d.Type.Results.List[0].Type].Type == types.Typ[types.String] + return fn, kind } // isFormatter reports whether t satisfies fmt.Formatter. // Unlike fmt.Stringer, it's impossible to satisfy fmt.Formatter without importing fmt. -func (f *File) isFormatter(t types.Type) bool { - return formatterType != nil && types.Implements(t, formatterType) +func isFormatter(pass *analysis.Pass, t types.Type) bool { + for _, imp := range pass.Pkg.Imports() { + if imp.Path() == "fmt" { + formatter := imp.Scope().Lookup("Formatter").Type().Underlying().(*types.Interface) + return types.Implements(t, formatter) + } + } + return false } // formatState holds the parsed representation of a printf directive such as "%3.*[4]d". @@ -557,7 +436,7 @@ type formatState struct { argNums []int // the successive argument numbers that are consumed, adjusted to refer to actual arg in call firstArg int // Index of first argument after the format in the Printf call. // Used only during parse. - file *File + pass *analysis.Pass call *ast.CallExpr argNum int // Which argument we're expecting to format now. hasIndex bool // Whether the argument is indexed. @@ -566,11 +445,11 @@ type formatState struct { } // checkPrintf checks a call to a formatted print routine such as Printf. -func (f *File) checkPrintf(call *ast.CallExpr, name string) { - format, idx := formatString(f, call) +func checkPrintf(pass *analysis.Pass, call *ast.CallExpr, fn *types.Func) { + format, idx := formatString(pass, call) if idx < 0 { - if *verbose { - f.Warn(call.Pos(), "can't check non-constant format in call to", name) + if false { + pass.Reportf(call.Lparen, "can't check non-constant format in call to %s", fn.Name()) } return } @@ -578,7 +457,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string) { firstArg := idx + 1 // Arguments are immediately after format string. if !strings.Contains(format, "%") { if len(call.Args) > firstArg { - f.Badf(call.Pos(), "%s call has arguments but no formatting directives", name) + pass.Reportf(call.Lparen, "%s call has arguments but no formatting directives", fn.Name()) } return } @@ -591,12 +470,12 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string) { if format[i] != '%' { continue } - state := f.parsePrintfVerb(call, name, format[i:], firstArg, argNum) + state := parsePrintfVerb(pass, call, fn.Name(), format[i:], firstArg, argNum) if state == nil { return } w = len(state.format) - if !f.okPrintfArg(call, state) { // One error per format is enough. + if !okPrintfArg(pass, call, state) { // One error per format is enough. return } if state.hasIndex { @@ -624,7 +503,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string) { if maxArgNum != len(call.Args) { expect := maxArgNum - firstArg numArgs := len(call.Args) - firstArg - f.Badf(call.Pos(), "%s call needs %v but has %v", name, count(expect, "arg"), count(numArgs, "arg")) + pass.Reportf(call.Pos(), "%s call needs %v but has %v", fn.Name(), count(expect, "arg"), count(numArgs, "arg")) } } @@ -665,13 +544,13 @@ func (s *formatState) parseIndex() bool { ok = false s.nbytes = strings.Index(s.format, "]") if s.nbytes < 0 { - s.file.Badf(s.call.Pos(), "%s format %s is missing closing ]", s.name, s.format) + s.pass.Reportf(s.call.Pos(), "%s format %s is missing closing ]", s.name, s.format) return false } } arg32, err := strconv.ParseInt(s.format[start:s.nbytes], 10, 32) if err != nil || !ok || arg32 <= 0 || arg32 > int64(len(s.call.Args)-s.firstArg) { - s.file.Badf(s.call.Pos(), "%s format has invalid argument index [%s]", s.name, s.format[start:s.nbytes]) + s.pass.Reportf(s.call.Pos(), "%s format has invalid argument index [%s]", s.name, s.format[start:s.nbytes]) return false } s.nbytes++ // skip ']' @@ -717,7 +596,7 @@ func (s *formatState) parsePrecision() bool { // parsePrintfVerb looks the formatting directive that begins the format string // and returns a formatState that encodes what the directive wants, without looking // at the actual arguments present in the call. The result is nil if there is an error. -func (f *File) parsePrintfVerb(call *ast.CallExpr, name, format string, firstArg, argNum int) *formatState { +func parsePrintfVerb(pass *analysis.Pass, call *ast.CallExpr, name, format string, firstArg, argNum int) *formatState { state := &formatState{ format: format, name: name, @@ -726,7 +605,7 @@ func (f *File) parsePrintfVerb(call *ast.CallExpr, name, format string, firstArg argNums: make([]int, 0, 1), nbytes: 1, // There's guaranteed to be a percent sign. firstArg: firstArg, - file: f, + pass: pass, call: call, } // There may be flags. @@ -748,7 +627,7 @@ func (f *File) parsePrintfVerb(call *ast.CallExpr, name, format string, firstArg return nil } if state.nbytes == len(state.format) { - f.Badf(call.Pos(), "%s format %s is missing verb at end of string", name, state.format) + pass.Reportf(call.Pos(), "%s format %s is missing verb at end of string", name, state.format) return nil } verb, w := utf8.DecodeRuneInString(state.format[state.nbytes:]) @@ -821,7 +700,7 @@ var printVerbs = []printVerb{ // okPrintfArg compares the formatState to the arguments actually present, // reporting any discrepancies it can discern. If the final argument is ellipsissed, // there's little it can do for that. -func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) { +func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, state *formatState) (ok bool) { var v printVerb found := false // Linear scan is fast enough for a small list. @@ -835,14 +714,14 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) { // Does current arg implement fmt.Formatter? formatter := false if state.argNum < len(call.Args) { - if tv, ok := f.pkg.types[call.Args[state.argNum]]; ok { - formatter = f.isFormatter(tv.Type) + if tv, ok := pass.TypesInfo.Types[call.Args[state.argNum]]; ok { + formatter = isFormatter(pass, tv.Type) } } if !formatter { if !found { - f.Badf(call.Pos(), "%s format %s has unknown verb %c", state.name, state.format, state.verb) + pass.Reportf(call.Pos(), "%s format %s has unknown verb %c", state.name, state.format, state.verb) return false } for _, flag := range state.flags { @@ -852,7 +731,7 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) { continue } if !strings.ContainsRune(v.flags, rune(flag)) { - f.Badf(call.Pos(), "%s format %s has unrecognized flag %c", state.name, state.format, flag) + pass.Reportf(call.Pos(), "%s format %s has unrecognized flag %c", state.name, state.format, flag) return false } } @@ -866,86 +745,73 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) { nargs := len(state.argNums) for i := 0; i < nargs-trueArgs; i++ { argNum := state.argNums[i] - if !f.argCanBeChecked(call, i, state) { + if !argCanBeChecked(pass, call, i, state) { return } arg := call.Args[argNum] - if !f.matchArgType(argInt, nil, arg) { - f.Badf(call.Pos(), "%s format %s uses non-int %s as argument of *", state.name, state.format, f.gofmt(arg)) + if !matchArgType(pass, argInt, nil, arg) { + pass.Reportf(call.Pos(), "%s format %s uses non-int %s as argument of *", state.name, state.format, analysisutil.Format(pass.Fset, arg)) return false } } + if state.verb == '%' || formatter { return true } argNum := state.argNums[len(state.argNums)-1] - if !f.argCanBeChecked(call, len(state.argNums)-1, state) { + if !argCanBeChecked(pass, call, len(state.argNums)-1, state) { return false } arg := call.Args[argNum] - if f.isFunctionValue(arg) && state.verb != 'p' && state.verb != 'T' { - f.Badf(call.Pos(), "%s format %s arg %s is a func value, not called", state.name, state.format, f.gofmt(arg)) + if isFunctionValue(pass, arg) && state.verb != 'p' && state.verb != 'T' { + pass.Reportf(call.Pos(), "%s format %s arg %s is a func value, not called", state.name, state.format, analysisutil.Format(pass.Fset, arg)) return false } - if !f.matchArgType(v.typ, nil, arg) { + if !matchArgType(pass, v.typ, nil, arg) { typeString := "" - if typ := f.pkg.types[arg].Type; typ != nil { + if typ := pass.TypesInfo.Types[arg].Type; typ != nil { typeString = typ.String() } - f.Badf(call.Pos(), "%s format %s has arg %s of wrong type %s", state.name, state.format, f.gofmt(arg), typeString) + pass.Reportf(call.Pos(), "%s format %s has arg %s of wrong type %s", state.name, state.format, analysisutil.Format(pass.Fset, arg), typeString) return false } - if v.typ&argString != 0 && v.verb != 'T' && !bytes.Contains(state.flags, []byte{'#'}) && f.recursiveStringer(arg) { - f.Badf(call.Pos(), "%s format %s with arg %s causes recursive String method call", state.name, state.format, f.gofmt(arg)) + if v.typ&argString != 0 && v.verb != 'T' && !bytes.Contains(state.flags, []byte{'#'}) && recursiveStringer(pass, arg) { + pass.Reportf(call.Pos(), "%s format %s with arg %s causes recursive String method call", state.name, state.format, analysisutil.Format(pass.Fset, arg)) return false } return true } -// recursiveStringer reports whether the provided argument is r or &r for the -// fmt.Stringer receiver identifier r. -func (f *File) recursiveStringer(e ast.Expr) bool { - if len(f.stringerPtrs) == 0 { - return false - } - ptr := false - var obj *ast.Object - switch e := e.(type) { - case *ast.Ident: - obj = e.Obj - case *ast.UnaryExpr: - if id, ok := e.X.(*ast.Ident); ok && e.Op == token.AND { - obj = id.Obj - ptr = true - } - } +// recursiveStringer reports whether the argument e is a potential +// recursive call to stringer, such as t and &t in these examples: +// +// func (t *T) String() string { printf("%s", t) } +// func (t T) String() string { printf("%s", t) } +// func (t T) String() string { printf("%s", &t) } +// +func recursiveStringer(pass *analysis.Pass, e ast.Expr) bool { + typ := pass.TypesInfo.Types[e].Type // It's unlikely to be a recursive stringer if it has a Format method. - if typ := f.pkg.types[e].Type; typ != nil { - if f.isFormatter(typ) { - return false - } + if isFormatter(pass, typ) { + return false } - // We compare the underlying Object, which checks that the identifier - // is the one we declared as the receiver for the String method in - // which this printf appears. - ptrRecv, exist := f.stringerPtrs[obj] - if !exist { + // Does e allow e.String()? + obj, _, _ := types.LookupFieldOrMethod(typ, false, pass.Pkg, "String") + stringMethod, ok := obj.(*types.Func) + if !ok { return false } - // We also need to check that using &t when we declared String - // on (t *T) is ok; in such a case, the address is printed. - if ptr && ptrRecv { - return false - } - return true + + // Is the expression e within the body of that String method? + return stringMethod.Pkg() == pass.Pkg && stringMethod.Scope().Contains(e.Pos()) } // isFunctionValue reports whether the expression is a function as opposed to a function call. // It is almost always a mistake to print a function value. -func (f *File) isFunctionValue(e ast.Expr) bool { - if typ := f.pkg.types[e].Type; typ != nil { +func isFunctionValue(pass *analysis.Pass, e ast.Expr) bool { + if typ := pass.TypesInfo.Types[e].Type; typ != nil { _, ok := typ.(*types.Signature) return ok } @@ -955,7 +821,7 @@ func (f *File) isFunctionValue(e ast.Expr) bool { // argCanBeChecked reports whether the specified argument is statically present; // it may be beyond the list of arguments or in a terminal slice... argument, which // means we can't see it. -func (f *File) argCanBeChecked(call *ast.CallExpr, formatArg int, state *formatState) bool { +func argCanBeChecked(pass *analysis.Pass, call *ast.CallExpr, formatArg int, state *formatState) bool { argNum := state.argNums[formatArg] if argNum <= 0 { // Shouldn't happen, so catch it with prejudice. @@ -973,7 +839,7 @@ func (f *File) argCanBeChecked(call *ast.CallExpr, formatArg int, state *formatS // There are bad indexes in the format or there are fewer arguments than the format needs. // 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(), "%s format %s reads arg #%d, but call has %v", state.name, state.format, arg, count(len(call.Args)-state.firstArg, "arg")) + pass.Reportf(call.Pos(), "%s format %s reads arg #%d, but call has %v", state.name, state.format, arg, count(len(call.Args)-state.firstArg, "arg")) return false } @@ -990,9 +856,9 @@ const ( ) // checkPrint checks a call to an unformatted print routine such as Println. -func (f *File) checkPrint(call *ast.CallExpr, name string) { +func checkPrint(pass *analysis.Pass, call *ast.CallExpr, fn *types.Func) { firstArg := 0 - typ := f.pkg.types[call.Fun].Type + typ := pass.TypesInfo.Types[call.Fun].Type if typ == nil { // Skip checking functions with unknown type. return @@ -1024,7 +890,7 @@ func (f *File) checkPrint(call *ast.CallExpr, name string) { if sel, ok := call.Args[0].(*ast.SelectorExpr); ok { if x, ok := sel.X.(*ast.Ident); ok { if x.Name == "os" && strings.HasPrefix(sel.Sel.Name, "Std") { - f.Badf(call.Pos(), "%s does not take io.Writer but has first arg %s", name, f.gofmt(call.Args[0])) + pass.Reportf(call.Pos(), "%s does not take io.Writer but has first arg %s", fn.Name(), analysisutil.Format(pass.Fset, call.Args[0])) } } } @@ -1038,26 +904,26 @@ func (f *File) checkPrint(call *ast.CallExpr, name string) { if strings.Contains(s, "%") { m := printFormatRE.FindStringSubmatch(s) if m != nil { - f.Badf(call.Pos(), "%s call has possible formatting directive %s", name, m[0]) + pass.Reportf(call.Pos(), "%s call has possible formatting directive %s", fn.Name(), m[0]) } } } - if strings.HasSuffix(name, "ln") { + if strings.HasSuffix(fn.Name(), "ln") { // The last item, if a string, should not have a newline. arg = args[len(args)-1] if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRING { str, _ := strconv.Unquote(lit.Value) if strings.HasSuffix(str, "\n") { - f.Badf(call.Pos(), "%s arg list ends with redundant newline", name) + pass.Reportf(call.Pos(), "%s arg list ends with redundant newline", fn.Name()) } } } for _, arg := range args { - if f.isFunctionValue(arg) { - f.Badf(call.Pos(), "%s arg %s is a func value, not called", name, f.gofmt(arg)) + if isFunctionValue(pass, arg) { + pass.Reportf(call.Pos(), "%s arg %s is a func value, not called", fn.Name(), analysisutil.Format(pass.Fset, arg)) } - if f.recursiveStringer(arg) { - f.Badf(call.Pos(), "%s arg %s causes recursive call to String method", name, f.gofmt(arg)) + if recursiveStringer(pass, arg) { + pass.Reportf(call.Pos(), "%s arg %s causes recursive call to String method", fn.Name(), analysisutil.Format(pass.Fset, arg)) } } } @@ -1070,3 +936,29 @@ func count(n int, what string) string { } return fmt.Sprintf("%d %ss", n, what) } + +// stringSet is a set-of-nonempty-strings-valued flag. +// Note: elements without a '.' get lower-cased. +type stringSet map[string]bool + +func (ss stringSet) String() string { + var list []string + for name := range ss { + list = append(list, name) + } + sort.Strings(list) + return strings.Join(list, ",") +} + +func (ss stringSet) Set(flag string) error { + for _, name := range strings.Split(flag, ",") { + if len(name) == 0 { + return fmt.Errorf("empty string") + } + if !strings.Contains(name, ".") { + name = strings.ToLower(name) + } + ss[name] = true + } + return nil +} diff --git a/go/analysis/passes/printf/printf_test.go b/go/analysis/passes/printf/printf_test.go new file mode 100644 index 00000000..937bbe7a --- /dev/null +++ b/go/analysis/passes/printf/printf_test.go @@ -0,0 +1,14 @@ +package printf_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/printf" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + printf.Analyzer.Flags.Set("funcs", "Warn,Warnf") + analysistest.Run(t, testdata, printf.Analyzer, "a", "b") +} diff --git a/go/analysis/passes/printf/testdata/src/a/a.go b/go/analysis/passes/printf/testdata/src/a/a.go index 88163b59..b73d1cad 100644 --- a/go/analysis/passes/printf/testdata/src/a/a.go +++ b/go/analysis/passes/printf/testdata/src/a/a.go @@ -4,7 +4,7 @@ // This file contains tests for the printf checker. -package testdata +package a import ( "fmt" @@ -13,8 +13,10 @@ import ( "os" "testing" "unsafe" // just for test case printing unsafe.Pointer + // For testing printf-like functions from external package. // "github.com/foobar/externalprintf" + "b" ) func UnsafePointerPrintfTest() { @@ -104,92 +106,92 @@ func PrintfTests() { fmt.Printf("%g", 1+2i) fmt.Printf("%#e %#E %#f %#F %#g %#G", 1.2, 1.2, 1.2, 1.2, 1.2, 1.2) // OK since Go 1.9 // Some bad format/argTypes - fmt.Printf("%b", "hi") // ERROR "Printf format %b has arg \x22hi\x22 of wrong type string" - fmt.Printf("%t", c) // ERROR "Printf format %t has arg c of wrong type complex64" - fmt.Printf("%t", 1+2i) // ERROR "Printf format %t has arg 1 \+ 2i of wrong type complex128" - fmt.Printf("%c", 2.3) // ERROR "Printf format %c has arg 2.3 of wrong type float64" - fmt.Printf("%d", 2.3) // ERROR "Printf format %d has arg 2.3 of wrong type float64" - fmt.Printf("%e", "hi") // ERROR "Printf format %e has arg \x22hi\x22 of wrong type string" - fmt.Printf("%E", true) // ERROR "Printf format %E has arg true of wrong type bool" - fmt.Printf("%f", "hi") // ERROR "Printf format %f has arg \x22hi\x22 of wrong type string" - fmt.Printf("%F", 'x') // ERROR "Printf format %F has arg 'x' of wrong type rune" - fmt.Printf("%g", "hi") // ERROR "Printf format %g has arg \x22hi\x22 of wrong type string" - fmt.Printf("%g", imap) // ERROR "Printf format %g has arg imap of wrong type map\[int\]int" - fmt.Printf("%G", i) // ERROR "Printf format %G has arg i of wrong type int" - fmt.Printf("%o", x) // ERROR "Printf format %o has arg x of wrong type float64" - fmt.Printf("%p", nil) // ERROR "Printf format %p has arg nil of wrong type untyped nil" - fmt.Printf("%p", 23) // ERROR "Printf format %p has arg 23 of wrong type int" - fmt.Printf("%q", x) // ERROR "Printf format %q has arg x of wrong type float64" - fmt.Printf("%s", b) // ERROR "Printf format %s has arg b of wrong type bool" - fmt.Printf("%s", byte(65)) // ERROR "Printf format %s has arg byte\(65\) of wrong type byte" - fmt.Printf("%t", 23) // ERROR "Printf format %t has arg 23 of wrong type int" - fmt.Printf("%U", x) // ERROR "Printf format %U has arg x of wrong type float64" - fmt.Printf("%x", nil) // ERROR "Printf format %x has arg nil of wrong type untyped nil" - fmt.Printf("%X", 2.3) // ERROR "Printf format %X has arg 2.3 of wrong type float64" - fmt.Printf("%s", stringerv) // ERROR "Printf format %s has arg stringerv of wrong type testdata.ptrStringer" - fmt.Printf("%t", stringerv) // ERROR "Printf format %t has arg stringerv of wrong type testdata.ptrStringer" - fmt.Printf("%s", embeddedStringerv) // ERROR "Printf format %s has arg embeddedStringerv of wrong type testdata.embeddedStringer" - fmt.Printf("%t", embeddedStringerv) // ERROR "Printf format %t has arg embeddedStringerv of wrong type testdata.embeddedStringer" - fmt.Printf("%q", notstringerv) // ERROR "Printf format %q has arg notstringerv of wrong type testdata.notstringer" - fmt.Printf("%t", notstringerv) // ERROR "Printf format %t has arg notstringerv of wrong type testdata.notstringer" - fmt.Printf("%t", stringerarrayv) // ERROR "Printf format %t has arg stringerarrayv of wrong type testdata.stringerarray" - fmt.Printf("%t", notstringerarrayv) // ERROR "Printf format %t has arg notstringerarrayv of wrong type testdata.notstringerarray" - fmt.Printf("%q", notstringerarrayv) // ERROR "Printf format %q has arg notstringerarrayv of wrong type testdata.notstringerarray" - fmt.Printf("%d", BoolFormatter(true)) // ERROR "Printf format %d has arg BoolFormatter\(true\) of wrong type testdata.BoolFormatter" + fmt.Printf("%b", "hi") // want "Printf format %b has arg \x22hi\x22 of wrong type string" + fmt.Printf("%t", c) // want "Printf format %t has arg c of wrong type complex64" + fmt.Printf("%t", 1+2i) // want `Printf format %t has arg 1 \+ 2i of wrong type complex128` + fmt.Printf("%c", 2.3) // want "Printf format %c has arg 2.3 of wrong type float64" + fmt.Printf("%d", 2.3) // want "Printf format %d has arg 2.3 of wrong type float64" + fmt.Printf("%e", "hi") // want `Printf format %e has arg "hi" of wrong type string` + fmt.Printf("%E", true) // want "Printf format %E has arg true of wrong type bool" + fmt.Printf("%f", "hi") // want "Printf format %f has arg \x22hi\x22 of wrong type string" + fmt.Printf("%F", 'x') // want "Printf format %F has arg 'x' of wrong type rune" + fmt.Printf("%g", "hi") // want `Printf format %g has arg "hi" of wrong type string` + fmt.Printf("%g", imap) // want `Printf format %g has arg imap of wrong type map\[int\]int` + fmt.Printf("%G", i) // want "Printf format %G has arg i of wrong type int" + fmt.Printf("%o", x) // want "Printf format %o has arg x of wrong type float64" + fmt.Printf("%p", nil) // want "Printf format %p has arg nil of wrong type untyped nil" + fmt.Printf("%p", 23) // want "Printf format %p has arg 23 of wrong type int" + fmt.Printf("%q", x) // want "Printf format %q has arg x of wrong type float64" + fmt.Printf("%s", b) // want "Printf format %s has arg b of wrong type bool" + fmt.Printf("%s", byte(65)) // want `Printf format %s has arg byte\(65\) of wrong type byte` + fmt.Printf("%t", 23) // want "Printf format %t has arg 23 of wrong type int" + fmt.Printf("%U", x) // want "Printf format %U has arg x of wrong type float64" + fmt.Printf("%x", nil) // want "Printf format %x has arg nil of wrong type untyped nil" + fmt.Printf("%X", 2.3) // want "Printf format %X has arg 2.3 of wrong type float64" + fmt.Printf("%s", stringerv) // want "Printf format %s has arg stringerv of wrong type a.ptrStringer" + fmt.Printf("%t", stringerv) // want "Printf format %t has arg stringerv of wrong type a.ptrStringer" + fmt.Printf("%s", embeddedStringerv) // want "Printf format %s has arg embeddedStringerv of wrong type a.embeddedStringer" + fmt.Printf("%t", embeddedStringerv) // want "Printf format %t has arg embeddedStringerv of wrong type a.embeddedStringer" + fmt.Printf("%q", notstringerv) // want "Printf format %q has arg notstringerv of wrong type a.notstringer" + fmt.Printf("%t", notstringerv) // want "Printf format %t has arg notstringerv of wrong type a.notstringer" + fmt.Printf("%t", stringerarrayv) // want "Printf format %t has arg stringerarrayv of wrong type a.stringerarray" + fmt.Printf("%t", notstringerarrayv) // want "Printf format %t has arg notstringerarrayv of wrong type a.notstringerarray" + fmt.Printf("%q", notstringerarrayv) // want "Printf format %q has arg notstringerarrayv of wrong type a.notstringerarray" + fmt.Printf("%d", BoolFormatter(true)) // want `Printf format %d has arg BoolFormatter\(true\) of wrong type a.BoolFormatter` fmt.Printf("%z", FormatterVal(true)) // correct (the type is responsible for formatting) fmt.Printf("%d", FormatterVal(true)) // correct (the type is responsible for formatting) fmt.Printf("%s", nonemptyinterface) // correct (the type is responsible for formatting) - fmt.Printf("%.*s %d %6g", 3, "hi", 23, 'x') // ERROR "Printf format %6g has arg 'x' of wrong type rune" + fmt.Printf("%.*s %d %6g", 3, "hi", 23, 'x') // want "Printf format %6g has arg 'x' of wrong type rune" fmt.Println() // not an error - fmt.Println("%s", "hi") // ERROR "Println call has possible formatting directive %s" - fmt.Println("%v", "hi") // ERROR "Println call has possible formatting directive %v" - fmt.Println("%T", "hi") // ERROR "Println call has possible formatting directive %T" + fmt.Println("%s", "hi") // want "Println call has possible formatting directive %s" + fmt.Println("%v", "hi") // want "Println call has possible formatting directive %v" + fmt.Println("%T", "hi") // want "Println call has possible formatting directive %T" fmt.Println("0.0%") // correct (trailing % couldn't be a formatting directive) - fmt.Printf("%s", "hi", 3) // ERROR "Printf call needs 1 arg but has 2 args" - _ = fmt.Sprintf("%"+("s"), "hi", 3) // ERROR "Sprintf call needs 1 arg but has 2 args" + fmt.Printf("%s", "hi", 3) // want "Printf call needs 1 arg but has 2 args" + _ = fmt.Sprintf("%"+("s"), "hi", 3) // want "Sprintf call needs 1 arg but has 2 args" 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 x", 3, 3, 3, 3) // ERROR "Printf call needs 2 args but has 4 args" - fmt.Printf("%.*d x", "hi", 3) // ERROR "Printf format %.*d uses non-int \x22hi\x22 as argument of \*" + fmt.Printf("%.*d x", 3, 3, 3, 3) // want "Printf call needs 2 args but has 4 args" + fmt.Printf("%.*d x", "hi", 3) // want `Printf format %.*d uses non-int "hi" as argument of \*` fmt.Printf("%.*d x", i, 3) // correct - fmt.Printf("%.*d x", s, 3) // ERROR "Printf format %.\*d uses non-int s as argument of \*" - fmt.Printf("%*% x", 0.22) // ERROR "Printf format %\*% uses non-int 0.22 as argument of \*" + fmt.Printf("%.*d x", s, 3) // want `Printf format %.\*d uses non-int s as argument of \*` + fmt.Printf("%*% x", 0.22) // want `Printf format %\*% uses non-int 0.22 as argument of \*` fmt.Printf("%q %q", multi()...) // ok fmt.Printf("%#q", `blah`) // ok // printf("now is the time", "buddy") // no error "printf call has arguments but no formatting directives" - Printf("now is the time", "buddy") // ERROR "Printf call has arguments but no formatting directives" + Printf("now is the time", "buddy") // want "Printf call has arguments but no formatting directives" Printf("hi") // ok const format = "%s %s\n" Printf(format, "hi", "there") - Printf(format, "hi") // ERROR "Printf format %s reads arg #2, but call has 1 arg$" - Printf("%s %d %.3v %q", "str", 4) // ERROR "Printf format %.3v reads arg #3, but call has 2 args" + Printf(format, "hi") // want "Printf format %s reads arg #2, but call has 1 arg$" + Printf("%s %d %.3v %q", "str", 4) // want "Printf format %.3v reads arg #3, but call has 2 args" f := new(ptrStringer) - f.Warn(0, "%s", "hello", 3) // ERROR "Warn call has possible formatting directive %s" - f.Warnf(0, "%s", "hello", 3) // ERROR "Warnf call needs 1 arg but has 2 args" - f.Warnf(0, "%r", "hello") // ERROR "Warnf format %r has unknown verb r" - f.Warnf(0, "%#s", "hello") // ERROR "Warnf format %#s has unrecognized flag #" - f.Warn2(0, "%s", "hello", 3) // ERROR "Warn2 call has possible formatting directive %s" - f.Warnf2(0, "%s", "hello", 3) // ERROR "Warnf2 call needs 1 arg but has 2 args" - f.Warnf2(0, "%r", "hello") // ERROR "Warnf2 format %r has unknown verb r" - f.Warnf2(0, "%#s", "hello") // ERROR "Warnf2 format %#s has unrecognized flag #" - f.Wrap(0, "%s", "hello", 3) // ERROR "Wrap call has possible formatting directive %s" - f.Wrapf(0, "%s", "hello", 3) // ERROR "Wrapf call needs 1 arg but has 2 args" - f.Wrapf(0, "%r", "hello") // ERROR "Wrapf format %r has unknown verb r" - f.Wrapf(0, "%#s", "hello") // ERROR "Wrapf format %#s has unrecognized flag #" - f.Wrap2(0, "%s", "hello", 3) // ERROR "Wrap2 call has possible formatting directive %s" - f.Wrapf2(0, "%s", "hello", 3) // ERROR "Wrapf2 call needs 1 arg but has 2 args" - f.Wrapf2(0, "%r", "hello") // ERROR "Wrapf2 format %r has unknown verb r" - f.Wrapf2(0, "%#s", "hello") // ERROR "Wrapf2 format %#s has unrecognized flag #" + f.Warn(0, "%s", "hello", 3) // want "Warn call has possible formatting directive %s" + f.Warnf(0, "%s", "hello", 3) // want "Warnf call needs 1 arg but has 2 args" + f.Warnf(0, "%r", "hello") // want "Warnf format %r has unknown verb r" + f.Warnf(0, "%#s", "hello") // want "Warnf format %#s has unrecognized flag #" + f.Warn2(0, "%s", "hello", 3) // want "Warn2 call has possible formatting directive %s" + f.Warnf2(0, "%s", "hello", 3) // want "Warnf2 call needs 1 arg but has 2 args" + f.Warnf2(0, "%r", "hello") // want "Warnf2 format %r has unknown verb r" + f.Warnf2(0, "%#s", "hello") // want "Warnf2 format %#s has unrecognized flag #" + f.Wrap(0, "%s", "hello", 3) // want "Wrap call has possible formatting directive %s" + f.Wrapf(0, "%s", "hello", 3) // want "Wrapf call needs 1 arg but has 2 args" + f.Wrapf(0, "%r", "hello") // want "Wrapf format %r has unknown verb r" + f.Wrapf(0, "%#s", "hello") // want "Wrapf format %#s has unrecognized flag #" + f.Wrap2(0, "%s", "hello", 3) // want "Wrap2 call has possible formatting directive %s" + f.Wrapf2(0, "%s", "hello", 3) // want "Wrapf2 call needs 1 arg but has 2 args" + f.Wrapf2(0, "%r", "hello") // want "Wrapf2 format %r has unknown verb r" + f.Wrapf2(0, "%#s", "hello") // want "Wrapf2 format %#s has unrecognized flag #" fmt.Printf("%#s", FormatterVal(true)) // correct (the type is responsible for formatting) - Printf("d%", 2) // ERROR "Printf format % is missing verb at end of string" + Printf("d%", 2) // want "Printf format % is missing verb at end of string" Printf("%d", percentDV) Printf("%d", &percentDV) - Printf("%d", notPercentDV) // ERROR "Printf format %d has arg notPercentDV of wrong type testdata.notPercentDStruct" - Printf("%d", ¬PercentDV) // ERROR "Printf format %d has arg ¬PercentDV of wrong type \*testdata.notPercentDStruct" + Printf("%d", notPercentDV) // want "Printf format %d has arg notPercentDV of wrong type a.notPercentDStruct" + Printf("%d", ¬PercentDV) // want `Printf format %d has arg ¬PercentDV of wrong type \*a.notPercentDStruct` Printf("%p", ¬PercentDV) // Works regardless: we print it as a pointer. - Printf("%q", &percentDV) // ERROR "Printf format %q has arg &percentDV of wrong type \*testdata.percentDStruct" + Printf("%q", &percentDV) // want `Printf format %q has arg &percentDV of wrong type \*a.percentDStruct` Printf("%s", percentSV) Printf("%s", &percentSV) // Good argument reorderings. @@ -199,22 +201,23 @@ func PrintfTests() { Printf("%[2]*.[1]*[3]d", 2, 3, 4) fmt.Fprintf(os.Stderr, "%[2]*.[1]*[3]d", 2, 3, 4) // Use Fprintf to make sure we count arguments correctly. // Bad argument reorderings. - Printf("%[xd", 3) // ERROR "Printf format %\[xd is missing closing \]" - Printf("%[x]d x", 3) // ERROR "Printf format has invalid argument index \[x\]" - Printf("%[3]*s x", "hi", 2) // ERROR "Printf format has invalid argument index \[3\]" - _ = fmt.Sprintf("%[3]d x", 2) // ERROR "Sprintf format has invalid argument index \[3\]" - Printf("%[2]*.[1]*[3]d x", 2, "hi", 4) // ERROR "Printf format %\[2]\*\.\[1\]\*\[3\]d uses non-int \x22hi\x22 as argument of \*" - Printf("%[0]s x", "arg1") // ERROR "Printf format has invalid argument index \[0\]" - Printf("%[0]d x", 1) // ERROR "Printf format has invalid argument index \[0\]" + Printf("%[xd", 3) // want `Printf format %\[xd is missing closing \]` + Printf("%[x]d x", 3) // want `Printf format has invalid argument index \[x\]` + Printf("%[3]*s x", "hi", 2) // want `Printf format has invalid argument index \[3\]` + _ = fmt.Sprintf("%[3]d x", 2) // want `Sprintf format has invalid argument index \[3\]` + Printf("%[2]*.[1]*[3]d x", 2, "hi", 4) // want `Printf format %\[2]\*\.\[1\]\*\[3\]d uses non-int \x22hi\x22 as argument of \*` + Printf("%[0]s x", "arg1") // want `Printf format has invalid argument index \[0\]` + Printf("%[0]d x", 1) // want `Printf format has invalid argument index \[0\]` // Something that satisfies the error interface. var e error fmt.Println(e.Error()) // ok // Something that looks like an error interface but isn't, such as the (*T).Error method // in the testing package. var et1 *testing.T - et1.Error() // ok - et1.Error("hi") // ok - et1.Error("%d", 3) // ERROR "Error call has possible formatting directive %d" + et1.Error() // ok + et1.Error("hi") // ok + et1.Error("%d", 3) // want "Error call has possible formatting directive %d" + et1.Errorf("%s", 1) // want "Errorf format %s has arg 1 of wrong type int" var et3 errorTest3 et3.Error() // ok, not an error method. var et4 errorTest4 @@ -227,9 +230,9 @@ func PrintfTests() { } fmt.Printf("%f", iface) // ok: fmt treats interfaces as transparent and iface may well have a float concrete type // Can't print a function. - Printf("%d", someFunction) // ERROR "Printf format %d arg someFunction is a func value, not called" - Printf("%v", someFunction) // ERROR "Printf format %v arg someFunction is a func value, not called" - Println(someFunction) // ERROR "Println arg someFunction is a func value, not called" + Printf("%d", someFunction) // want "Printf format %d arg someFunction is a func value, not called" + Printf("%v", someFunction) // want "Printf format %v arg someFunction is a func value, not called" + Println(someFunction) // want "Println arg someFunction is a func value, not called" Printf("%p", someFunction) // ok: maybe someone wants to see the pointer Printf("%T", someFunction) // ok: maybe someone wants to see the type // Bug: used to recur forever. @@ -240,17 +243,20 @@ func PrintfTests() { // Special handling for Log. math.Log(3) // OK var t *testing.T - t.Log("%d", 3) // ERROR "Log call has possible formatting directive %d" + t.Log("%d", 3) // want "Log call has possible formatting directive %d" t.Logf("%d", 3) - t.Logf("%d", "hi") // ERROR "Logf format %d has arg \x22hi\x22 of wrong type string" + t.Logf("%d", "hi") // want `Logf format %d has arg "hi" of wrong type string` Errorf(1, "%d", 3) // OK - Errorf(1, "%d", "hi") // ERROR "Errorf format %d has arg \x22hi\x22 of wrong type string" + Errorf(1, "%d", "hi") // want `Errorf format %d has arg "hi" 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") // no error "errorf format %d reads arg #1, but call has 0 args" + errorf("ERROR", "%d") // want "errorf format %d reads arg #1, but call has 0 args" + + var tb testing.TB + tb.Errorf("%s", 1) // want "Errorf format %s has arg 1 of wrong type int" // Printf from external package // externalprintf.Printf("%d", 42) // OK @@ -276,42 +282,42 @@ func PrintfTests() { // indexed arguments Printf("%d %[3]d %d %[2]d x", 1, 2, 3, 4) // OK - Printf("%d %[0]d %d %[2]d x", 1, 2, 3, 4) // ERROR "Printf format has invalid argument index \[0\]" - Printf("%d %[3]d %d %[-2]d x", 1, 2, 3, 4) // ERROR "Printf format has invalid argument index \[-2\]" - Printf("%d %[3]d %d %[2234234234234]d x", 1, 2, 3, 4) // ERROR "Printf format has invalid argument index \[2234234234234\]" - Printf("%d %[3]d %-10d %[2]d x", 1, 2, 3) // ERROR "Printf format %-10d reads arg #4, but call has 3 args" - Printf("%[1][3]d x", 1, 2) // ERROR "Printf format %\[1\]\[ has unknown verb \[" + Printf("%d %[0]d %d %[2]d x", 1, 2, 3, 4) // want `Printf format has invalid argument index \[0\]` + Printf("%d %[3]d %d %[-2]d x", 1, 2, 3, 4) // want `Printf format has invalid argument index \[-2\]` + Printf("%d %[3]d %d %[2234234234234]d x", 1, 2, 3, 4) // want `Printf format has invalid argument index \[2234234234234\]` + Printf("%d %[3]d %-10d %[2]d x", 1, 2, 3) // want "Printf format %-10d reads arg #4, but call has 3 args" + Printf("%[1][3]d x", 1, 2) // want `Printf format %\[1\]\[ has unknown verb \[` Printf("%[1]d x", 1, 2) // OK Printf("%d %[3]d %d %[2]d x", 1, 2, 3, 4, 5) // OK // wrote Println but meant Fprintln Printf("%p\n", os.Stdout) // OK - Println(os.Stdout, "hello") // ERROR "Println does not take io.Writer but has first arg os.Stdout" + Println(os.Stdout, "hello") // want "Println does not take io.Writer but has first arg os.Stdout" Printf(someString(), "hello") // OK // Printf wrappers in package log should be detected automatically - logpkg.Fatal("%d", 1) // ERROR "Fatal call has possible formatting directive %d" - logpkg.Fatalf("%d", "x") // ERROR "Fatalf format %d has arg \x22x\x22 of wrong type string" - logpkg.Fatalln("%d", 1) // ERROR "Fatalln call has possible formatting directive %d" - logpkg.Panic("%d", 1) // ERROR "Panic call has possible formatting directive %d" - logpkg.Panicf("%d", "x") // ERROR "Panicf format %d has arg \x22x\x22 of wrong type string" - logpkg.Panicln("%d", 1) // ERROR "Panicln call has possible formatting directive %d" - logpkg.Print("%d", 1) // ERROR "Print call has possible formatting directive %d" - logpkg.Printf("%d", "x") // ERROR "Printf format %d has arg \x22x\x22 of wrong type string" - logpkg.Println("%d", 1) // ERROR "Println call has possible formatting directive %d" + logpkg.Fatal("%d", 1) // want "Fatal call has possible formatting directive %d" + logpkg.Fatalf("%d", "x") // want `Fatalf format %d has arg "x" of wrong type string` + logpkg.Fatalln("%d", 1) // want "Fatalln call has possible formatting directive %d" + logpkg.Panic("%d", 1) // want "Panic call has possible formatting directive %d" + logpkg.Panicf("%d", "x") // want `Panicf format %d has arg "x" of wrong type string` + logpkg.Panicln("%d", 1) // want "Panicln call has possible formatting directive %d" + logpkg.Print("%d", 1) // want "Print call has possible formatting directive %d" + logpkg.Printf("%d", "x") // want `Printf format %d has arg "x" of wrong type string` + logpkg.Println("%d", 1) // want "Println call has possible formatting directive %d" // Methods too. var l *logpkg.Logger - l.Fatal("%d", 1) // ERROR "Fatal call has possible formatting directive %d" - l.Fatalf("%d", "x") // ERROR "Fatalf format %d has arg \x22x\x22 of wrong type string" - l.Fatalln("%d", 1) // ERROR "Fatalln call has possible formatting directive %d" - l.Panic("%d", 1) // ERROR "Panic call has possible formatting directive %d" - l.Panicf("%d", "x") // ERROR "Panicf format %d has arg \x22x\x22 of wrong type string" - l.Panicln("%d", 1) // ERROR "Panicln call has possible formatting directive %d" - l.Print("%d", 1) // ERROR "Print call has possible formatting directive %d" - l.Printf("%d", "x") // ERROR "Printf format %d has arg \x22x\x22 of wrong type string" - l.Println("%d", 1) // ERROR "Println call has possible formatting directive %d" + l.Fatal("%d", 1) // want "Fatal call has possible formatting directive %d" + l.Fatalf("%d", "x") // want `Fatalf format %d has arg "x" of wrong type string` + l.Fatalln("%d", 1) // want "Fatalln call has possible formatting directive %d" + l.Panic("%d", 1) // want "Panic call has possible formatting directive %d" + l.Panicf("%d", "x") // want `Panicf format %d has arg "x" of wrong type string` + l.Panicln("%d", 1) // want "Panicln call has possible formatting directive %d" + l.Print("%d", 1) // want "Print call has possible formatting directive %d" + l.Printf("%d", "x") // want `Printf format %d has arg "x" of wrong type string` + l.Println("%d", 1) // want "Println call has possible formatting directive %d" // Issue 26486 dbg("", 1) // no error "call has arguments but no formatting directive" @@ -343,29 +349,29 @@ func (ss *someStruct) log(f func(), args ...interface{}) {} func someFunction() {} // Printf is used by the test so we must declare it. -func Printf(format string, args ...interface{}) { +func Printf(format string, args ...interface{}) { // want Printf:"printfWrapper" fmt.Printf(format, args...) } // Println is used by the test so we must declare it. -func Println(args ...interface{}) { +func Println(args ...interface{}) { // want Println:"printWrapper" fmt.Println(args...) } // printf is used by the test so we must declare it. -func printf(format string, args ...interface{}) { +func printf(format string, args ...interface{}) { // want printf:"printfWrapper" fmt.Printf(format, args...) } // 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{}) { +func Errorf(i int, format string, args ...interface{}) { // want Errorf:"printfWrapper" _ = fmt.Errorf(format, args...) } // 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{}) { +func errorf(level, format string, args ...interface{}) { // want errorf:"printfWrapper" _ = fmt.Errorf(format, args...) } @@ -386,44 +392,46 @@ func (*ptrStringer) String() string { return "string" } -func (p *ptrStringer) Warn2(x int, args ...interface{}) string { +func (p *ptrStringer) Warn2(x int, args ...interface{}) string { // want Warn2:"printWrapper" return p.Warn(x, args...) } -func (p *ptrStringer) Warnf2(x int, format string, args ...interface{}) string { +func (p *ptrStringer) Warnf2(x int, format string, args ...interface{}) string { // want Warnf2:"printfWrapper" return p.Warnf(x, format, args...) } +// During testing -printf.funcs flag matches Warn. func (*ptrStringer) Warn(x int, args ...interface{}) string { return "warn" } +// During testing -printf.funcs flag matches Warnf. func (*ptrStringer) Warnf(x int, format string, args ...interface{}) string { return "warnf" } -func (p *ptrStringer) Wrap2(x int, args ...interface{}) string { +func (p *ptrStringer) Wrap2(x int, args ...interface{}) string { // want Wrap2:"printWrapper" return p.Wrap(x, args...) } -func (p *ptrStringer) Wrapf2(x int, format string, args ...interface{}) string { +func (p *ptrStringer) Wrapf2(x int, format string, args ...interface{}) string { // want Wrapf2:"printfWrapper" return p.Wrapf(x, format, args...) } -func (*ptrStringer) Wrap(x int, args ...interface{}) string { +func (*ptrStringer) Wrap(x int, args ...interface{}) string { // want Wrap:"printWrapper" return fmt.Sprint(args...) } -func (*ptrStringer) Wrapf(x int, format string, args ...interface{}) string { +func (*ptrStringer) Wrapf(x int, format string, args ...interface{}) string { // want Wrapf:"printfWrapper" return fmt.Sprintf(format, args...) } func (*ptrStringer) BadWrap(x int, args ...interface{}) string { - return fmt.Sprint(args) // ERROR "missing ... in args forwarded to print-like function" + return fmt.Sprint(args) // want "missing ... in args forwarded to print-like function" } func (*ptrStringer) BadWrapf(x int, format string, args ...interface{}) string { - return fmt.Sprintf(format, args) // ERROR "missing ... in args forwarded to printf-like function" + return fmt.Sprintf(format, args) // want "missing ... in args forwarded to printf-like function" } func (*ptrStringer) WrapfFalsePositive(x int, arg1 string, arg2 ...interface{}) string { @@ -492,10 +500,10 @@ type recursiveStringer int func (s recursiveStringer) String() string { _ = fmt.Sprintf("%d", s) _ = fmt.Sprintf("%#v", s) - _ = fmt.Sprintf("%v", s) // ERROR "Sprintf format %v with arg s causes recursive String method call" - _ = fmt.Sprintf("%v", &s) // ERROR "Sprintf format %v with arg &s causes recursive String method call" + _ = fmt.Sprintf("%v", s) // want "Sprintf format %v with arg s causes recursive String method call" + _ = fmt.Sprintf("%v", &s) // want "Sprintf format %v with arg &s causes recursive String method call" _ = fmt.Sprintf("%T", s) // ok; does not recursively call String - return fmt.Sprintln(s) // ERROR "Sprintln arg s causes recursive call to String method" + return fmt.Sprintln(s) // want "Sprintln arg s causes recursive call to String method" } type recursivePtrStringer int @@ -503,7 +511,7 @@ type recursivePtrStringer int func (p *recursivePtrStringer) String() string { _ = fmt.Sprintf("%v", *p) _ = fmt.Sprint(&p) // ok; prints address - return fmt.Sprintln(p) // ERROR "Sprintln arg p causes recursive call to String method" + return fmt.Sprintln(p) // want "Sprintln arg p causes recursive call to String method" } type BoolFormatter bool @@ -587,47 +595,47 @@ func UnexportedStringerOrError() { fmt.Printf("%s", unexportedInterface{3}) // ok; we can't see the problem us := unexportedStringer{} - fmt.Printf("%s", us) // ERROR "Printf format %s has arg us of wrong type testdata.unexportedStringer" - fmt.Printf("%s", &us) // ERROR "Printf format %s has arg &us of wrong type [*]testdata.unexportedStringer" + fmt.Printf("%s", us) // want "Printf format %s has arg us of wrong type a.unexportedStringer" + fmt.Printf("%s", &us) // want "Printf format %s has arg &us of wrong type [*]a.unexportedStringer" usf := unexportedStringerOtherFields{ s: "foo", S: "bar", } - fmt.Printf("%s", usf) // ERROR "Printf format %s has arg usf of wrong type testdata.unexportedStringerOtherFields" - fmt.Printf("%s", &usf) // ERROR "Printf format %s has arg &usf of wrong type [*]testdata.unexportedStringerOtherFields" + fmt.Printf("%s", usf) // want "Printf format %s has arg usf of wrong type a.unexportedStringerOtherFields" + fmt.Printf("%s", &usf) // want "Printf format %s has arg &usf of wrong type [*]a.unexportedStringerOtherFields" ue := unexportedError{ e: &errorer{}, } - fmt.Printf("%s", ue) // ERROR "Printf format %s has arg ue of wrong type testdata.unexportedError" - fmt.Printf("%s", &ue) // ERROR "Printf format %s has arg &ue of wrong type [*]testdata.unexportedError" + fmt.Printf("%s", ue) // want "Printf format %s has arg ue of wrong type a.unexportedError" + fmt.Printf("%s", &ue) // want "Printf format %s has arg &ue of wrong type [*]a.unexportedError" uef := unexportedErrorOtherFields{ s: "foo", e: &errorer{}, S: "bar", } - fmt.Printf("%s", uef) // ERROR "Printf format %s has arg uef of wrong type testdata.unexportedErrorOtherFields" - fmt.Printf("%s", &uef) // ERROR "Printf format %s has arg &uef of wrong type [*]testdata.unexportedErrorOtherFields" + fmt.Printf("%s", uef) // want "Printf format %s has arg uef of wrong type a.unexportedErrorOtherFields" + fmt.Printf("%s", &uef) // want "Printf format %s has arg &uef of wrong type [*]a.unexportedErrorOtherFields" uce := unexportedCustomError{ e: errorer{}, } - fmt.Printf("%s", uce) // ERROR "Printf format %s has arg uce of wrong type testdata.unexportedCustomError" + fmt.Printf("%s", uce) // want "Printf format %s has arg uce of wrong type a.unexportedCustomError" uei := unexportedErrorInterface{} - fmt.Printf("%s", uei) // ERROR "Printf format %s has arg uei of wrong type testdata.unexportedErrorInterface" + fmt.Printf("%s", uei) // want "Printf format %s has arg uei of wrong type a.unexportedErrorInterface" fmt.Println("foo\n", "bar") // not an error - fmt.Println("foo\n") // ERROR "Println arg list ends with redundant newline" + fmt.Println("foo\n") // want "Println arg list ends with redundant newline" fmt.Println("foo\\n") // not an error fmt.Println(`foo\n`) // not an error intSlice := []int{3, 4} - fmt.Printf("%s", intSlice) // ERROR "Printf format %s has arg intSlice of wrong type \[\]int" + fmt.Printf("%s", intSlice) // want `Printf format %s has arg intSlice of wrong type \[\]int` nonStringerArray := [1]unexportedStringer{{}} - fmt.Printf("%s", nonStringerArray) // ERROR "Printf format %s has arg nonStringerArray of wrong type \[1\]testdata.unexportedStringer" + fmt.Printf("%s", nonStringerArray) // want `Printf format %s has arg nonStringerArray of wrong type \[1\]a.unexportedStringer` fmt.Printf("%s", []stringer{3, 4}) // not an error fmt.Printf("%s", [2]stringer{3, 4}) // not an error } @@ -645,3 +653,11 @@ func dbg(format string, args ...interface{}) { } fmt.Printf(format, args...) } + +// Printf wrappers from external package +func externalPackage() { + b.Wrapf("%s", 1) // want "Wrapf format %s has arg 1 of wrong type int" + b.Wrap("%s", 1) // want "Wrap call has possible formatting directive %s" + b.NoWrap("%s", 1) + b.Wrapf2("%s", 1) // want "Wrapf2 format %s has arg 1 of wrong type int" +} diff --git a/go/analysis/passes/printf/testdata/src/b/b.go b/go/analysis/passes/printf/testdata/src/b/b.go new file mode 100644 index 00000000..cf5f0822 --- /dev/null +++ b/go/analysis/passes/printf/testdata/src/b/b.go @@ -0,0 +1,33 @@ +package b + +import "fmt" + +// Wrapf is a printf wrapper. +func Wrapf(format string, args ...interface{}) { // want Wrapf:"printfWrapper" + fmt.Sprintf(format, args...) +} + +// Wrap is a print wrapper. +func Wrap(args ...interface{}) { // want Wrap:"printWrapper" + fmt.Sprint(args...) +} + +// NoWrap is not a wrapper. +func NoWrap(format string, args ...interface{}) { +} + +// Wrapf2 is another printf wrapper. +func Wrapf2(format string, args ...interface{}) string { // want Wrapf2:"printfWrapper" + + // This statement serves as an assertion that this function is a + // printf wrapper and that calls to it should be checked + // accordingly, even though the delegation below is obscured by + // the "("+format+")" operations. + if false { + fmt.Sprintf(format, args...) + } + + // Effectively a printf delegation, + // but the printf checker can't see it. + return fmt.Sprintf("("+format+")", args...) +} diff --git a/go/analysis/passes/printf/types.go b/go/analysis/passes/printf/types.go index 0b02250f..701d08be 100644 --- a/go/analysis/passes/printf/types.go +++ b/go/analysis/passes/printf/types.go @@ -1,137 +1,15 @@ -// +build ignore - -// Copyright 2010 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// This file contains the pieces of the tool that use typechecking from the go/types package. - -package main +package printf import ( "go/ast" "go/build" - "go/importer" - "go/token" "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/internal/analysisutil" ) -// stdImporter is the importer we use to import packages. -// It is shared so that all packages are imported by the same importer. -var stdImporter types.Importer - -var ( - errorType *types.Interface - stringerType *types.Interface // possibly nil - formatterType *types.Interface // possibly nil -) - -func inittypes() { - errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface) - - if typ := importType("fmt", "Stringer"); typ != nil { - stringerType = typ.Underlying().(*types.Interface) - } - if typ := importType("fmt", "Formatter"); typ != nil { - formatterType = typ.Underlying().(*types.Interface) - } -} - -// isNamedType reports whether t is the named type path.name. -func isNamedType(t types.Type, path, name string) bool { - n, ok := t.(*types.Named) - if !ok { - return false - } - obj := n.Obj() - return obj.Name() == name && obj.Pkg() != nil && obj.Pkg().Path() == path -} - -// importType returns the type denoted by the qualified identifier -// path.name, and adds the respective package to the imports map -// as a side effect. In case of an error, importType returns nil. -func importType(path, name string) types.Type { - pkg, err := stdImporter.Import(path) - if err != nil { - // This can happen if the package at path hasn't been compiled yet. - warnf("import failed: %v", err) - return nil - } - if obj, ok := pkg.Scope().Lookup(name).(*types.TypeName); ok { - return obj.Type() - } - warnf("invalid type name %q", name) - return nil -} - -func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) []error { - if stdImporter == nil { - if *source { - stdImporter = importer.For("source", nil) - } else { - stdImporter = importer.Default() - } - inittypes() - } - pkg.defs = make(map[*ast.Ident]types.Object) - pkg.uses = make(map[*ast.Ident]types.Object) - pkg.implicits = make(map[ast.Node]types.Object) - pkg.selectors = make(map[*ast.SelectorExpr]*types.Selection) - pkg.spans = make(map[types.Object]Span) - pkg.types = make(map[ast.Expr]types.TypeAndValue) - - var allErrors []error - config := types.Config{ - // We use the same importer for all imports to ensure that - // everybody sees identical packages for the given paths. - Importer: stdImporter, - // By providing a Config with our own error function, it will continue - // past the first error. We collect them all for printing later. - Error: func(e error) { - allErrors = append(allErrors, e) - }, - - Sizes: archSizes, - } - info := &types.Info{ - Selections: pkg.selectors, - Types: pkg.types, - Defs: pkg.defs, - Uses: pkg.uses, - Implicits: pkg.implicits, - } - typesPkg, err := config.Check(pkg.path, fs, astFiles, info) - if len(allErrors) == 0 && err != nil { - allErrors = append(allErrors, err) - } - pkg.typesPkg = typesPkg - // update spans - for id, obj := range pkg.defs { - // Ignore identifiers that don't denote objects - // (package names, symbolic variables such as t - // in t := x.(type) of type switch headers). - if obj != nil { - pkg.growSpan(obj, id.Pos(), id.End()) - } - } - for id, obj := range pkg.uses { - pkg.growSpan(obj, id.Pos(), id.End()) - } - for node, obj := range pkg.implicits { - // A type switch with a short variable declaration - // such as t := x.(type) doesn't declare the symbolic - // variable (t in the example) at the switch header; - // instead a new variable t (with specific type) is - // declared implicitly for each case. Such variables - // are found in the types.Info.Implicits (not Defs) - // map. Add them here, assuming they are declared at - // the type cases' colon ":". - if cc, ok := node.(*ast.CaseClause); ok { - pkg.growSpan(obj, cc.Colon, cc.Colon) - } - } - return allErrors -} +var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface) // matchArgType reports an error if printf verb t is not appropriate // for operand arg. @@ -141,31 +19,31 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) []error { // (Recursion arises from the compound types {map,chan,slice} which // may be printed with %d etc. if that is appropriate for their element // types.) -func (f *File) matchArgType(t printfArgType, typ types.Type, arg ast.Expr) bool { - return f.matchArgTypeInternal(t, typ, arg, make(map[types.Type]bool)) +func matchArgType(pass *analysis.Pass, t printfArgType, typ types.Type, arg ast.Expr) bool { + return matchArgTypeInternal(pass, t, typ, arg, make(map[types.Type]bool)) } // matchArgTypeInternal is the internal version of matchArgType. It carries a map // remembering what types are in progress so we don't recur when faced with recursive // types or mutually recursive types. -func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Expr, inProgress map[types.Type]bool) bool { +func matchArgTypeInternal(pass *analysis.Pass, t printfArgType, typ types.Type, arg ast.Expr, inProgress map[types.Type]bool) bool { // %v, %T accept any argument type. if t == anyType { return true } if typ == nil { // external call - typ = f.pkg.types[arg].Type + typ = pass.TypesInfo.Types[arg].Type if typ == nil { return true // probably a type check problem } } // If the type implements fmt.Formatter, we have nothing to check. - if f.isFormatter(typ) { + if isFormatter(pass, typ) { return true } // If we can use a string, might arg (dynamically) implement the Stringer or Error interface? - if t&argString != 0 && isConvertibleToString(typ) { + if t&argString != 0 && isConvertibleToString(pass, typ) { return true } @@ -183,7 +61,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp case *types.Map: // Recur: map[int]int matches %d. return t&argPointer != 0 || - (f.matchArgTypeInternal(t, typ.Key(), arg, inProgress) && f.matchArgTypeInternal(t, typ.Elem(), arg, inProgress)) + (matchArgTypeInternal(pass, t, typ.Key(), arg, inProgress) && matchArgTypeInternal(pass, t, typ.Elem(), arg, inProgress)) case *types.Chan: return t&argPointer != 0 @@ -194,7 +72,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp return true // %s matches []byte } // Recur: []int matches %d. - return t&argPointer != 0 || f.matchArgTypeInternal(t, typ.Elem(), arg, inProgress) + return t&argPointer != 0 || matchArgTypeInternal(pass, t, typ.Elem(), arg, inProgress) case *types.Slice: // Same as array. @@ -204,14 +82,14 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp // Recur: []int matches %d. But watch out for // type T []T // If the element is a pointer type (type T[]*T), it's handled fine by the Pointer case below. - return t&argPointer != 0 || f.matchArgTypeInternal(t, typ.Elem(), arg, inProgress) + return t&argPointer != 0 || matchArgTypeInternal(pass, t, typ.Elem(), arg, inProgress) case *types.Pointer: // Ugly, but dealing with an edge case: a known pointer to an invalid type, // probably something from a failed import. if typ.Elem().String() == "invalid type" { - if *verbose { - f.Warnf(arg.Pos(), "printf argument %v is pointer to invalid or unknown type", f.gofmt(arg)) + if false { + pass.Reportf(arg.Pos(), "printf argument %v is pointer to invalid or unknown type", analysisutil.Format(pass.Fset, arg)) } return true // special case } @@ -221,13 +99,13 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp } // 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 f.matchStructArgType(t, str, arg, inProgress) + return matchStructArgType(pass, t, str, arg, inProgress) } // Check whether the rest can print pointers. return t&argPointer != 0 case *types.Struct: - return f.matchStructArgType(t, typ, arg, inProgress) + return matchStructArgType(pass, t, typ, arg, inProgress) case *types.Interface: // There's little we can do. @@ -279,8 +157,8 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp return false case types.Invalid: - if *verbose { - f.Warnf(arg.Pos(), "printf argument %v has invalid or unknown type", f.gofmt(arg)) + if false { + pass.Reportf(arg.Pos(), "printf argument %v has invalid or unknown type", analysisutil.Format(pass.Fset, arg)) } return true // Probably a type check problem. } @@ -290,7 +168,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp return false } -func isConvertibleToString(typ types.Type) bool { +func isConvertibleToString(pass *analysis.Pass, typ types.Type) bool { if bt, ok := typ.(*types.Basic); ok && bt.Kind() == types.UntypedNil { // We explicitly don't want untyped nil, which is // convertible to both of the interfaces below, as it @@ -300,15 +178,25 @@ func isConvertibleToString(typ types.Type) bool { if types.ConvertibleTo(typ, errorType) { return true // via .Error() } - if stringerType != nil && types.ConvertibleTo(typ, stringerType) { - return true // via .String() + + // Does it implement fmt.Stringer? + if obj, _, _ := types.LookupFieldOrMethod(typ, false, nil, "String"); obj != nil { + if fn, ok := obj.(*types.Func); ok { + sig := fn.Type().(*types.Signature) + if sig.Params().Len() == 0 && + sig.Results().Len() == 1 && + sig.Results().At(0).Type() == types.Typ[types.String] { + return true + } + } } + return false } // hasBasicType reports whether x's type is a types.Basic with the given kind. -func (f *File) hasBasicType(x ast.Expr, kind types.BasicKind) bool { - t := f.pkg.types[x].Type +func hasBasicType(pass *analysis.Pass, x ast.Expr, kind types.BasicKind) bool { + t := pass.TypesInfo.Types[x].Type if t != nil { t = t.Underlying() } @@ -318,13 +206,13 @@ func (f *File) hasBasicType(x ast.Expr, kind types.BasicKind) bool { // matchStructArgType reports whether all the elements of the struct match the expected // type. For instance, with "%d" all the elements must be printable with the "%d" format. -func (f *File) matchStructArgType(t printfArgType, typ *types.Struct, arg ast.Expr, inProgress map[types.Type]bool) bool { +func matchStructArgType(pass *analysis.Pass, t printfArgType, typ *types.Struct, arg ast.Expr, inProgress map[types.Type]bool) bool { for i := 0; i < typ.NumFields(); i++ { typf := typ.Field(i) - if !f.matchArgTypeInternal(t, typf.Type(), arg, inProgress) { + if !matchArgTypeInternal(pass, t, typf.Type(), arg, inProgress) { return false } - if t&argString != 0 && !typf.Exported() && isConvertibleToString(typf.Type()) { + if t&argString != 0 && !typf.Exported() && isConvertibleToString(pass, typf.Type()) { // Issue #17798: unexported Stringer or error cannot be properly fomatted. return false } diff --git a/go/types/typeutil/callee.go b/go/types/typeutil/callee.go index c079a499..38f596da 100644 --- a/go/types/typeutil/callee.go +++ b/go/types/typeutil/callee.go @@ -7,13 +7,15 @@ package typeutil import ( "go/ast" "go/types" + + "golang.org/x/tools/go/ast/astutil" ) -// StaticCallee returns the target (function or method) of a static -// function call, if any. It returns nil for calls to builtin. -func StaticCallee(info *types.Info, call *ast.CallExpr) *types.Func { +// Callee returns the named target of a function call, if any: +// a function, method, builtin, or variable. +func Callee(info *types.Info, call *ast.CallExpr) types.Object { var obj types.Object - switch fun := call.Fun.(type) { + switch fun := astutil.Unparen(call.Fun).(type) { case *ast.Ident: obj = info.Uses[fun] // type, var, builtin, or declared func case *ast.SelectorExpr: @@ -23,7 +25,16 @@ func StaticCallee(info *types.Info, call *ast.CallExpr) *types.Func { obj = info.Uses[fun.Sel] // qualified identifier? } } - if f, ok := obj.(*types.Func); ok && !interfaceMethod(f) { + if _, ok := obj.(*types.TypeName); ok { + return nil // T(x) is a conversion, not a call + } + return obj +} + +// StaticCallee returns the target (function or method) of a static +// function call, if any. It returns nil for calls to builtins. +func StaticCallee(info *types.Info, call *ast.CallExpr) *types.Func { + if f, ok := Callee(info, call).(*types.Func); ok && !interfaceMethod(f) { return f } return nil