From 38981630ecb498da1c103c57bf9b8edb43715328 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 5 Oct 2018 16:37:45 -0400 Subject: [PATCH] go/analysis/passes/tests: split out from vet The analysistest.Run function now applies a single analysis to a set of packages, not just one, as this is necessary for testing the "tests" Analyzer. The Run function also returns a richer Result for each package, allowing a test to perform additional checks if necessary. I really don't understand how Gerrit decides whether to render a file such as passes/tests/tests.go as a mv+edit or an add; small changes to the CL seem to perturb the heuristic. When reviewing these CLs please inspect the logical diff of passes/vet/tests.go -> passes/tests/tests.go Change-Id: I7812837278b20c8608ccbb6c709c675588a84db1 Reviewed-on: https://go-review.googlesource.com/c/140457 Reviewed-by: Michael Matloob Run-TryBot: Michael Matloob --- go/analysis/analysistest/analysistest.go | 61 +++--- go/analysis/internal/checker/checker.go | 42 ++-- go/analysis/passes/ctrlflow/ctrlflow_test.go | 10 +- go/analysis/passes/tests/testdata/src/a/a.go | 3 + .../passes/tests/testdata/src/a/a_test.go | 78 ++++++++ .../passes/tests/testdata/src/a/ax_test.go | 7 + go/analysis/passes/tests/tests.go | 171 ++++++++++++++++ go/analysis/passes/tests/tests_test.go | 14 ++ .../passes/vet/testdata/testingpkg/tests.go | 1 - .../vet/testdata/testingpkg/tests_test.go | 74 ------- go/analysis/passes/vet/tests.go | 189 ------------------ 11 files changed, 336 insertions(+), 314 deletions(-) create mode 100644 go/analysis/passes/tests/testdata/src/a/a.go create mode 100644 go/analysis/passes/tests/testdata/src/a/a_test.go create mode 100644 go/analysis/passes/tests/testdata/src/a/ax_test.go create mode 100644 go/analysis/passes/tests/tests.go create mode 100644 go/analysis/passes/tests/tests_test.go delete mode 100644 go/analysis/passes/vet/testdata/testingpkg/tests.go delete mode 100644 go/analysis/passes/vet/testdata/testingpkg/tests_test.go delete mode 100644 go/analysis/passes/vet/tests.go diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index 794461c2595..83e1a6af986 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -60,10 +60,11 @@ type Testing interface { Errorf(format string, args ...interface{}) } -// Run applies an analysis to the named package. -// It loads the package from the specified GOPATH-style project +// Run applies an analysis to the packages denoted by the "go list" patterns. +// +// It loads the packages from the specified GOPATH-style project // directory using golang.org/x/tools/go/packages, runs the analysis on -// it, and checks that each the analysis emits the expected diagnostics +// them, and checks that each analysis emits the expected diagnostics // and facts specified by the contents of '// want ...' comments in the // package's source files. // @@ -92,32 +93,36 @@ type Testing interface { // Unexpected diagnostics and facts, and unmatched expectations, are // reported as errors to the Testing. // -// You may wish to call this function from within a (*testing.T).Run -// subtest to ensure that errors have adequate contextual description. -// -// Run returns the pass and the result of the Analyzer's Run function, -// or (nil, nil) if loading or analysis failed. -func Run(t Testing, dir string, a *analysis.Analyzer, pkgname string) (*analysis.Pass, interface{}) { - pkg, err := loadPackage(dir, pkgname) +// Run reports an error to the Testing if loading or analysis failed. +// Run also returns a Result for each package for which analysis was +// attempted, even if unsuccessful. It is safe for a test to ignore all +// the results, but a test may use it to perform additional checks. +func Run(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result { + pkgs, err := loadPackages(dir, patterns...) if err != nil { - t.Errorf("loading %s: %v", pkgname, err) - return nil, nil + t.Errorf("loading %s: %v", patterns, err) + return nil } - pass, diagnostics, facts, result, err := checker.Analyze(pkg, a) - if err != nil { - t.Errorf("analyzing %s: %v", pkgname, err) - return nil, nil + results := checker.TestAnalyzer(a, pkgs) + for _, result := range results { + if result.Err != nil { + t.Errorf("error analyzing %s: %v", result.Pass, result.Err) + } else { + check(t, dir, result.Pass, result.Diagnostics, result.Facts) + } } - - check(t, dir, pass, diagnostics, facts) - - return pass, result + return results } -// loadPackage loads the specified package (from source, with -// dependencies) from dir, which is the root of a GOPATH-style project tree. -func loadPackage(dir, pkgpath string) (*packages.Package, error) { +// A Result holds the result of applying an analyzer to a package. +type Result = checker.TestAnalyzerResult + +// loadPackages uses go/packages to load a specified packages (from source, with +// dependencies) from dir, which is the root of a GOPATH-style project +// tree. It returns an error if any package had an error, or the pattern +// matched no packages. +func loadPackages(dir string, patterns ...string) ([]*packages.Package, error) { // packages.Load loads the real standard library, not a minimal // fake version, which would be more efficient, especially if we // have many small tests that import, say, net/http. @@ -131,7 +136,7 @@ func loadPackage(dir, pkgpath string) (*packages.Package, error) { Tests: true, Env: append(os.Environ(), "GOPATH="+dir, "GO111MODULE=off", "GOPROXY=off"), } - pkgs, err := packages.Load(cfg, pkgpath) + pkgs, err := packages.Load(cfg, patterns...) if err != nil { return nil, err } @@ -140,12 +145,10 @@ func loadPackage(dir, pkgpath string) (*packages.Package, error) { // some Analyzers may be disposed to RunDespiteErrors. packages.PrintErrors(pkgs) - if len(pkgs) != 1 { - return nil, fmt.Errorf("pattern %q expanded to %d packages, want 1", - pkgpath, len(pkgs)) + if len(pkgs) == 0 { + return nil, fmt.Errorf("no packages matched %s", patterns) } - - return pkgs[0], nil + return pkgs, nil } // check inspects an analysis pass on which the analysis has already diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index 656478df93f..a7c78269998 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -150,29 +150,39 @@ func load(patterns []string, allSyntax bool) ([]*packages.Package, error) { return initial, err } -// Analyze applies an analysis to a package (and their dependencies if -// necessary) and returns the graph of results. +// TestAnalyzer applies an analysis to a set of packages (and their +// dependencies if necessary) and returns the results. // // Facts about pkg are returned in a map keyed by object; package facts // have a nil key. // -// It is exposed for use in testing. -func Analyze(pkg *packages.Package, a *analysis.Analyzer) (*analysis.Pass, []analysis.Diagnostic, map[types.Object][]analysis.Fact, interface{}, error) { - act := analyze([]*packages.Package{pkg}, []*analysis.Analyzer{a})[0] - - facts := make(map[types.Object][]analysis.Fact) - for key, fact := range act.objectFacts { - if key.obj.Pkg() == pkg.Types { - facts[key.obj] = append(facts[key.obj], fact) +// This entry point is used only by analysistest. +func TestAnalyzer(a *analysis.Analyzer, pkgs []*packages.Package) []*TestAnalyzerResult { + var results []*TestAnalyzerResult + for _, act := range analyze(pkgs, []*analysis.Analyzer{a}) { + facts := make(map[types.Object][]analysis.Fact) + for key, fact := range act.objectFacts { + if key.obj.Pkg() == act.pass.Pkg { + facts[key.obj] = append(facts[key.obj], fact) + } } - } - for key, fact := range act.packageFacts { - if key.pkg == pkg.Types { - facts[nil] = append(facts[nil], fact) + for key, fact := range act.packageFacts { + if key.pkg == act.pass.Pkg { + facts[nil] = append(facts[nil], fact) + } } - } - return act.pass, act.diagnostics, facts, act.result, act.err + results = append(results, &TestAnalyzerResult{act.pass, act.diagnostics, facts, act.result, act.err}) + } + return results +} + +type TestAnalyzerResult struct { + Pass *analysis.Pass + Diagnostics []analysis.Diagnostic + Facts map[types.Object][]analysis.Fact + Result interface{} + Err error } func analyze(pkgs []*packages.Package, analyzers []*analysis.Analyzer) []*action { diff --git a/go/analysis/passes/ctrlflow/ctrlflow_test.go b/go/analysis/passes/ctrlflow/ctrlflow_test.go index 61e404552aa..658da9ff33b 100644 --- a/go/analysis/passes/ctrlflow/ctrlflow_test.go +++ b/go/analysis/passes/ctrlflow/ctrlflow_test.go @@ -12,18 +12,18 @@ func Test(t *testing.T) { testdata := analysistest.TestData() // load testdata/src/a/a.go - pass, result := analysistest.Run(t, testdata, ctrlflow.Analyzer, "a") + results := analysistest.Run(t, testdata, ctrlflow.Analyzer, "a") // Perform a minimal smoke test on // the result (CFG) computed by ctrlflow. - if result != nil { - cfgs := result.(*ctrlflow.CFGs) + for _, result := range results { + cfgs := result.Result.(*ctrlflow.CFGs) - for _, decl := range pass.Files[0].Decls { + for _, decl := range result.Pass.Files[0].Decls { if decl, ok := decl.(*ast.FuncDecl); ok && decl.Body != nil { if cfgs.FuncDecl(decl) == nil { t.Errorf("%s: no CFG for func %s", - pass.Fset.Position(decl.Pos()), decl.Name.Name) + result.Pass.Fset.Position(decl.Pos()), decl.Name.Name) } } } diff --git a/go/analysis/passes/tests/testdata/src/a/a.go b/go/analysis/passes/tests/testdata/src/a/a.go new file mode 100644 index 00000000000..1ff3964413a --- /dev/null +++ b/go/analysis/passes/tests/testdata/src/a/a.go @@ -0,0 +1,3 @@ +package a + +func Foo() {} diff --git a/go/analysis/passes/tests/testdata/src/a/a_test.go b/go/analysis/passes/tests/testdata/src/a/a_test.go new file mode 100644 index 00000000000..67bfda7a92f --- /dev/null +++ b/go/analysis/passes/tests/testdata/src/a/a_test.go @@ -0,0 +1,78 @@ +package a + +import ( + "testing" +) + +// Buf is a ... +type Buf []byte + +// Append ... +func (*Buf) Append([]byte) {} + +func (Buf) Reset() {} + +func (Buf) Len() int { return 0 } + +// DefaultBuf is a ... +var DefaultBuf Buf + +func Example() {} // OK because is package-level. + +func Example_goodSuffix() {} // OK because refers to suffix annotation. + +func Example_BadSuffix() {} // want "Example_BadSuffix has malformed example suffix: BadSuffix" + +func ExampleBuf() {} // OK because refers to known top-level type. + +func ExampleBuf_Append() {} // OK because refers to known method. + +func ExampleBuf_Clear() {} // want "ExampleBuf_Clear refers to unknown field or method: Buf.Clear" + +func ExampleBuf_suffix() {} // OK because refers to suffix annotation. + +func ExampleBuf_Append_Bad() {} // want "ExampleBuf_Append_Bad has malformed example suffix: Bad" + +func ExampleBuf_Append_suffix() {} // OK because refers to known method with valid suffix. + +func ExampleDefaultBuf() {} // OK because refers to top-level identifier. + +func ExampleBuf_Reset() bool { return true } // want "ExampleBuf_Reset should return nothing" + +func ExampleBuf_Len(i int) {} // want "ExampleBuf_Len should be niladic" + +// "Puffer" is German for "Buffer". + +func ExamplePuffer() {} // want "ExamplePuffer refers to unknown identifier: Puffer" + +func ExamplePuffer_Append() {} // want "ExamplePuffer_Append refers to unknown identifier: Puffer" + +func ExamplePuffer_suffix() {} // want "ExamplePuffer_suffix refers to unknown identifier: Puffer" + +func ExampleFoo() {} // OK because a.Foo exists + +func ExampleBar() {} // want "ExampleBar refers to unknown identifier: Bar" + +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) {} // want "first letter after 'Test' must not be lowercase" + +func TestemptyImportBadSuffix(*testing.T) {} // want "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) {} // want "first letter after 'Benchmark' must not be lowercase" diff --git a/go/analysis/passes/tests/testdata/src/a/ax_test.go b/go/analysis/passes/tests/testdata/src/a/ax_test.go new file mode 100644 index 00000000000..982e6a10cec --- /dev/null +++ b/go/analysis/passes/tests/testdata/src/a/ax_test.go @@ -0,0 +1,7 @@ +package a_test + +import _ "a" + +func ExampleFoo() {} // OK because a.Foo exists + +func ExampleBar() {} // want "ExampleBar refers to unknown identifier: Bar" diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go new file mode 100644 index 00000000000..fca9854c749 --- /dev/null +++ b/go/analysis/passes/tests/tests.go @@ -0,0 +1,171 @@ +// 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 tests + +import ( + "go/ast" + "go/types" + "strings" + "unicode" + "unicode/utf8" + + "golang.org/x/tools/go/analysis" +) + +var Analyzer = &analysis.Analyzer{ + Name: "tests", + Doc: `check for common mistaken usages of tests and examples + +The tests checker walks Test, Benchmark and Example functions checking +malformed names, wrong signatures and examples documenting non-existent +identifiers.`, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, f := range pass.Files { + if !strings.HasSuffix(pass.Fset.File(f.Pos()).Name(), "_test.go") { + continue + } + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv != nil { + // Ignore non-functions or functions with receivers. + continue + } + + switch { + case strings.HasPrefix(fn.Name.Name, "Example"): + checkExample(pass, fn) + case strings.HasPrefix(fn.Name.Name, "Test"): + checkTest(pass, fn, "Test") + case strings.HasPrefix(fn.Name.Name, "Benchmark"): + checkTest(pass, fn, "Benchmark") + } + } + } + return nil, nil +} + +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(pkg *types.Package, name string) types.Object { + if o := pkg.Scope().Lookup(name); o != nil { + return o + } + + // If this package is ".../foo_test" and it imports a package + // ".../foo", try looking in the latter package. + // This heuristic should work even on build systems that do not + // record any special link between the packages. + if basePath := strings.TrimSuffix(pkg.Path(), "_test"); basePath != pkg.Path() { + for _, imp := range pkg.Imports() { + if imp.Path() == basePath { + return imp.Scope().Lookup(name) + } + } + } + return nil +} + +func checkExample(pass *analysis.Pass, fn *ast.FuncDecl) { + fnName := fn.Name.Name + if params := fn.Type.Params; len(params.List) != 0 { + pass.Reportf(fn.Pos(), "%s should be niladic", fnName) + } + if results := fn.Type.Results; results != nil && len(results.List) != 0 { + pass.Reportf(fn.Pos(), "%s should return nothing", fnName) + } + + if fnName == "Example" { + // Nothing more to do. + return + } + + var ( + exName = strings.TrimPrefix(fnName, "Example") + elems = strings.SplitN(exName, "_", 3) + ident = elems[0] + obj = lookup(pass.Pkg, ident) + ) + if ident != "" && obj == nil { + // Check ExampleFoo and ExampleBadFoo. + pass.Reportf(fn.Pos(), "%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) { + pass.Reportf(fn.Pos(), "%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 { + pass.Reportf(fn.Pos(), "%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. + pass.Reportf(fn.Pos(), "%s has malformed example suffix: %s", fnName, elems[2]) + } +} + +func checkTest(pass *analysis.Pass, fn *ast.FuncDecl, prefix string) { + // 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):]) { + pass.Reportf(fn.Pos(), "%s has malformed name: first letter after '%s' must not be lowercase", fn.Name.Name, prefix) + } +} diff --git a/go/analysis/passes/tests/tests_test.go b/go/analysis/passes/tests/tests_test.go new file mode 100644 index 00000000000..6d42a1a9e83 --- /dev/null +++ b/go/analysis/passes/tests/tests_test.go @@ -0,0 +1,14 @@ +package tests_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/tests" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + // Loads "a", "a [a.test]", and "a.test". + analysistest.Run(t, testdata, tests.Analyzer, "a") +} diff --git a/go/analysis/passes/vet/testdata/testingpkg/tests.go b/go/analysis/passes/vet/testdata/testingpkg/tests.go deleted file mode 100644 index 69d29d3c6c6..00000000000 --- a/go/analysis/passes/vet/testdata/testingpkg/tests.go +++ /dev/null @@ -1 +0,0 @@ -package testdata diff --git a/go/analysis/passes/vet/testdata/testingpkg/tests_test.go b/go/analysis/passes/vet/testdata/testingpkg/tests_test.go deleted file mode 100644 index f5bbc3922a9..00000000000 --- a/go/analysis/passes/vet/testdata/testingpkg/tests_test.go +++ /dev/null @@ -1,74 +0,0 @@ -package testdata - -import ( - "testing" -) - -// Buf is a ... -type Buf []byte - -// Append ... -func (*Buf) Append([]byte) {} - -func (Buf) Reset() {} - -func (Buf) Len() int { return 0 } - -// DefaultBuf is a ... -var DefaultBuf Buf - -func Example() {} // OK because is package-level. - -func Example_goodSuffix() // OK because refers to suffix annotation. - -func Example_BadSuffix() // ERROR "Example_BadSuffix has malformed example suffix: BadSuffix" - -func ExampleBuf() // OK because refers to known top-level type. - -func ExampleBuf_Append() {} // OK because refers to known method. - -func ExampleBuf_Clear() {} // ERROR "ExampleBuf_Clear refers to unknown field or method: Buf.Clear" - -func ExampleBuf_suffix() {} // OK because refers to suffix annotation. - -func ExampleBuf_Append_Bad() {} // ERROR "ExampleBuf_Append_Bad has malformed example suffix: Bad" - -func ExampleBuf_Append_suffix() {} // OK because refers to known method with valid suffix. - -func ExampleDefaultBuf() {} // OK because refers to top-level identifier. - -func ExampleBuf_Reset() bool { return true } // ERROR "ExampleBuf_Reset should return nothing" - -func ExampleBuf_Len(i int) {} // ERROR "ExampleBuf_Len should be niladic" - -// "Puffer" is German for "Buffer". - -func ExamplePuffer() // ERROR "ExamplePuffer 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 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/go/analysis/passes/vet/tests.go b/go/analysis/passes/vet/tests.go deleted file mode 100644 index 131d3e287df..00000000000 --- a/go/analysis/passes/vet/tests.go +++ /dev/null @@ -1,189 +0,0 @@ -// +build ignore - -// 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(f *File) []*types.Scope { - scopes := []*types.Scope{f.pkg.typesPkg.Scope()} - if f.basePkg != nil { - scopes = append(scopes, f.basePkg.typesPkg.Scope()) - } else { - // If basePkg is not specified (e.g. when checking a single file) try to - // find it among imports. - pkgName := f.pkg.typesPkg.Name() - if strings.HasSuffix(pkgName, "_test") { - basePkgName := strings.TrimSuffix(pkgName, "_test") - for _, p := range f.pkg.typesPkg.Imports() { - if p.Name() == basePkgName { - scopes = append(scopes, p.Scope()) - break - } - } - } - } - return scopes -} - -func checkExample(fn *ast.FuncDecl, f *File, 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(f)) - ) - 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 nonexistent -// 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, report) - case strings.HasPrefix(fn.Name.Name, "Test"): - checkTest(fn, "Test", report) - case strings.HasPrefix(fn.Name.Name, "Benchmark"): - checkTest(fn, "Benchmark", report) - } -}