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

+# 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

src/cmd/govet/doc.go

+// 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

src/cmd/govet/govet.go

+// 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
+}

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\