mirror of
https://github.com/golang/go
synced 2024-09-25 07:30:13 -06:00
cmd/vet: add a check for tests with malformed names
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>
This commit is contained in:
parent
c3ecded729
commit
6e6637bdb4
@ -85,12 +85,12 @@ Flag: -copylocks
|
|||||||
|
|
||||||
Locks that are erroneously passed by value.
|
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
|
Mistakes involving tests including functions with incorrect names or signatures
|
||||||
function signatures, or that document identifiers not in the package.
|
and example tests that document identifiers not in the package.
|
||||||
|
|
||||||
Methods
|
Methods
|
||||||
|
|
||||||
|
@ -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
|
|
||||||
}
|
|
@ -1,7 +1,9 @@
|
|||||||
// Test of examples.
|
|
||||||
|
|
||||||
package testdata
|
package testdata
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
// Buf is a ...
|
// Buf is a ...
|
||||||
type Buf []byte
|
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_Append() // ERROR "ExamplePuffer_Append refers to unknown identifier: Puffer"
|
||||||
|
|
||||||
func ExamplePuffer_suffix() // ERROR "ExamplePuffer_suffix 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"
|
182
src/cmd/vet/tests.go
Normal file
182
src/cmd/vet/tests.go
Normal file
@ -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)
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue
Block a user