]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/vet: add a check for tests with malformed names
authorCatalin Nicutar <cnicutar@google.com>
Sun, 21 Feb 2016 18:32:25 +0000 (18:32 +0000)
committerRob Pike <r@golang.org>
Wed, 24 Feb 2016 10:40:34 +0000 (10:40 +0000)
According to golang.org/pkg/testing the first character after Test has
to be non-lowercase. Functions that don't conform to this are not
considered tests and are not loaded which can cause surprises.

This change adds a check to warn about Test-like functions in a _test
file that are not actually run by go test.

Moved over from https://go-review.googlesource.com/#/c/19466/

Change-Id: I2f89676058b27a0e35f721bdabc9fa8a9d34430d
Reviewed-on: https://go-review.googlesource.com/19724
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
src/cmd/vet/doc.go
src/cmd/vet/example.go [deleted file]
src/cmd/vet/testdata/tests_test.go [moved from src/cmd/vet/testdata/examples_test.go with 59% similarity]
src/cmd/vet/tests.go [new file with mode: 0644]

index 53db6dde93bca7ec4d51e5f0c09f7a42ebbcc4c4..bb3238fc9ed46e21151315a59b308c063537f366 100644 (file)
@@ -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 (file)
index 797c3ce..0000000
+++ /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
-}
similarity index 59%
rename from src/cmd/vet/testdata/examples_test.go
rename to src/cmd/vet/testdata/tests_test.go
index 9c53672a7d5ad5bc3a569ba00ae0e370c038abfc..f5bbc3922a99f1938b567c8c2156ad46314417c7 100644 (file)
@@ -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 (file)
index 0000000..52ad334
--- /dev/null
@@ -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)
+       }
+}