diff options
Diffstat (limited to 'src/cmd/vet')
-rw-r--r-- | src/cmd/vet/Makefile | 5 | ||||
-rw-r--r-- | src/cmd/vet/atomic.go | 59 | ||||
-rw-r--r-- | src/cmd/vet/buildtag.go | 91 | ||||
-rw-r--r-- | src/cmd/vet/buildtag_bad.go | 11 | ||||
-rw-r--r-- | src/cmd/vet/doc.go | 20 | ||||
-rw-r--r-- | src/cmd/vet/main.go | 231 | ||||
-rw-r--r-- | src/cmd/vet/method.go | 7 | ||||
-rw-r--r-- | src/cmd/vet/print.go | 382 | ||||
-rw-r--r-- | src/cmd/vet/rangeloop.go | 65 | ||||
-rw-r--r-- | src/cmd/vet/structtag.go | 3 | ||||
-rw-r--r-- | src/cmd/vet/taglit.go | 31 | ||||
-rw-r--r-- | src/cmd/vet/test_atomic.go | 43 | ||||
-rw-r--r-- | src/cmd/vet/test_buildtag.go | 15 | ||||
-rw-r--r-- | src/cmd/vet/test_method.go | 24 | ||||
-rw-r--r-- | src/cmd/vet/test_print.go | 153 | ||||
-rw-r--r-- | src/cmd/vet/test_rangeloop.go | 61 | ||||
-rw-r--r-- | src/cmd/vet/test_structtag.go | 15 | ||||
-rw-r--r-- | src/cmd/vet/test_taglit.go | 31 |
18 files changed, 1101 insertions, 146 deletions
diff --git a/src/cmd/vet/Makefile b/src/cmd/vet/Makefile index 2a35d1ae3..307f4729c 100644 --- a/src/cmd/vet/Makefile +++ b/src/cmd/vet/Makefile @@ -3,5 +3,6 @@ # license that can be found in the LICENSE file. test testshort: - go build - ../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' print.go + go build -tags vet_test + ../../../test/errchk ./vet -compositewhitelist=false -printfuncs='Warn:1,Warnf:1' *.go + diff --git a/src/cmd/vet/atomic.go b/src/cmd/vet/atomic.go new file mode 100644 index 000000000..4ab256f64 --- /dev/null +++ b/src/cmd/vet/atomic.go @@ -0,0 +1,59 @@ +// Copyright 2013 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. + +package main + +import ( + "go/ast" + "go/token" +) + +// checkAtomicAssignment walks the assignment statement checking for common +// mistaken usage of atomic package, such as: x = atomic.AddUint64(&x, 1) +func (f *File) checkAtomicAssignment(n *ast.AssignStmt) { + if !vet("atomic") { + return + } + + if len(n.Lhs) != len(n.Rhs) { + return + } + + for i, right := range n.Rhs { + call, ok := right.(*ast.CallExpr) + if !ok { + continue + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + continue + } + pkg, ok := sel.X.(*ast.Ident) + if !ok || pkg.Name != "atomic" { + continue + } + + switch sel.Sel.Name { + case "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr": + f.checkAtomicAddAssignment(n.Lhs[i], call) + } + } +} + +// checkAtomicAddAssignment walks the atomic.Add* method calls checking for assigning the return value +// to the same variable being used in the operation +func (f *File) checkAtomicAddAssignment(left ast.Expr, call *ast.CallExpr) { + arg := call.Args[0] + broken := false + + if uarg, ok := arg.(*ast.UnaryExpr); ok && uarg.Op == token.AND { + broken = f.gofmt(left) == f.gofmt(uarg.X) + } else if star, ok := left.(*ast.StarExpr); ok { + broken = f.gofmt(star.X) == f.gofmt(arg) + } + + if broken { + f.Warn(left.Pos(), "direct assignment to atomic value") + } +} diff --git a/src/cmd/vet/buildtag.go b/src/cmd/vet/buildtag.go new file mode 100644 index 000000000..4b7580457 --- /dev/null +++ b/src/cmd/vet/buildtag.go @@ -0,0 +1,91 @@ +// Copyright 2013 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. + +package main + +import ( + "bytes" + "fmt" + "os" + "strings" + "unicode" +) + +var ( + nl = []byte("\n") + slashSlash = []byte("//") + plusBuild = []byte("+build") +) + +// checkBuildTag checks that build tags are in the correct location and well-formed. +func checkBuildTag(name string, data []byte) { + if !vet("buildtags") { + return + } + lines := bytes.SplitAfter(data, nl) + + // Determine cutpoint where +build comments are no longer valid. + // They are valid in leading // comments in the file followed by + // a blank line. + var cutoff int + for i, line := range lines { + line = bytes.TrimSpace(line) + if len(line) == 0 { + cutoff = i + continue + } + if bytes.HasPrefix(line, slashSlash) { + continue + } + break + } + + for i, line := range lines { + line = bytes.TrimSpace(line) + if !bytes.HasPrefix(line, slashSlash) { + continue + } + text := bytes.TrimSpace(line[2:]) + if bytes.HasPrefix(text, plusBuild) { + fields := bytes.Fields(text) + if !bytes.Equal(fields[0], plusBuild) { + // Comment is something like +buildasdf not +build. + fmt.Fprintf(os.Stderr, "%s:%d: possible malformed +build comment\n", name, i+1) + continue + } + if i >= cutoff { + fmt.Fprintf(os.Stderr, "%s:%d: +build comment appears too late in file\n", name, i+1) + setExit(1) + continue + } + // Check arguments. + Args: + for _, arg := range fields[1:] { + for _, elem := range strings.Split(string(arg), ",") { + if strings.HasPrefix(elem, "!!") { + fmt.Fprintf(os.Stderr, "%s:%d: invalid double negative in build constraint: %s\n", name, i+1, arg) + setExit(1) + break Args + } + if strings.HasPrefix(elem, "!") { + elem = elem[1:] + } + for _, c := range elem { + if !unicode.IsLetter(c) && !unicode.IsDigit(c) && c != '_' { + fmt.Fprintf(os.Stderr, "%s:%d: invalid non-alphanumeric build constraint: %s\n", name, i+1, arg) + setExit(1) + break Args + } + } + } + } + continue + } + // Comment with +build but not at beginning. + if bytes.Contains(line, plusBuild) && i < cutoff { + fmt.Fprintf(os.Stderr, "%s:%d: possible malformed +build comment\n", name, i+1) + continue + } + } +} diff --git a/src/cmd/vet/buildtag_bad.go b/src/cmd/vet/buildtag_bad.go new file mode 100644 index 000000000..4dca6a443 --- /dev/null +++ b/src/cmd/vet/buildtag_bad.go @@ -0,0 +1,11 @@ +// This file contains misplaced or malformed build constraints. +// The Go tool will skip it, because the constraints are invalid. +// It serves only to test the tag checker during make test. + +// Mention +build // ERROR "possible malformed \+build comment" + +// +build !!bang // ERROR "invalid double negative in build constraint" +// +build @#$ // ERROR "invalid non-alphanumeric build constraint" + +// +build toolate // ERROR "build comment appears too late in file" +package main diff --git a/src/cmd/vet/doc.go b/src/cmd/vet/doc.go index 620964aaf..f164eaca2 100644 --- a/src/cmd/vet/doc.go +++ b/src/cmd/vet/doc.go @@ -9,9 +9,12 @@ calls whose arguments do not align with the format string. Vet uses heuristics that do not guarantee all reports are genuine problems, but it can find errors not caught by the compilers. +By default all checks are performed, but if explicit flags are provided, only +those identified by the flags are performed. + Available checks: -1. Printf family +1. Printf family, flag -printf Suspicious calls to functions in the Printf family, including any functions with these names: @@ -28,24 +31,29 @@ complains about arguments that look like format descriptor strings. It also checks for errors such as using a Writer as the first argument of Printf. -2. Methods +2. Methods, flag -methods Non-standard signatures for methods with familiar names, including: Format GobEncode GobDecode MarshalJSON MarshalXML - Peek ReadByte ReadFrom ReadRune Scan Seek + Peek ReadByte ReadFrom ReadRune Scan Seek UnmarshalJSON UnreadByte UnreadRune WriteByte WriteTo -3. Struct tags +3. Struct tags, flag -structtags Struct tags that do not follow the format understood by reflect.StructTag.Get. +4. Untagged composite literals, flag -composites + +Composite struct literals that do not use the type-tagged syntax. + + Usage: go tool vet [flag] [file.go ...] go tool vet [flag] [directory ...] # Scan all .go files under directory, recursively -The flags are: +The other flags are: -v Verbose mode -printfuncs @@ -59,4 +67,4 @@ The flags are: -printfuncs=Warn:1,Warnf:1 */ -package documentation +package main diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go index 625133315..20f6cca1a 100644 --- a/src/cmd/vet/main.go +++ b/src/cmd/vet/main.go @@ -11,9 +11,12 @@ import ( "flag" "fmt" "go/ast" + "go/build" "go/parser" + "go/printer" "go/token" - "io" + "go/types" + "io/ioutil" "os" "path/filepath" "strconv" @@ -23,6 +26,24 @@ import ( var verbose = flag.Bool("v", false, "verbose") var exitCode = 0 +// Flags to control which checks to perform. "all" is set to true here, and disabled later if +// a flag is set explicitly. +var report = map[string]*bool{ + "all": flag.Bool("all", true, "check everything; disabled if any explicit check is requested"), + "atomic": flag.Bool("atomic", false, "check for common mistaken usages of the sync/atomic package"), + "buildtags": flag.Bool("buildtags", false, "check that +build tags are valid"), + "composites": flag.Bool("composites", false, "check that composite literals used type-tagged elements"), + "methods": flag.Bool("methods", false, "check that canonically named methods are canonically defined"), + "printf": flag.Bool("printf", false, "check printf-like invocations"), + "structtags": flag.Bool("structtags", false, "check that struct field tags have canonical format"), + "rangeloops": flag.Bool("rangeloops", false, "check that range loop variables are used correctly"), +} + +// vet tells whether to report errors for the named check, a flag name. +func vet(name string) bool { + return *report["all"] || *report[name] +} + // setExit sets the value for os.Exit when it is called, later. It // remembers the highest value. func setExit(err int) { @@ -34,6 +55,8 @@ func setExit(err int) { // Usage is a replacement usage function for the flags package. func Usage() { fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) + fmt.Fprintf(os.Stderr, "\tvet [flags] directory...\n") + fmt.Fprintf(os.Stderr, "\tvet [flags] files... # Must be a single package\n") flag.PrintDefaults() os.Exit(2) } @@ -41,7 +64,9 @@ func Usage() { // 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 { + pkg *Package fset *token.FileSet + name string file *ast.File b bytes.Buffer // for use by methods } @@ -50,6 +75,14 @@ func main() { flag.Usage = Usage flag.Parse() + // If a check is named explicitly, turn off the 'all' flag. + for name, ptr := range report { + if name != "all" && *ptr { + *report["all"] = false + break + } + } + if *printfuncs != "" { for _, name := range strings.Split(*printfuncs, ",") { if len(name) == 0 { @@ -74,41 +107,135 @@ func main() { } if flag.NArg() == 0 { - doFile("stdin", os.Stdin) - } else { + Usage() + } + dirs := false + files := false + for _, name := range flag.Args() { + // Is it a directory? + fi, err := os.Stat(name) + if err != nil { + warnf("error walking tree: %s", err) + continue + } + if fi.IsDir() { + dirs = true + } else { + files = true + } + } + if dirs && files { + Usage() + } + if dirs { for _, name := range flag.Args() { - // Is it a directory? - if fi, err := os.Stat(name); err == nil && fi.IsDir() { - walkDir(name) - } else { - doFile(name, nil) - } + walkDir(name) } + return } + doPackage(flag.Args()) 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) { - fs := token.NewFileSet() - parsedFile, err := parser.ParseFile(fs, name, reader, 0) +// prefixDirectory places the directory name on the beginning of each name in the list. +func prefixDirectory(directory string, names []string) { + if directory != "." { + for i, name := range names { + names[i] = filepath.Join(directory, name) + } + } +} + +// doPackageDir analyzes the single package found in the directory, if there is one, +// plus a test package, if there is one. +func doPackageDir(directory string) { + pkg, err := build.Default.ImportDir(directory, 0) if err != nil { - errorf("%s: %s", name, err) + // If it's just that there are no go source files, that's fine. + if _, nogo := err.(*build.NoGoError); nogo { + return + } + // Non-fatal: we are doing a recursive walk and there may be other directories. + warnf("cannot process directory %s: %s", directory, err) return } - file := &File{fset: fs, file: parsedFile} - file.walkFile(name, parsedFile) + var names []string + names = append(names, pkg.GoFiles...) + names = append(names, pkg.CgoFiles...) + names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package. + prefixDirectory(directory, names) + doPackage(names) + // Is there also a "foo_test" package? If so, do that one as well. + if len(pkg.XTestGoFiles) > 0 { + names = pkg.XTestGoFiles + prefixDirectory(directory, names) + doPackage(names) + } +} + +type Package struct { + types map[ast.Expr]types.Type + values map[ast.Expr]interface{} +} + +// doPackage analyzes the single package constructed from the named files. +func doPackage(names []string) { + var files []*File + var astFiles []*ast.File + fs := token.NewFileSet() + for _, name := range names { + f, err := os.Open(name) + if err != nil { + errorf("%s: %s", name, err) + } + defer f.Close() + data, err := ioutil.ReadAll(f) + if err != nil { + errorf("%s: %s", name, err) + } + checkBuildTag(name, data) + parsedFile, err := parser.ParseFile(fs, name, bytes.NewReader(data), 0) + if err != nil { + errorf("%s: %s", name, err) + } + files = append(files, &File{fset: fs, name: name, file: parsedFile}) + astFiles = append(astFiles, parsedFile) + } + pkg := new(Package) + pkg.types = make(map[ast.Expr]types.Type) + pkg.values = make(map[ast.Expr]interface{}) + exprFn := func(x ast.Expr, typ types.Type, val interface{}) { + pkg.types[x] = typ + if val != nil { + pkg.values[x] = val + } + } + // By providing the Context with our own error function, it will continue + // past the first error. There is no need for that function to do anything. + context := types.Context{ + Expr: exprFn, + Error: func(error) {}, + } + // Type check the package. + _, err := context.Check(fs, astFiles) + if err != nil && *verbose { + warnf("%s", err) + } + for _, file := range files { + file.pkg = pkg + file.walkFile(file.name, file.file) + } } func visit(path string, f os.FileInfo, err error) error { if err != nil { errorf("walk error: %s", err) - return nil } - if !f.IsDir() && strings.HasSuffix(path, ".go") { - doFile(path, nil) + // One package per directory. Ignore the files themselves. + if !f.IsDir() { + return nil } + doPackageDir(path) return nil } @@ -117,11 +244,18 @@ func walkDir(root string) { filepath.Walk(root, visit) } -// error formats the error to standard error, adding program -// identification and a newline +// errorf formats the error to standard error, adding program +// identification and a newline, and exits. func errorf(format string, args ...interface{}) { fmt.Fprintf(os.Stderr, "vet: "+format+"\n", args...) - setExit(2) + os.Exit(2) +} + +// warnf formats the error to standard error, adding program +// identification and a newline, but does not exit. +func warnf(format string, args ...interface{}) { + fmt.Fprintf(os.Stderr, "vet: "+format+"\n", args...) + setExit(1) } // Println is fmt.Println guarded by -v. @@ -152,16 +286,22 @@ func (f *File) Badf(pos token.Pos, format string, args ...interface{}) { setExit(1) } +func (f *File) loc(pos token.Pos) string { + // Do not print columns. Because the pos often points to the start of an + // expression instead of the inner part with the actual error, the + // precision can mislead. + posn := f.fset.Position(pos) + return fmt.Sprintf("%s:%d: ", posn.Filename, posn.Line) +} + // Warn reports an error but does not set the exit code. func (f *File) Warn(pos token.Pos, args ...interface{}) { - loc := f.fset.Position(pos).String() + ": " - fmt.Fprint(os.Stderr, loc+fmt.Sprintln(args...)) + fmt.Fprint(os.Stderr, f.loc(pos)+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.fset.Position(pos).String() + ": " - fmt.Fprintf(os.Stderr, loc+format+"\n", args...) + fmt.Fprintf(os.Stderr, f.loc(pos)+format+"\n", args...) } // walkFile walks the file's tree. @@ -173,6 +313,8 @@ func (f *File) walkFile(name string, file *ast.File) { // Visit implements the ast.Visitor interface. func (f *File) Visit(node ast.Node) ast.Visitor { switch n := node.(type) { + case *ast.AssignStmt: + f.walkAssignStmt(n) case *ast.CallExpr: f.walkCallExpr(n) case *ast.CompositeLit: @@ -183,15 +325,32 @@ func (f *File) Visit(node ast.Node) ast.Visitor { f.walkMethodDecl(n) case *ast.InterfaceType: f.walkInterfaceType(n) + case *ast.RangeStmt: + f.walkRangeStmt(n) } return f } +// walkAssignStmt walks an assignment statement +func (f *File) walkAssignStmt(stmt *ast.AssignStmt) { + f.checkAtomicAssignment(stmt) +} + // walkCall walks a call expression. func (f *File) walkCall(call *ast.CallExpr, name string) { f.checkFmtPrintfCall(call, name) } +// walkCallExpr walks a call expression. +func (f *File) walkCallExpr(call *ast.CallExpr) { + switch x := call.Fun.(type) { + case *ast.Ident: + f.walkCall(call, x.Name) + case *ast.SelectorExpr: + f.walkCall(call, x.Sel.Name) + } +} + // walkCompositeLit walks a composite literal. func (f *File) walkCompositeLit(c *ast.CompositeLit) { f.checkUntaggedLiteral(c) @@ -228,12 +387,14 @@ func (f *File) walkInterfaceType(t *ast.InterfaceType) { } } -// walkCallExpr walks a call expression. -func (f *File) walkCallExpr(call *ast.CallExpr) { - switch x := call.Fun.(type) { - case *ast.Ident: - f.walkCall(call, x.Name) - case *ast.SelectorExpr: - f.walkCall(call, x.Sel.Name) - } +// walkRangeStmt walks a range statement. +func (f *File) walkRangeStmt(n *ast.RangeStmt) { + checkRangeLoop(f, n) +} + +// gofmt returns a string representation of the expression. +func (f *File) gofmt(x ast.Expr) string { + f.b.Reset() + printer.Fprint(&f.b, f.fset, x) + return f.b.String() } diff --git a/src/cmd/vet/method.go b/src/cmd/vet/method.go index 41cb40ff9..bf982dc7a 100644 --- a/src/cmd/vet/method.go +++ b/src/cmd/vet/method.go @@ -55,6 +55,9 @@ var canonicalMethods = map[string]MethodSig{ } func (f *File) checkCanonicalMethod(id *ast.Ident, t *ast.FuncType) { + if !vet("methods") { + return + } // Expected input/output. expect, ok := canonicalMethods[id.Name] if !ok { @@ -87,9 +90,7 @@ func (f *File) checkCanonicalMethod(id *ast.Ident, t *ast.FuncType) { fmt.Fprintf(&f.b, "<%s>", err) } actual := f.b.String() - if strings.HasPrefix(actual, "func(") { - actual = actual[4:] - } + actual = strings.TrimPrefix(actual, "func") actual = id.Name + actual f.Warnf(id.Pos(), "method %s should have signature %s", actual, expectFmt) diff --git a/src/cmd/vet/print.go b/src/cmd/vet/print.go index ee9a33c70..fb0fb9f9b 100644 --- a/src/cmd/vet/print.go +++ b/src/cmd/vet/print.go @@ -8,9 +8,10 @@ package main import ( "flag" - "fmt" "go/ast" "go/token" + "go/types" + "strconv" "strings" "unicode/utf8" ) @@ -43,6 +44,9 @@ var printList = map[string]int{ // checkCall triggers the print-specific checks if the call invokes a print function. func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) { + if !vet("printf") { + return + } name := strings.ToLower(Name) if skip, ok := printfList[name]; ok { f.checkPrintf(call, Name, skip) @@ -54,73 +58,134 @@ func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) { } } +// literal returns the literal value represented by the expression, or nil if it is not a literal. +func (f *File) literal(value ast.Expr) *ast.BasicLit { + switch v := value.(type) { + case *ast.BasicLit: + return v + case *ast.ParenExpr: + return f.literal(v.X) + case *ast.BinaryExpr: + if v.Op != token.ADD { + break + } + litX := f.literal(v.X) + litY := f.literal(v.Y) + if litX != nil && litY != nil { + lit := *litX + x, errX := strconv.Unquote(litX.Value) + y, errY := strconv.Unquote(litY.Value) + if errX == nil && errY == nil { + return &ast.BasicLit{ + ValuePos: lit.ValuePos, + Kind: lit.Kind, + Value: strconv.Quote(x + y), + } + } + } + case *ast.Ident: + // See if it's a constant or initial value (we can't tell the difference). + if v.Obj == nil || v.Obj.Decl == nil { + return nil + } + valueSpec, ok := v.Obj.Decl.(*ast.ValueSpec) + if ok && len(valueSpec.Names) == len(valueSpec.Values) { + // Find the index in the list of names + var i int + for i = 0; i < len(valueSpec.Names); i++ { + if valueSpec.Names[i].Name == v.Name { + if lit, ok := valueSpec.Values[i].(*ast.BasicLit); ok { + return lit + } + return nil + } + } + } + } + return nil +} + // 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 { +// call.Args[formatIndex] is (well, should be) the format argument. +func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) { + if formatIndex >= len(call.Args) { return } - // Common case: literal is first argument. - arg := call.Args[skip] - lit, ok := arg.(*ast.BasicLit) - if !ok { - // Too hard to check. + lit := f.literal(call.Args[formatIndex]) + if lit == nil { if *verbose { f.Warn(call.Pos(), "can't check non-literal format in call to", name) } return } - if lit.Kind == token.STRING { - if !strings.Contains(lit.Value, "%") { - if len(call.Args) > skip+1 { - f.Badf(call.Pos(), "no formatting directive in %s call", name) - } - return + if lit.Kind != token.STRING { + f.Badf(call.Pos(), "literal %v not a string in call to", lit.Value, name) + } + format, err := strconv.Unquote(lit.Value) + if err != nil { + // Shouldn't happen if parser returned no errors, but be safe. + f.Badf(call.Pos(), "invalid quoted string literal") + } + firstArg := formatIndex + 1 // Arguments are immediately after format string. + if !strings.Contains(format, "%") { + if len(call.Args) > firstArg { + f.Badf(call.Pos(), "no formatting directive in %s call", name) } + return } // Hard part: check formats against args. - // Trivial but useful test: count. - numArgs := 0 - for i, w := 0, 0; i < len(lit.Value); i += w { + argNum := firstArg + for i, w := 0, 0; i < len(format); i += w { w = 1 - if lit.Value[i] == '%' { - nbytes, nargs := f.parsePrintfVerb(call, lit.Value[i:]) + if format[i] == '%' { + verb, flags, nbytes, nargs := f.parsePrintfVerb(call, format[i:]) w = nbytes - numArgs += nargs + if verb == '%' { // "%%" does nothing interesting. + continue + } + // If we've run out of args, print after loop will pick that up. + if argNum+nargs <= len(call.Args) { + f.checkPrintfArg(call, verb, flags, argNum, nargs) + } + argNum += nargs } } - expect := len(call.Args) - (skip + 1) - if numArgs != expect { - f.Badf(call.Pos(), "wrong number of args in %s call: %d needed but %d args", name, numArgs, expect) + // TODO: Dotdotdot is hard. + if call.Ellipsis.IsValid() && argNum != len(call.Args) { + return + } + if argNum != len(call.Args) { + expect := argNum - firstArg + numArgs := len(call.Args) - firstArg + f.Badf(call.Pos(), "wrong number of args for format in %s call: %d needed but %d args", name, expect, numArgs) } } -// parsePrintfVerb returns the number of bytes and number of arguments -// consumed by the Printf directive that begins s, including its percent sign -// and verb. -func (f *File) parsePrintfVerb(call *ast.CallExpr, s string) (nbytes, nargs int) { +// parsePrintfVerb returns the verb that begins the format string, along with its flags, +// the number of bytes to advance the format to step past the verb, and number of +// arguments it consumes. +func (f *File) parsePrintfVerb(call *ast.CallExpr, format string) (verb rune, flags []byte, nbytes, nargs int) { // There's guaranteed a percent sign. - flags := make([]byte, 0, 5) + flags = make([]byte, 0, 5) nbytes = 1 - end := len(s) + end := len(format) // There may be flags. FlagLoop: for nbytes < end { - switch s[nbytes] { + switch format[nbytes] { case '#', '0', '+', '-', ' ': - flags = append(flags, s[nbytes]) + flags = append(flags, format[nbytes]) nbytes++ default: break FlagLoop } } getNum := func() { - if nbytes < end && s[nbytes] == '*' { + if nbytes < end && format[nbytes] == '*' { nbytes++ nargs++ } else { - for nbytes < end && '0' <= s[nbytes] && s[nbytes] <= '9' { + for nbytes < end && '0' <= format[nbytes] && format[nbytes] <= '9' { nbytes++ } } @@ -128,24 +193,38 @@ FlagLoop: // There may be a width. getNum() // If there's a period, there may be a precision. - if nbytes < end && s[nbytes] == '.' { + if nbytes < end && format[nbytes] == '.' { flags = append(flags, '.') // Treat precision as a flag. nbytes++ getNum() } // Now a verb. - c, w := utf8.DecodeRuneInString(s[nbytes:]) + c, w := utf8.DecodeRuneInString(format[nbytes:]) nbytes += w + verb = c if c != '%' { nargs++ - f.checkPrintfVerb(call, c, flags) } return } +// printfArgType encodes the types of expressions a printf verb accepts. It is a bitmask. +type printfArgType int + +const ( + argBool printfArgType = 1 << iota + argInt + argRune + argString + argFloat + argPointer + anyType printfArgType = ^0 +) + type printVerb struct { verb rune flags string // known flags are all ASCII + typ printfArgType } // Common flag sets for printf verbs. @@ -164,36 +243,57 @@ var printVerbs = []printVerb{ // '+' is required sign for numbers, Go format for %v. // '#' is alternate format for several verbs. // ' ' is spacer for numbers - {'b', numFlag}, - {'c', "-"}, - {'d', numFlag}, - {'e', numFlag}, - {'E', numFlag}, - {'f', numFlag}, - {'F', numFlag}, - {'g', numFlag}, - {'G', numFlag}, - {'o', sharpNumFlag}, - {'p', "-#"}, - {'q', "-+#."}, - {'s', "-."}, - {'t', "-"}, - {'T', "-"}, - {'U', "-#"}, - {'v', allFlags}, - {'x', sharpNumFlag}, - {'X', sharpNumFlag}, + {'b', numFlag, argInt | argFloat}, + {'c', "-", argRune | argInt}, + {'d', numFlag, argInt}, + {'e', numFlag, argFloat}, + {'E', numFlag, argFloat}, + {'f', numFlag, argFloat}, + {'F', numFlag, argFloat}, + {'g', numFlag, argFloat}, + {'G', numFlag, argFloat}, + {'o', sharpNumFlag, argInt}, + {'p', "-#", argPointer}, + {'q', " -+.0#", argRune | argInt | argString}, + {'s', " -+.0", argString}, + {'t', "-", argBool}, + {'T', "-", anyType}, + {'U', "-#", argRune | argInt}, + {'v', allFlags, anyType}, + {'x', sharpNumFlag, argRune | argInt | argString}, + {'X', sharpNumFlag, argRune | argInt | argString}, } const printfVerbs = "bcdeEfFgGopqstTvxUX" -func (f *File) checkPrintfVerb(call *ast.CallExpr, verb rune, flags []byte) { +func (f *File) checkPrintfArg(call *ast.CallExpr, verb rune, flags []byte, argNum, nargs int) { // Linear scan is fast enough for a small list. for _, v := range printVerbs { if v.verb == verb { for _, flag := range flags { if !strings.ContainsRune(v.flags, rune(flag)) { f.Badf(call.Pos(), "unrecognized printf flag for verb %q: %q", verb, flag) + return + } + } + // Verb is good. If nargs>1, we have something like %.*s and all but the final + // arg must be integer. + for i := 0; i < nargs-1; i++ { + if !f.matchArgType(argInt, call.Args[argNum+i]) { + f.Badf(call.Pos(), "arg %s for * in printf format not of type int", f.gofmt(call.Args[argNum+i])) + } + } + for _, v := range printVerbs { + if v.verb == verb { + arg := call.Args[argNum+nargs-1] + if !f.matchArgType(v.typ, arg) { + typeString := "" + if typ := f.pkg.types[arg]; typ != nil { + typeString = typ.String() + } + f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: %s", f.gofmt(arg), verb, typeString) + } + break } } return @@ -202,15 +302,67 @@ func (f *File) checkPrintfVerb(call *ast.CallExpr, verb rune, flags []byte) { f.Badf(call.Pos(), "unrecognized printf verb %q", verb) } +func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool { + // TODO: for now, we can only test builtin types and untyped constants. + typ := f.pkg.types[arg] + if typ == nil { + return true + } + basic, ok := typ.(*types.Basic) + if !ok { + return true + } + switch basic.Kind { + case types.Bool: + return t&argBool != 0 + case types.Int, types.Int8, types.Int16, types.Int32, types.Int64: + fallthrough + case types.Uint, types.Uint8, types.Uint16, types.Uint32, types.Uint64, types.Uintptr: + return t&argInt != 0 + case types.Float32, types.Float64, types.Complex64, types.Complex128: + return t&argFloat != 0 + case types.String: + return t&argString != 0 + case types.UnsafePointer: + return t&(argPointer|argInt) != 0 + case types.UntypedBool: + return t&argBool != 0 + case types.UntypedComplex: + return t&argFloat != 0 + case types.UntypedFloat: + // If it's integral, we can use an int format. + switch f.pkg.values[arg].(type) { + case int, int8, int16, int32, int64: + return t&(argInt|argFloat) != 0 + case uint, uint8, uint16, uint32, uint64: + return t&(argInt|argFloat) != 0 + } + return t&argFloat != 0 + case types.UntypedInt: + return t&argInt != 0 + case types.UntypedRune: + return t&(argInt|argRune) != 0 + case types.UntypedString: + return t&argString != 0 + case types.UntypedNil: + return t&argPointer != 0 // TODO? + case types.Invalid: + if *verbose { + f.Warnf(arg.Pos(), "printf argument %v has invalid or unknown type", arg) + } + return true // Probably a type check problem. + } + return false +} + // 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) { +// call.Args[firstArg] is the first argument to be printed. +func (f *File) checkPrint(call *ast.CallExpr, name string, firstArg int) { isLn := strings.HasSuffix(name, "ln") isF := strings.HasPrefix(name, "F") args := call.Args // check for Println(os.Stderr, ...) - if skip == 0 && !isF && len(args) > 0 { + if firstArg == 0 && !isF && len(args) > 0 { if sel, ok := args[0].(*ast.SelectorExpr); ok { if x, ok := sel.X.(*ast.Ident); ok { if x.Name == "os" && strings.HasPrefix(sel.Sel.Name, "Std") { @@ -219,13 +371,23 @@ func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) { } } } - if len(args) <= skip { - if *verbose && !isLn { - f.Badf(call.Pos(), "no args in %s call", name) + if len(args) <= firstArg { + // If we have a call to a method called Error that satisfies the Error interface, + // then it's ok. Otherwise it's something like (*T).Error from the testing package + // and we need to check it. + if name == "Error" && f.isErrorMethodCall(call) { + return + } + // If it's an Error call now, it's probably for printing errors. + if !isLn { + // Check the signature to be sure: there are niladic functions called "error". + if firstArg != 0 || f.numArgsInSignature(call) != firstArg { + f.Badf(call.Pos(), "no args in %s call", name) + } } return } - arg := args[skip] + arg := args[firstArg] if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRING { if strings.Contains(lit.Value, "%") { f.Badf(call.Pos(), "possible formatting directive in %s call", name) @@ -242,37 +404,63 @@ func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) { } } -// This function never executes, but it serves as a simple test for the program. -// Test with make test. -func BadFunctionUsedInTests() { - 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 in Printf call" - fmt.Printf("%s%%%d", "hi", 3) // correct - fmt.Printf("%.*d", 3, 3) // correct - fmt.Printf("%.*d", 3, 3, 3) // ERROR "wrong number of args in Printf call" - printf("now is the time", "buddy") // ERROR "no formatting directive" - Printf("now is the time", "buddy") // ERROR "no formatting directive" - Printf("hi") // ok - f := new(File) - f.Warn(0, "%s", "hello", 3) // ERROR "possible formatting directive in Warn call" - f.Warnf(0, "%s", "hello", 3) // ERROR "wrong number of args in Warnf call" - f.Warnf(0, "%r", "hello") // ERROR "unrecognized printf verb" - f.Warnf(0, "%#s", "hello") // ERROR "unrecognized printf flag" -} - -type BadTypeUsedInTests struct { - X int "hello" // ERROR "struct field tag" -} - -func (t *BadTypeUsedInTests) Scan(x fmt.ScanState, c byte) { // ERROR "method Scan[(]x fmt.ScanState, c byte[)] should have signature Scan[(]fmt.ScanState, rune[)] error" -} - -type BadInterfaceUsedInTests interface { - ReadByte() byte // ERROR "method ReadByte[(][)] byte should have signature ReadByte[(][)] [(]byte, error[)]" +// numArgsInSignature tells how many formal arguments the function type +// being called has. +func (f *File) numArgsInSignature(call *ast.CallExpr) int { + // Check the type of the function or method declaration + typ := f.pkg.types[call.Fun] + if typ == nil { + return 0 + } + // The type must be a signature, but be sure for safety. + sig, ok := typ.(*types.Signature) + if !ok { + return 0 + } + return len(sig.Params) } -// printf is used by the test. -func printf(format string, args ...interface{}) { - panic("don't call - testing only") +// isErrorMethodCall reports whether the call is of a method with signature +// func Error() string +// where "string" is the universe's string type. We know the method is called "Error". +func (f *File) isErrorMethodCall(call *ast.CallExpr) bool { + // Is it a selector expression? Otherwise it's a function call, not a method call. + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + // The package is type-checked, so if there are no arguments, we're done. + if len(call.Args) > 0 { + return false + } + // Check the type of the method declaration + typ := f.pkg.types[sel] + if typ == nil { + return false + } + // The type must be a signature, but be sure for safety. + sig, ok := typ.(*types.Signature) + if !ok { + return false + } + // There must be a receiver for it to be a method call. Otherwise it is + // a function, not something that satisfies the error interface. + if sig.Recv == nil { + return false + } + // There must be no arguments. Already verified by type checking, but be thorough. + if len(sig.Params) > 0 { + return false + } + // Finally the real questions. + // There must be one result. + if len(sig.Results) != 1 { + return false + } + // It must have return type "string" from the universe. + result := sig.Results[0].Type + if types.IsIdentical(result, types.Typ[types.String]) { + return true + } + return false } diff --git a/src/cmd/vet/rangeloop.go b/src/cmd/vet/rangeloop.go new file mode 100644 index 000000000..ecc595427 --- /dev/null +++ b/src/cmd/vet/rangeloop.go @@ -0,0 +1,65 @@ +// Copyright 2012 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 code to check range loop variables bound inside function +literals that are deferred or launched in new goroutines. We only check +instances where the defer or go statement is the last statement in the loop +body, as otherwise we would need whole program analysis. + +For example: + + for i, v := range s { + go func() { + println(i, v) // not what you might expect + }() + } + +See: http://golang.org/doc/go_faq.html#closures_and_goroutines +*/ + +package main + +import "go/ast" + +// checkRangeLoop walks the body of the provided range statement, checking if +// its index or value variables are used unsafely inside goroutines or deferred +// function literals. +func checkRangeLoop(f *File, n *ast.RangeStmt) { + if !vet("rangeloops") { + return + } + key, _ := n.Key.(*ast.Ident) + val, _ := n.Value.(*ast.Ident) + if key == nil && val == nil { + return + } + sl := n.Body.List + if len(sl) == 0 { + return + } + var last *ast.CallExpr + switch s := sl[len(sl)-1].(type) { + case *ast.GoStmt: + last = s.Call + case *ast.DeferStmt: + last = s.Call + default: + return + } + lit, ok := last.Fun.(*ast.FuncLit) + if !ok { + return + } + ast.Inspect(lit.Body, func(n ast.Node) bool { + id, ok := n.(*ast.Ident) + if !ok || id.Obj == nil { + return true + } + if key != nil && id.Obj == key.Obj || val != nil && id.Obj == val.Obj { + f.Warn(id.Pos(), "range variable", id.Name, "enclosed by function") + } + return true + }) +} diff --git a/src/cmd/vet/structtag.go b/src/cmd/vet/structtag.go index ea2a9d863..545e420c1 100644 --- a/src/cmd/vet/structtag.go +++ b/src/cmd/vet/structtag.go @@ -14,6 +14,9 @@ import ( // checkField checks a struct field tag. func (f *File) checkCanonicalFieldTag(field *ast.Field) { + if !vet("structtags") { + return + } if field.Tag == nil { return } diff --git a/src/cmd/vet/taglit.go b/src/cmd/vet/taglit.go index c3c4f3234..0324e37b0 100644 --- a/src/cmd/vet/taglit.go +++ b/src/cmd/vet/taglit.go @@ -7,13 +7,38 @@ package main import ( + "flag" "go/ast" + "go/types" "strings" ) +var compositeWhiteList = flag.Bool("compositewhitelist", true, "use composite white list; for testing only") + // checkUntaggedLiteral checks if a composite literal is an struct literal with // untagged fields. func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) { + if !vet("composites") { + return + } + + // Check that the CompositeLit's type is a slice or array (which needs no tag), if possible. + typ := f.pkg.types[c] + if typ != nil { + // If it's a named type, pull out the underlying type. + if namedType, ok := typ.(*types.NamedType); ok { + typ = namedType.Underlying + } + switch typ.(type) { + case *types.Slice: + return + case *types.Array: + return + } + } + + // It's a struct, or we can't tell it's not a struct because we don't have types. + // Check if the CompositeLit contains an untagged field. allKeyValue := true for _, e := range c.Elts { @@ -42,12 +67,12 @@ func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) { f.Warnf(c.Pos(), "unresolvable package for %s.%s literal", pkg.Name, s.Sel.Name) return } - typ := path + "." + s.Sel.Name - if untaggedLiteralWhitelist[typ] { + typeName := path + "." + s.Sel.Name + if *compositeWhiteList && untaggedLiteralWhitelist[typeName] { return } - f.Warnf(c.Pos(), "%s struct literal uses untagged fields", typ) + f.Warnf(c.Pos(), "%s composite literal uses untagged fields", typ) } // pkgPath returns the import path "image/png" for the package name "png". diff --git a/src/cmd/vet/test_atomic.go b/src/cmd/vet/test_atomic.go new file mode 100644 index 000000000..9231e9dc0 --- /dev/null +++ b/src/cmd/vet/test_atomic.go @@ -0,0 +1,43 @@ +// Copyright 2013 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. + +// +build vet_test + +// This file contains tests for the atomic checker. + +package main + +import ( + "sync/atomic" +) + +type Counter uint64 + +func AtomicTests() { + x := uint64(1) + x = atomic.AddUint64(&x, 1) // ERROR "direct assignment to atomic value" + _, x = 10, atomic.AddUint64(&x, 1) // ERROR "direct assignment to atomic value" + x, _ = atomic.AddUint64(&x, 1), 10 // ERROR "direct assignment to atomic value" + + y := &x + *y = atomic.AddUint64(y, 1) // ERROR "direct assignment to atomic value" + + var su struct{ Counter uint64 } + su.Counter = atomic.AddUint64(&su.Counter, 1) // ERROR "direct assignment to atomic value" + z1 := atomic.AddUint64(&su.Counter, 1) + _ = z1 // Avoid err "z declared and not used" + + var sp struct{ Counter *uint64 } + *sp.Counter = atomic.AddUint64(sp.Counter, 1) // ERROR "direct assignment to atomic value" + z2 := atomic.AddUint64(sp.Counter, 1) + _ = z2 // Avoid err "z declared and not used" + + au := []uint64{10, 20} + au[0] = atomic.AddUint64(&au[0], 1) // ERROR "direct assignment to atomic value" + au[1] = atomic.AddUint64(&au[0], 1) + + ap := []*uint64{&au[0], &au[1]} + *ap[0] = atomic.AddUint64(ap[0], 1) // ERROR "direct assignment to atomic value" + *ap[1] = atomic.AddUint64(ap[0], 1) +} diff --git a/src/cmd/vet/test_buildtag.go b/src/cmd/vet/test_buildtag.go new file mode 100644 index 000000000..d7174ade2 --- /dev/null +++ b/src/cmd/vet/test_buildtag.go @@ -0,0 +1,15 @@ +// Copyright 2013 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 buildtag checker. + +// +build vet_test +// +builder // ERROR "possible malformed \+build comment" +// +build !ignore + +package main + +// +build toolate // ERROR "build comment appears too late in file" + +var _ = 3 diff --git a/src/cmd/vet/test_method.go b/src/cmd/vet/test_method.go new file mode 100644 index 000000000..41de62bb1 --- /dev/null +++ b/src/cmd/vet/test_method.go @@ -0,0 +1,24 @@ +// 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 tests for the canonical method checker. + +// +build vet_test + +// This file contains the code to check canonical methods. + +package main + +import ( + "fmt" +) + +type MethodTest int + +func (t *MethodTest) Scan(x fmt.ScanState, c byte) { // ERROR "should have signature Scan" +} + +type MethodTestInterface interface { + ReadByte() byte // ERROR "should have signature ReadByte" +} diff --git a/src/cmd/vet/test_print.go b/src/cmd/vet/test_print.go new file mode 100644 index 000000000..8b41e6c69 --- /dev/null +++ b/src/cmd/vet/test_print.go @@ -0,0 +1,153 @@ +// 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. + +// +build vet_test + +// This file contains tests for the printf checker. + +package main + +import ( + "fmt" + "unsafe" // just for test case printing unsafe.Pointer +) + +func UnsafePointerPrintfTest() { + var up unsafe.Pointer + fmt.Printf("%p, %x %X", up, up, up) +} + +// Error methods that do not satisfy the Error interface and should be checked. +type errorTest1 int + +func (errorTest1) Error(...interface{}) string { + return "hi" +} + +type errorTest2 int // Analogous to testing's *T type. +func (errorTest2) Error(...interface{}) { +} + +type errorTest3 int + +func (errorTest3) Error() { // No return value. +} + +type errorTest4 int + +func (errorTest4) Error() int { // Different return type. + return 3 +} + +type errorTest5 int + +func (errorTest5) error() { // niladic; don't complain if no args (was bug) +} + +// This function never executes, but it serves as a simple test for the program. +// Test with make test. +func PrintfTests() { + var b bool + var i int + var r rune + var s string + var x float64 + var p *int + // Some good format/argtypes + fmt.Printf("") + fmt.Printf("%b %b", 3, i) + fmt.Printf("%c %c %c %c", 3, i, 'x', r) + fmt.Printf("%d %d", 3, i) + fmt.Printf("%e %e", 3e9, x) + fmt.Printf("%E %E", 3e9, x) + fmt.Printf("%f %f", 3e9, x) + fmt.Printf("%F %F", 3e9, x) + fmt.Printf("%g %g", 3e9, x) + fmt.Printf("%G %G", 3e9, x) + fmt.Printf("%o %o", 3, i) + fmt.Printf("%p %p", p, nil) + fmt.Printf("%q %q %q %q", 3, i, 'x', r) + fmt.Printf("%s %s", "hi", s) + fmt.Printf("%t %t", true, b) + fmt.Printf("%T %T", 3, i) + fmt.Printf("%U %U", 3, i) + fmt.Printf("%v %v", 3, i) + fmt.Printf("%x %x %x %x", 3, i, "hi", s) + fmt.Printf("%X %X %X %X", 3, i, "hi", s) + fmt.Printf("%.*s %d %g", 3, "hi", 23, 2.3) + // Some bad format/argTypes + fmt.Printf("%b", "hi") // ERROR "arg .hi. for printf verb %b of wrong type" + fmt.Printf("%c", 2.3) // ERROR "arg 2.3 for printf verb %c of wrong type" + fmt.Printf("%d", 2.3) // ERROR "arg 2.3 for printf verb %d of wrong type" + fmt.Printf("%e", "hi") // ERROR "arg .hi. for printf verb %e of wrong type" + fmt.Printf("%E", true) // ERROR "arg true for printf verb %E of wrong type" + fmt.Printf("%f", "hi") // ERROR "arg .hi. for printf verb %f of wrong type" + fmt.Printf("%F", 'x') // ERROR "arg 'x' for printf verb %F of wrong type" + fmt.Printf("%g", "hi") // ERROR "arg .hi. for printf verb %g of wrong type" + fmt.Printf("%G", i) // ERROR "arg i for printf verb %G of wrong type" + fmt.Printf("%o", x) // ERROR "arg x for printf verb %o of wrong type" + fmt.Printf("%p", 23) // ERROR "arg 23 for printf verb %p of wrong type" + fmt.Printf("%q", x) // ERROR "arg x for printf verb %q of wrong type" + fmt.Printf("%s", b) // ERROR "arg b for printf verb %s of wrong type" + fmt.Printf("%t", 23) // ERROR "arg 23 for printf verb %t of wrong type" + fmt.Printf("%U", x) // ERROR "arg x for printf verb %U of wrong type" + fmt.Printf("%x", nil) // ERROR "arg nil for printf verb %x of wrong type" + fmt.Printf("%X", 2.3) // ERROR "arg 2.3 for printf verb %X of wrong type" + fmt.Printf("%.*s %d %g", 3, "hi", 23, 'x') // ERROR "arg 'x' for printf verb %g of wrong type" + // TODO + 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.Printf("%"+("s"), "hi", 3) // ERROR "wrong number of args for format in Printf call" + 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", 3, 3, 3) // ERROR "wrong number of args for format in Printf call" + fmt.Printf("%.*d", "hi", 3) // ERROR "arg .hi. for \* in printf format not of type int" + fmt.Printf("%.*d", i, 3) // correct + fmt.Printf("%.*d", s, 3) // ERROR "arg s for \* in printf format not of type int" + fmt.Printf("%q %q", multi()...) // ok + fmt.Printf("%#q", `blah`) // ok + printf("now is the time", "buddy") // ERROR "no formatting directive" + Printf("now is the time", "buddy") // ERROR "no formatting directive" + Printf("hi") // ok + const format = "%s %s\n" + Printf(format, "hi", "there") + Printf(format, "hi") // ERROR "wrong number of args for format in Printf call" + f := new(File) + f.Warn(0, "%s", "hello", 3) // ERROR "possible formatting directive in Warn call" + f.Warnf(0, "%s", "hello", 3) // ERROR "wrong number of args for format in Warnf call" + f.Warnf(0, "%r", "hello") // ERROR "unrecognized printf verb" + f.Warnf(0, "%#s", "hello") // ERROR "unrecognized printf flag" + // 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 errorTest1 + fmt.Println(et1.Error()) // ERROR "no args in Error call" + fmt.Println(et1.Error("hi")) // ok + fmt.Println(et1.Error("%d", 3)) // ERROR "possible formatting directive in Error call" + var et2 errorTest2 + et2.Error() // ERROR "no args in Error call" + et2.Error("hi") // ok, not an error method. + et2.Error("%d", 3) // ERROR "possible formatting directive in Error call" + var et3 errorTest3 + et3.Error() // ok, not an error method. + var et4 errorTest4 + et4.Error() // ok, not an error method. + var et5 errorTest5 + et5.error() // ok, not an error method. +} + +// printf is used by the test. +func printf(format string, args ...interface{}) { + panic("don't call - testing only") +} + +// multi is used by the test. +func multi() []interface{} { + panic("don't call - testing only") +} diff --git a/src/cmd/vet/test_rangeloop.go b/src/cmd/vet/test_rangeloop.go new file mode 100644 index 000000000..941fd72aa --- /dev/null +++ b/src/cmd/vet/test_rangeloop.go @@ -0,0 +1,61 @@ +// Copyright 2012 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 rangeloop checker. + +// +build vet_test + +package main + +func RangeLoopTests() { + var s []int + for i, v := range s { + go func() { + println(i) // ERROR "range variable i enclosed by function" + println(v) // ERROR "range variable v enclosed by function" + }() + } + for i, v := range s { + defer func() { + println(i) // ERROR "range variable i enclosed by function" + println(v) // ERROR "range variable v enclosed by function" + }() + } + for i := range s { + go func() { + println(i) // ERROR "range variable i enclosed by function" + }() + } + for _, v := range s { + go func() { + println(v) // ERROR "range variable v enclosed by function" + }() + } + for i, v := range s { + go func() { + println(i, v) + }() + println("unfortunately, we don't catch the error above because of this statement") + } + for i, v := range s { + go func(i, v int) { + println(i, v) + }(i, v) + } + for i, v := range s { + i, v := i, v + go func() { + println(i, v) + }() + } + // If the key of the range statement is not an identifier + // the code should not panic (it used to). + var x [2]int + var f int + for x[0], f = range s { + go func() { + _ = f // ERROR "range variable f enclosed by function" + }() + } +} diff --git a/src/cmd/vet/test_structtag.go b/src/cmd/vet/test_structtag.go new file mode 100644 index 000000000..08cf737fd --- /dev/null +++ b/src/cmd/vet/test_structtag.go @@ -0,0 +1,15 @@ +// 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 tests for the structtag checker. + +// +build vet_test + +// This file contains the test for canonical struct tags. + +package main + +type StructTagTest struct { + X int "hello" // ERROR "not compatible with reflect.StructTag.Get" +} diff --git a/src/cmd/vet/test_taglit.go b/src/cmd/vet/test_taglit.go new file mode 100644 index 000000000..0d83b18fd --- /dev/null +++ b/src/cmd/vet/test_taglit.go @@ -0,0 +1,31 @@ +// Copyright 2012 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 untagged struct literal checker. + +// +build vet_test + +// This file contains the test for untagged struct literals. + +package main + +import ( + "flag" + "go/scanner" +) + +// Testing is awkward because we need to reference things from a separate package +// to trigger the warnings. + +var BadStructLiteralUsedInTests = flag.Flag{ // ERROR "untagged fields" + "Name", + "Usage", + nil, // Value + "DefValue", +} + +// Used to test the check for slices and arrays: If that test is disabled and +// vet is run with --compositewhitelist=false, this line triggers an error. +// Clumsy but sufficient. +var scannerErrorListTest = scanner.ErrorList{nil, nil} |