go/analysis/passes/test: remove some false positives

Alternative build systems like blaze don't always provide a way to determine
the relationship between a package and the package it's testing. This means
that sometimes the check for misspelled Example function names over-reports
because it doesn't find the object being exemplified. Don't report errors
unless a object can't be found in any of the imports. This means that there
won't be any false positives though of course this comes at the cost of
false positives.

Change-Id: I7435eeb2333b6dd72e06bb6383fff2ac17bee845
Reviewed-on: https://go-review.googlesource.com/c/tools/+/168404
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This commit is contained in:
Michael Matloob 2019-03-20 14:41:40 -04:00
parent c5e06eb4cd
commit db798565ff
4 changed files with 51 additions and 17 deletions

View File

@ -0,0 +1,8 @@
package b
type Foo struct {
}
func (f *Foo) F() {
}

View File

@ -0,0 +1,16 @@
package b_x_test
import "b"
func ExampleFoo_F() {
var x b.Foo
x.F()
}
func ExampleFoo_G() { // want "ExampleFoo_G refers to unknown field or method: Foo.G"
}
func ExampleBar_F() { // want "ExampleBar_F refers to unknown identifier: Bar"
}

View File

@ -84,23 +84,25 @@ func isTestParam(typ ast.Expr, wantType string) bool {
return false return false
} }
func lookup(pkg *types.Package, name string) types.Object { func lookup(pkg *types.Package, name string) []types.Object {
if o := pkg.Scope().Lookup(name); o != nil { if o := pkg.Scope().Lookup(name); o != nil {
return o return []types.Object{o}
} }
// If this package is ".../foo_test" and it imports a package var ret []types.Object
// ".../foo", try looking in the latter package. // Search through the imports to see if any of them define name.
// This heuristic should work even on build systems that do not // It's hard to tell in general which package is being tested, so
// record any special link between the packages. // for the purposes of the analysis, allow the object to appear
if basePath := strings.TrimSuffix(pkg.Path(), "_test"); basePath != pkg.Path() { // in any of the imports. This guarantees there are no false positives
for _, imp := range pkg.Imports() { // because the example needs to use the object so it must be defined
if imp.Path() == basePath { // in the package or one if its imports. On the other hand, false
return imp.Scope().Lookup(name) // negatives are possible, but should be rare.
} for _, imp := range pkg.Imports() {
if obj := imp.Scope().Lookup(name); obj != nil {
ret = append(ret, obj)
} }
} }
return nil return ret
} }
func checkExample(pass *analysis.Pass, fn *ast.FuncDecl) { func checkExample(pass *analysis.Pass, fn *ast.FuncDecl) {
@ -121,9 +123,9 @@ func checkExample(pass *analysis.Pass, fn *ast.FuncDecl) {
exName = strings.TrimPrefix(fnName, "Example") exName = strings.TrimPrefix(fnName, "Example")
elems = strings.SplitN(exName, "_", 3) elems = strings.SplitN(exName, "_", 3)
ident = elems[0] ident = elems[0]
obj = lookup(pass.Pkg, ident) objs = lookup(pass.Pkg, ident)
) )
if ident != "" && obj == nil { if ident != "" && len(objs) == 0 {
// Check ExampleFoo and ExampleBadFoo. // Check ExampleFoo and ExampleBadFoo.
pass.Reportf(fn.Pos(), "%s refers to unknown identifier: %s", fnName, ident) pass.Reportf(fn.Pos(), "%s refers to unknown identifier: %s", fnName, ident)
// Abort since obj is absent and no subsequent checks can be performed. // Abort since obj is absent and no subsequent checks can be performed.
@ -145,8 +147,15 @@ func checkExample(pass *analysis.Pass, fn *ast.FuncDecl) {
mmbr := elems[1] mmbr := elems[1]
if !isExampleSuffix(mmbr) { if !isExampleSuffix(mmbr) {
// Check ExampleFoo_Method and ExampleFoo_BadMethod. // Check ExampleFoo_Method and ExampleFoo_BadMethod.
if obj, _, _ := types.LookupFieldOrMethod(obj.Type(), true, obj.Pkg(), mmbr); obj == nil { found := false
pass.Reportf(fn.Pos(), "%s refers to unknown field or method: %s.%s", fnName, ident, mmbr) // Check if Foo.Method exists in this package or its imports.
for _, obj := range objs {
if obj, _, _ := types.LookupFieldOrMethod(obj.Type(), true, obj.Pkg(), mmbr); obj != nil {
found = true
}
if !found {
pass.Reportf(fn.Pos(), "%s refers to unknown field or method: %s.%s", fnName, ident, mmbr)
}
} }
} }
if len(elems) == 3 && !isExampleSuffix(elems[2]) { if len(elems) == 3 && !isExampleSuffix(elems[2]) {

View File

@ -15,7 +15,8 @@ func Test(t *testing.T) {
testdata := analysistest.TestData() testdata := analysistest.TestData()
analysistest.Run(t, testdata, tests.Analyzer, analysistest.Run(t, testdata, tests.Analyzer,
"a", // loads "a", "a [a.test]", and "a.test" "a", // loads "a", "a [a.test]", and "a.test"
"b_x_test", // loads "b" and "b_x_test"
"divergent", "divergent",
) )
} }