From 8e609cddef80ee828f16497f84fdaf674207d3d8 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Thu, 9 Dec 2010 12:37:18 -0500 Subject: [PATCH] govet: a new static checker for Go programs. At the moment, and for the forseeable future, it only checks arguments to print calls. R=rsc, gri, niemeyer, iant2, rog, lstoakes, jacek.masiulaniec, cw CC=golang-dev https://golang.org/cl/3522041 --- src/cmd/govet/Makefile | 11 ++ src/cmd/govet/doc.go | 37 ++++++ src/cmd/govet/govet.go | 281 +++++++++++++++++++++++++++++++++++++++++ src/pkg/Makefile | 2 + 4 files changed, 331 insertions(+) create mode 100644 src/cmd/govet/Makefile create mode 100644 src/cmd/govet/doc.go create mode 100644 src/cmd/govet/govet.go diff --git a/src/cmd/govet/Makefile b/src/cmd/govet/Makefile new file mode 100644 index 00000000000..291b271976d --- /dev/null +++ b/src/cmd/govet/Makefile @@ -0,0 +1,11 @@ +# 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. + +include ../../Make.inc + +TARG=govet +GOFILES=\ + govet.go\ + +include ../../Make.cmd diff --git a/src/cmd/govet/doc.go b/src/cmd/govet/doc.go new file mode 100644 index 00000000000..fd66d3c0b01 --- /dev/null +++ b/src/cmd/govet/doc.go @@ -0,0 +1,37 @@ +// 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. + +/* + +Govet does simple checking of Go source code. + +It checks for simple errors in calls to functions named + Print Printf Println + Fprint Fprintf Fprintln + Sprint Sprintf Sprintln + Error Errorf + Fatal Fatalf +If the function name ends with an 'f', the function is assumed to take +a format descriptor string in the manner of fmt.Printf. If not, govet +complains about arguments that look like format descriptor strings. + +Usage: + + govet [flag] [file.go ...] + +The flags are: + -v + Verbose mode + -printfuncs + A comma-separated list of print-like functions to supplement + the standard list. Each entry is in the form Name:N where N + is the zero-based argument position of the first argument + involved in the print: either the format or the first print + argument for non-formatted prints. For example, + if you have Warn and Warnf functions that take an + io.Writer as their first argument, like Fprintf, + -printfuncs=Warn:1,Warnf:1 + +*/ +package documentation diff --git a/src/cmd/govet/govet.go b/src/cmd/govet/govet.go new file mode 100644 index 00000000000..c748c8018f1 --- /dev/null +++ b/src/cmd/govet/govet.go @@ -0,0 +1,281 @@ +// 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. + +// Govet is a simple checker for static errors in Go source code. +// See doc.go for more information. +package main + +import ( + "bytes" + "flag" + "fmt" + "io" + "go/ast" + "go/parser" + "go/token" + "os" + "strconv" + "strings" +) + +var verbose = flag.Bool("v", false, "verbose") +var printfuncs = flag.String("printfuncs", "", "comma-separated list of print function names to check") +var exitCode = 0 + +// Usage is a replacement usage function for the flags package. +func Usage() { + fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) + flag.PrintDefaults() + os.Exit(2) +} + +// File is a wrapper for the state of a file used in the parser. +// The parse tree walkers are all methods of this type. +type File struct { + file *token.File +} + +func main() { + flag.Usage = Usage + flag.Parse() + + if *printfuncs != "" { + for _, name := range strings.Split(*printfuncs, ",", -1) { + if len(name) == 0 { + flag.Usage() + } + skip := 0 + if colon := strings.LastIndex(name, ":"); colon > 0 { + var err os.Error + skip, err = strconv.Atoi(name[colon+1:]) + if err != nil { + die(`illegal format for "Func:N" argument %q; %s`, name, err) + } + name = name[:colon] + } + if name[len(name)-1] == 'f' { + printfList[name] = skip + } else { + printList[name] = skip + } + } + } + + if flag.NArg() == 0 { + doFile("stdin", os.Stdin) + } else { + for _, arg := range flag.Args() { + doFile(arg, nil) + } + } + os.Exit(exitCode) +} + +// doFile analyzes one file. If the reader is nil, the source code is read from the +// named file. +func doFile(name string, reader io.Reader) { + // TODO: process directories? + fs := token.NewFileSet() + parsedFile, err := parser.ParseFile(fs, name, reader, 0) + if err != nil { + die("%s: %s", name, err) + } + file := &File{fs.File(parsedFile.Pos())} + file.checkFile(name, parsedFile) +} + +// die formats the error to standard error, adding program identification +// and a newline, and exits the program. +func die(format string, args ...interface{}) { + fmt.Fprintf(os.Stderr, "govet: "+format+"\n", args...) + os.Exit(2) +} + +// Println is fmt.Println guarded by -v. +func Println(args ...interface{}) { + if !*verbose { + return + } + fmt.Println(args...) +} + +// Printf is fmt.Printf guarded by -v. +func Printf(format string, args ...interface{}) { + if !*verbose { + return + } + fmt.Printf(format+"\n", args...) +} + +// Bad reports an error and sets the exit code.. +func (f *File) Bad(pos token.Pos, args ...interface{}) { + f.Warn(pos, args...) + exitCode = 1 +} + +// Badf reports a formatted error and sets the exit code. +func (f *File) Badf(pos token.Pos, format string, args ...interface{}) { + f.Warnf(pos, format, args...) + exitCode = 1 +} + +// Warn reports an error but does not set the exit code. +func (f *File) Warn(pos token.Pos, args ...interface{}) { + loc := f.file.Position(pos).String() + ": " + fmt.Fprint(os.Stderr, loc+fmt.Sprintln(args...)) +} + +// Warnf reports a formatted error but does not set the exit code. +func (f *File) Warnf(pos token.Pos, format string, args ...interface{}) { + loc := f.file.Position(pos).String() + ": " + fmt.Fprintf(os.Stderr, loc+format+"\n", args...) +} + +// checkFile checks all the top-level declarations in a file. +func (f *File) checkFile(name string, file *ast.File) { + Println("Checking", name) + ast.Walk(f, file) +} + +// Visit implements the ast.Visitor interface. +func (f *File) Visit(node interface{}) ast.Visitor { + // TODO: could return nil for nodes that cannot contain a CallExpr - + // will shortcut traversal. Worthwhile? + switch n := node.(type) { + case *ast.CallExpr: + f.checkCallExpr(n) + } + return f +} + + +// checkCallExpr checks a call expression. +func (f *File) checkCallExpr(call *ast.CallExpr) { + switch x := call.Fun.(type) { + case *ast.Ident: + f.checkCall(call, x.Name) + case *ast.SelectorExpr: + f.checkCall(call, x.Sel.Name) + } +} + +// printfList records the formatted-print functions. The value is the location +// of the format parameter. +var printfList = map[string]int{ + "Errorf": 0, + "Fatalf": 0, + "Fprintf": 1, + "Printf": 0, + "Sprintf": 0, +} + +// printList records the unformatted-print functions. The value is the location +// of the first parameter to be printed. +var printList = map[string]int{ + "Error": 0, + "Fatal": 0, + "Fprint": 1, "Fprintln": 1, + "Print": 0, "Println": 0, + "Sprint": 0, "Sprintln": 0, +} + +// checkCall triggers the print-specific checks if the call invokes a print function. +func (f *File) checkCall(call *ast.CallExpr, name string) { + if skip, ok := printfList[name]; ok { + f.checkPrintf(call, name, skip) + return + } + if skip, ok := printList[name]; ok { + f.checkPrint(call, name, skip) + return + } +} + +// checkPrintf checks a call to a formatted print routine such as Printf. +// The skip argument records how many arguments to ignore; that is, +// call.Args[skip] is (well, should be) the format argument. +func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) { + if len(call.Args) <= skip { + return + } + // Common case: literal is first argument. + arg := call.Args[skip] + lit, ok := arg.(*ast.BasicLit) + if !ok { + // Too hard to check. + if *verbose { + f.Warn(call.Pos(), "can't check args for call to", name) + } + return + } + if lit.Kind == token.STRING { + if bytes.IndexByte(lit.Value, '%') < 0 { + if len(call.Args) > skip+1 { + f.Badf(call.Pos(), "no formatting directive in %s call", name) + } + return + } + } + // Hard part: check formats against args. + // Trivial but useful test: count. + numPercent := 0 + for i := 0; i < len(lit.Value); i++ { + if lit.Value[i] == '%' { + if i+1 < len(lit.Value) && lit.Value[i+1] == '%' { + // %% doesn't count. + i++ + } else { + numPercent++ + } + } + } + expect := len(call.Args) - (skip + 1) + if numPercent != expect { + f.Badf(call.Pos(), "wrong number of formatting directives in %s call: %d percent(s) for %d args", name, numPercent, expect) + } +} + +var terminalNewline = []byte(`\n"`) // \n at end of interpreted string + +// checkPrint checks a call to an unformatted print routine such as Println. +// The skip argument records how many arguments to ignore; that is, +// call.Args[skip] is the first argument to be printed. +func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) { + isLn := strings.HasSuffix(name, "ln") + args := call.Args + if len(args) <= skip { + if *verbose && !isLn { + f.Badf(call.Pos(), "no args in %s call", name) + } + return + } + arg := args[skip] + if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRING { + if bytes.IndexByte(lit.Value, '%') >= 0 { + f.Badf(call.Pos(), "possible formatting directive in %s call", name) + } + } + if isLn { + // The last item, if a string, should not have a newline. + arg = args[len(call.Args)-1] + if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRING { + if bytes.HasSuffix(lit.Value, terminalNewline) { + f.Badf(call.Pos(), "%s call ends with newline", name) + } + } + } +} + +// This function never executes, but it serves as a simple test for the program. +// Test with govet --funcs="Bad:1,Badf:1,Warn:1,Warnf:1" govet.go +func BadFunctionUsedInTests() { + fmt.Println() // niladic call + fmt.Println("%s", "hi") // % in call to Println + fmt.Printf("%s", "hi", 3) // wrong # percents + fmt.Printf("%s%%%d", "hi", 3) // right # percents + Printf("now is the time", "buddy") // no %s + f := new(File) + f.Warn(0, "%s", "hello", 3) // % in call to added function + f.Warnf(0, "%s", "hello", 3) // wrong # %s in call to added function +} diff --git a/src/pkg/Makefile b/src/pkg/Makefile index d2a8789c5fb..b46fa46e37f 100644 --- a/src/pkg/Makefile +++ b/src/pkg/Makefile @@ -137,6 +137,7 @@ DIRS=\ ../cmd/godoc\ ../cmd/gofmt\ ../cmd/goinstall\ + ../cmd/govet\ ../cmd/goyacc\ ../cmd/hgpatch\ @@ -163,6 +164,7 @@ NOTEST=\ ../cmd/godoc\ ../cmd/gofmt\ ../cmd/goinstall\ + ../cmd/govet\ ../cmd/goyacc\ ../cmd/hgpatch\