diff --git a/src/cmd/vet/doc.go b/src/cmd/vet/doc.go index 53db6dde93b..bb3238fc9ed 100644 --- a/src/cmd/vet/doc.go +++ b/src/cmd/vet/doc.go @@ -85,12 +85,12 @@ Flag: -copylocks Locks that are erroneously passed by value. -Documentation examples +Tests, benchmarks and documentation examples -Flag: -example +Flag: -tests -Mistakes involving example tests, including examples with incorrect names or -function signatures, or that document identifiers not in the package. +Mistakes involving tests including functions with incorrect names or signatures +and example tests that document identifiers not in the package. Methods diff --git a/src/cmd/vet/example.go b/src/cmd/vet/example.go deleted file mode 100644 index 797c3ceec83..00000000000 --- a/src/cmd/vet/example.go +++ /dev/null @@ -1,124 +0,0 @@ -// 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. - -package main - -import ( - "go/ast" - "go/types" - "strings" - "unicode" - "unicode/utf8" -) - -func init() { - register("example", - "check for common mistaken usages of documentation examples", - checkExample, - funcDecl) -} - -func isExampleSuffix(s string) bool { - r, size := utf8.DecodeRuneInString(s) - return size > 0 && unicode.IsLower(r) -} - -// checkExample walks the documentation example functions checking for common -// mistakes of misnamed functions, failure to map functions to existing -// identifiers, etc. -func checkExample(f *File, node ast.Node) { - if !strings.HasSuffix(f.name, "_test.go") { - return - } - var ( - pkg = f.pkg - pkgName = pkg.typesPkg.Name() - scopes = []*types.Scope{pkg.typesPkg.Scope()} - lookup = func(name string) types.Object { - for _, scope := range scopes { - if o := scope.Lookup(name); o != nil { - return o - } - } - return nil - } - ) - if strings.HasSuffix(pkgName, "_test") { - // Treat 'package foo_test' as an alias for 'package foo'. - var ( - basePkg = strings.TrimSuffix(pkgName, "_test") - pkg = f.pkg - ) - for _, p := range pkg.typesPkg.Imports() { - if p.Name() == basePkg { - scopes = append(scopes, p.Scope()) - break - } - } - } - fn, ok := node.(*ast.FuncDecl) - if !ok { - // Ignore non-functions. - return - } - var ( - fnName = fn.Name.Name - report = func(format string, args ...interface{}) { f.Badf(node.Pos(), format, args...) } - ) - if fn.Recv != nil || !strings.HasPrefix(fnName, "Example") { - // Ignore methods and types not named "Example". - return - } - if params := fn.Type.Params; len(params.List) != 0 { - report("%s should be niladic", fnName) - } - if results := fn.Type.Results; results != nil && len(results.List) != 0 { - report("%s should return nothing", fnName) - } - if fnName == "Example" { - // Nothing more to do. - return - } - if filesRun && !includesNonTest { - // The coherence checks between a test and the package it tests - // will report false positives if no non-test files have - // been provided. - return - } - var ( - exName = strings.TrimPrefix(fnName, "Example") - elems = strings.SplitN(exName, "_", 3) - ident = elems[0] - obj = lookup(ident) - ) - if ident != "" && obj == nil { - // Check ExampleFoo and ExampleBadFoo. - report("%s refers to unknown identifier: %s", fnName, ident) - // Abort since obj is absent and no subsequent checks can be performed. - return - } - if elemCnt := strings.Count(exName, "_"); elemCnt == 0 { - // Nothing more to do. - return - } - mmbr := elems[1] - if ident == "" { - // Check Example_suffix and Example_BadSuffix. - if residual := strings.TrimPrefix(exName, "_"); !isExampleSuffix(residual) { - report("%s has malformed example suffix: %s", fnName, residual) - } - return - } - if !isExampleSuffix(mmbr) { - // Check ExampleFoo_Method and ExampleFoo_BadMethod. - if obj, _, _ := types.LookupFieldOrMethod(obj.Type(), true, obj.Pkg(), mmbr); obj == nil { - report("%s refers to unknown field or method: %s.%s", fnName, ident, mmbr) - } - } - if len(elems) == 3 && !isExampleSuffix(elems[2]) { - // Check ExampleFoo_Method_suffix and ExampleFoo_Method_Badsuffix. - report("%s has malformed example suffix: %s", fnName, elems[2]) - } - return -} diff --git a/src/cmd/vet/testdata/examples_test.go b/src/cmd/vet/testdata/tests_test.go similarity index 59% rename from src/cmd/vet/testdata/examples_test.go rename to src/cmd/vet/testdata/tests_test.go index 9c53672a7d5..f5bbc3922a9 100644 --- a/src/cmd/vet/testdata/examples_test.go +++ b/src/cmd/vet/testdata/tests_test.go @@ -1,7 +1,9 @@ -// Test of examples. - package testdata +import ( + "testing" +) + // Buf is a ... type Buf []byte @@ -46,3 +48,27 @@ func ExamplePuffer() // ERROR "ExamplePuffer refers to unknown identifier: Puffe func ExamplePuffer_Append() // ERROR "ExamplePuffer_Append refers to unknown identifier: Puffer" func ExamplePuffer_suffix() // ERROR "ExamplePuffer_suffix refers to unknown identifier: Puffer" + +func nonTest() {} // OK because it doesn't start with "Test". + +func (Buf) TesthasReceiver() {} // OK because it has a receiver. + +func TestOKSuffix(*testing.T) {} // OK because first char after "Test" is Uppercase. + +func TestÜnicodeWorks(*testing.T) {} // OK because the first char after "Test" is Uppercase. + +func TestbadSuffix(*testing.T) {} // ERROR "first letter after 'Test' must not be lowercase" + +func TestemptyImportBadSuffix(*T) {} // ERROR "first letter after 'Test' must not be lowercase" + +func Test(*testing.T) {} // OK "Test" on its own is considered a test. + +func Testify() {} // OK because it takes no parameters. + +func TesttooManyParams(*testing.T, string) {} // OK because it takes too many parameters. + +func TesttooManyNames(a, b *testing.T) {} // OK because it takes too many names. + +func TestnoTParam(string) {} // OK because it doesn't take a *testing.T + +func BenchmarkbadSuffix(*testing.B) {} // ERROR "first letter after 'Benchmark' must not be lowercase" diff --git a/src/cmd/vet/tests.go b/src/cmd/vet/tests.go new file mode 100644 index 00000000000..52ad3340983 --- /dev/null +++ b/src/cmd/vet/tests.go @@ -0,0 +1,182 @@ +// 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. + +package main + +import ( + "go/ast" + "go/types" + "strings" + "unicode" + "unicode/utf8" +) + +func init() { + register("tests", + "check for common mistaken usages of tests/documentation examples", + checkTestFunctions, + funcDecl) +} + +func isExampleSuffix(s string) bool { + r, size := utf8.DecodeRuneInString(s) + return size > 0 && unicode.IsLower(r) +} + +func isTestSuffix(name string) bool { + if len(name) == 0 { + // "Test" is ok. + return true + } + r, _ := utf8.DecodeRuneInString(name) + return !unicode.IsLower(r) +} + +func isTestParam(typ ast.Expr, wantType string) bool { + ptr, ok := typ.(*ast.StarExpr) + if !ok { + // Not a pointer. + return false + } + // No easy way of making sure it's a *testing.T or *testing.B: + // ensure the name of the type matches. + if name, ok := ptr.X.(*ast.Ident); ok { + return name.Name == wantType + } + if sel, ok := ptr.X.(*ast.SelectorExpr); ok { + return sel.Sel.Name == wantType + } + return false +} + +func lookup(name string, scopes []*types.Scope) types.Object { + for _, scope := range scopes { + if o := scope.Lookup(name); o != nil { + return o + } + } + return nil +} + +func extendedScope(pkg *Package) []*types.Scope { + scopes := []*types.Scope{pkg.typesPkg.Scope()} + + pkgName := pkg.typesPkg.Name() + if strings.HasPrefix(pkgName, "_test") { + basePkg := strings.TrimSuffix(pkgName, "_test") + for _, p := range pkg.typesPkg.Imports() { + if p.Name() == basePkg { + scopes = append(scopes, p.Scope()) + break + } + } + } + return scopes +} + +func checkExample(fn *ast.FuncDecl, pkg *Package, report reporter) { + fnName := fn.Name.Name + if params := fn.Type.Params; len(params.List) != 0 { + report("%s should be niladic", fnName) + } + if results := fn.Type.Results; results != nil && len(results.List) != 0 { + report("%s should return nothing", fnName) + } + + if filesRun && !includesNonTest { + // The coherence checks between a test and the package it tests + // will report false positives if no non-test files have + // been provided. + return + } + + if fnName == "Example" { + // Nothing more to do. + return + } + + var ( + exName = strings.TrimPrefix(fnName, "Example") + elems = strings.SplitN(exName, "_", 3) + ident = elems[0] + obj = lookup(ident, extendedScope(pkg)) + ) + if ident != "" && obj == nil { + // Check ExampleFoo and ExampleBadFoo. + report("%s refers to unknown identifier: %s", fnName, ident) + // Abort since obj is absent and no subsequent checks can be performed. + return + } + if len(elems) < 2 { + // Nothing more to do. + return + } + + if ident == "" { + // Check Example_suffix and Example_BadSuffix. + if residual := strings.TrimPrefix(exName, "_"); !isExampleSuffix(residual) { + report("%s has malformed example suffix: %s", fnName, residual) + } + return + } + + mmbr := elems[1] + if !isExampleSuffix(mmbr) { + // Check ExampleFoo_Method and ExampleFoo_BadMethod. + if obj, _, _ := types.LookupFieldOrMethod(obj.Type(), true, obj.Pkg(), mmbr); obj == nil { + report("%s refers to unknown field or method: %s.%s", fnName, ident, mmbr) + } + } + if len(elems) == 3 && !isExampleSuffix(elems[2]) { + // Check ExampleFoo_Method_suffix and ExampleFoo_Method_Badsuffix. + report("%s has malformed example suffix: %s", fnName, elems[2]) + } +} + +func checkTest(fn *ast.FuncDecl, prefix string, report reporter) { + // Want functions with 0 results and 1 parameter. + if fn.Type.Results != nil && len(fn.Type.Results.List) > 0 || + fn.Type.Params == nil || + len(fn.Type.Params.List) != 1 || + len(fn.Type.Params.List[0].Names) > 1 { + return + } + + // The param must look like a *testing.T or *testing.B. + if !isTestParam(fn.Type.Params.List[0].Type, prefix[:1]) { + return + } + + if !isTestSuffix(fn.Name.Name[len(prefix):]) { + report("%s has malformed name: first letter after '%s' must not be lowercase", fn.Name.Name, prefix) + } +} + +type reporter func(format string, args ...interface{}) + +// checkTestFunctions walks Test, Benchmark and Example functions checking +// malformed names, wrong signatures and examples documenting inexistent +// identifiers. +func checkTestFunctions(f *File, node ast.Node) { + if !strings.HasSuffix(f.name, "_test.go") { + return + } + + fn, ok := node.(*ast.FuncDecl) + if !ok || fn.Recv != nil { + // Ignore non-functions or functions with receivers. + return + } + + report := func(format string, args ...interface{}) { f.Badf(node.Pos(), format, args...) } + + switch { + case strings.HasPrefix(fn.Name.Name, "Example"): + checkExample(fn, f.pkg, report) + case strings.HasPrefix(fn.Name.Name, "Test"): + checkTest(fn, "Test", report) + case strings.HasPrefix(fn.Name.Name, "Benchmark"): + checkTest(fn, "Benchmark", report) + } +}