diff --git a/cmd/vet/doc.go b/cmd/vet/doc.go index e15a9758..a19de3fa 100644 --- a/cmd/vet/doc.go +++ b/cmd/vet/doc.go @@ -150,6 +150,15 @@ there is a uintptr-typed word in memory that holds a pointer value, because that word will be invisible to stack copying and to the garbage collector. +Unused result of certain function calls + +Flag: -unusedresult + +Calls to well-known functions and methods that return a value that is +discarded. By default, this includes functions like fmt.Errorf and +fmt.Sprintf and methods like String and Error. The flags -unusedfuncs +and -unusedstringmethods control the set. + Shifts Flag: -shift diff --git a/cmd/vet/main.go b/cmd/vet/main.go index a58e18ac..16b8842c 100644 --- a/cmd/vet/main.go +++ b/cmd/vet/main.go @@ -128,6 +128,7 @@ var ( binaryExpr *ast.BinaryExpr callExpr *ast.CallExpr compositeLit *ast.CompositeLit + exprStmt *ast.ExprStmt field *ast.Field funcDecl *ast.FuncDecl funcLit *ast.FuncLit @@ -197,28 +198,8 @@ func main() { } } - if *printfuncs != "" { - for _, name := range strings.Split(*printfuncs, ",") { - if len(name) == 0 { - flag.Usage() - } - skip := 0 - if colon := strings.LastIndex(name, ":"); colon > 0 { - var err error - skip, err = strconv.Atoi(name[colon+1:]) - if err != nil { - errorf(`illegal format for "Func:N" argument %q; %s`, name, err) - } - name = name[:colon] - } - name = strings.ToLower(name) - if name[len(name)-1] == 'f' { - printfList[name] = skip - } else { - printList[name] = skip - } - } - } + initPrintFlags() + initUnusedFlags() if flag.NArg() == 0 { Usage() @@ -291,13 +272,14 @@ func doPackageDir(directory string) { } type Package struct { - path string - defs map[*ast.Ident]types.Object - uses map[*ast.Ident]types.Object - types map[ast.Expr]types.TypeAndValue - spans map[types.Object]Span - files []*File - typesPkg *types.Package + path string + defs map[*ast.Ident]types.Object + uses map[*ast.Ident]types.Object + selectors map[*ast.SelectorExpr]*types.Selection + types map[ast.Expr]types.TypeAndValue + spans map[types.Object]Span + files []*File + typesPkg *types.Package } // doPackage analyzes the single package constructed from the named files. @@ -466,6 +448,8 @@ func (f *File) Visit(node ast.Node) ast.Visitor { key = callExpr case *ast.CompositeLit: key = compositeLit + case *ast.ExprStmt: + key = exprStmt case *ast.Field: key = field case *ast.FuncDecl: diff --git a/cmd/vet/print.go b/cmd/vet/print.go index 82edd81e..b20d935e 100644 --- a/cmd/vet/print.go +++ b/cmd/vet/print.go @@ -28,6 +28,32 @@ func init() { funcDecl, callExpr) } +func initPrintFlags() { + if *printfuncs == "" { + return + } + for _, name := range strings.Split(*printfuncs, ",") { + if len(name) == 0 { + flag.Usage() + } + skip := 0 + if colon := strings.LastIndex(name, ":"); colon > 0 { + var err error + skip, err = strconv.Atoi(name[colon+1:]) + if err != nil { + errorf(`illegal format for "Func:N" argument %q; %s`, name, err) + } + name = name[:colon] + } + name = strings.ToLower(name) + if name[len(name)-1] == 'f' { + printfList[name] = skip + } else { + printList[name] = skip + } + } +} + // printfList records the formatted-print functions. The value is the location // of the format parameter. Names are lower-cased so the lookup is // case insensitive. diff --git a/cmd/vet/testdata/print.go b/cmd/vet/testdata/print.go index 22c6e0a9..3390a31f 100644 --- a/cmd/vet/testdata/print.go +++ b/cmd/vet/testdata/print.go @@ -130,7 +130,7 @@ func PrintfTests() { 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.Sprintf("%"+("s"), "hi", 3) // ERROR "wrong number of args for format in Sprintf call" + _ = fmt.Sprintf("%"+("s"), "hi", 3) // ERROR "wrong number of args for format in Sprintf call" fmt.Printf("%s%%%d", "hi", 3) // correct fmt.Printf("%08s", "woo") // correct fmt.Printf("% 8s", "woo") // correct @@ -172,7 +172,7 @@ func PrintfTests() { Printf("%[xd", 3) // ERROR "illegal syntax for printf argument index" Printf("%[x]d", 3) // ERROR "illegal syntax for printf argument index" Printf("%[3]*s", "hi", 2) // ERROR "missing argument for Printf.* reads arg 3, have only 2" - fmt.Sprintf("%[3]d", 2) // ERROR "missing argument for Sprintf.* reads arg 3, have only 1" + _ = fmt.Sprintf("%[3]d", 2) // ERROR "missing argument for Sprintf.* reads arg 3, have only 1" Printf("%[2]*.[1]*[3]d", 2, "hi", 4) // ERROR "arg .hi. for \* in printf format not of type int" Printf("%[0]s", "arg1") // ERROR "index value \[0\] for Printf.*; indexes start at 1" Printf("%[0]d", 1) // ERROR "index value \[0\] for Printf.*; indexes start at 1" @@ -292,18 +292,18 @@ var percentSV percentSStruct type recursiveStringer int func (s recursiveStringer) String() string { - fmt.Sprintf("%d", s) - fmt.Sprintf("%#v", s) - fmt.Sprintf("%v", s) // ERROR "arg s for printf causes recursive call to String method" - fmt.Sprintf("%v", &s) // ERROR "arg &s for printf causes recursive call to String method" - fmt.Sprintf("%T", s) // ok; does not recursively call String - return fmt.Sprintln(s) // ERROR "arg s for print causes recursive call to String method" + _ = fmt.Sprintf("%d", s) + _ = fmt.Sprintf("%#v", s) + _ = fmt.Sprintf("%v", s) // ERROR "arg s for printf causes recursive call to String method" + _ = fmt.Sprintf("%v", &s) // ERROR "arg &s for printf causes recursive call to String method" + _ = fmt.Sprintf("%T", s) // ok; does not recursively call String + return fmt.Sprintln(s) // ERROR "arg s for print causes recursive call to String method" } type recursivePtrStringer int func (p *recursivePtrStringer) String() string { - fmt.Sprintf("%v", *p) + _ = fmt.Sprintf("%v", *p) return fmt.Sprintln(p) // ERROR "arg p for print causes recursive call to String method" } diff --git a/cmd/vet/testdata/unused.go b/cmd/vet/testdata/unused.go new file mode 100644 index 00000000..d50f6594 --- /dev/null +++ b/cmd/vet/testdata/unused.go @@ -0,0 +1,29 @@ +// Copyright 2015 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 tests for the unusedresult checker. + +package testdata + +import ( + "bytes" + "errors" + "fmt" +) + +func _() { + fmt.Errorf("") // ERROR "result of fmt.Errorf call not used" + _ = fmt.Errorf("") + + errors.New("") // ERROR "result of errors.New call not used" + + err := errors.New("") + err.Error() // ERROR "result of \(error\).Error call not used" + + var buf bytes.Buffer + buf.String() // ERROR "result of \(bytes.Buffer\).String call not used" + + fmt.Sprint("") // ERROR "result of fmt.Sprint call not used" + fmt.Sprintf("") // ERROR "result of fmt.Sprintf call not used" +} diff --git a/cmd/vet/types.go b/cmd/vet/types.go index 8a0182b0..89e9989d 100644 --- a/cmd/vet/types.go +++ b/cmd/vet/types.go @@ -52,6 +52,7 @@ func importType(path, name string) types.Type { func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error { pkg.defs = make(map[*ast.Ident]types.Object) pkg.uses = make(map[*ast.Ident]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) config := types.Config{ @@ -63,9 +64,10 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error { Error: func(error) {}, } info := &types.Info{ - Types: pkg.types, - Defs: pkg.defs, - Uses: pkg.uses, + Selections: pkg.selectors, + Types: pkg.types, + Defs: pkg.defs, + Uses: pkg.uses, } typesPkg, err := config.Check(pkg.path, fs, astFiles, info) pkg.typesPkg = typesPkg diff --git a/cmd/vet/unused.go b/cmd/vet/unused.go new file mode 100644 index 00000000..cbec9bb8 --- /dev/null +++ b/cmd/vet/unused.go @@ -0,0 +1,95 @@ +// Copyright 2015 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 defines the check for unused results of calls to certain +// pure functions. + +package main + +import ( + "flag" + "go/ast" + "go/token" + "strings" + + "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/go/types" +) + +var unusedFuncsFlag = flag.String("unusedfuncs", + "errors.New,fmt.Errorf,fmt.Sprintf,fmt.Sprint,sort.Reverse", + "comma-separated list of functions whose results must be used") + +var unusedStringMethodsFlag = flag.String("unusedstringmethods", + "Error,String", + "comma-separated list of names of methods of type func() string whose results must be used") + +func init() { + register("unusedresult", + "check for unused result of calls to functions in -unusedfuncs list and methods in -unusedstringmethods list", + checkUnusedResult, + exprStmt) +} + +// func() string +var sigNoArgsStringResult = types.NewSignature(nil, nil, nil, + types.NewTuple(types.NewVar(token.NoPos, nil, "", types.Typ[types.String])), + false) + +var unusedFuncs = make(map[string]bool) +var unusedStringMethods = make(map[string]bool) + +func initUnusedFlags() { + commaSplit := func(s string, m map[string]bool) { + if s != "" { + for _, name := range strings.Split(s, ",") { + if len(name) == 0 { + flag.Usage() + } + m[name] = true + } + } + } + commaSplit(*unusedFuncsFlag, unusedFuncs) + commaSplit(*unusedStringMethodsFlag, unusedStringMethods) +} + +func checkUnusedResult(f *File, n ast.Node) { + call, ok := astutil.Unparen(n.(*ast.ExprStmt).X).(*ast.CallExpr) + if !ok { + return // not a call statement + } + fun := astutil.Unparen(call.Fun) + + if f.pkg.types[fun].IsType() { + return // a conversion, not a call + } + + selector, ok := fun.(*ast.SelectorExpr) + if !ok { + return // neither a method call nor a qualified ident + } + + sel, ok := f.pkg.selectors[selector] + if ok && sel.Kind() == types.MethodVal { + // method (e.g. foo.String()) + obj := sel.Obj().(*types.Func) + sig := sel.Type().(*types.Signature) + if types.Identical(sig, sigNoArgsStringResult) { + if unusedStringMethods[obj.Name()] { + f.Badf(call.Lparen, "result of (%s).%s call not used", + sig.Recv().Type(), obj.Name()) + } + } + } else if !ok { + // package-qualified function (e.g. fmt.Errorf) + obj, _ := f.pkg.uses[selector.Sel] + if obj, ok := obj.(*types.Func); ok { + qname := obj.Pkg().Path() + "." + obj.Name() + if unusedFuncs[qname] { + f.Badf(call.Lparen, "result of %v call not used", qname) + } + } + } +}