| 1 | // Copyright 2020 The Go Authors. All rights reserved. |
|---|---|
| 2 | // Use of this source code is governed by a BSD-style |
| 3 | // license that can be found in the LICENSE file. |
| 4 | |
| 5 | package testinggoroutine |
| 6 | |
| 7 | import ( |
| 8 | "go/ast" |
| 9 | |
| 10 | "golang.org/x/tools/go/analysis" |
| 11 | "golang.org/x/tools/go/analysis/passes/inspect" |
| 12 | "golang.org/x/tools/go/analysis/passes/internal/analysisutil" |
| 13 | "golang.org/x/tools/go/ast/inspector" |
| 14 | "golang.org/x/tools/internal/typeparams" |
| 15 | ) |
| 16 | |
| 17 | const Doc = `report calls to (*testing.T).Fatal from goroutines started by a test. |
| 18 | |
| 19 | Functions that abruptly terminate a test, such as the Fatal, Fatalf, FailNow, and |
| 20 | Skip{,f,Now} methods of *testing.T, must be called from the test goroutine itself. |
| 21 | This checker detects calls to these functions that occur within a goroutine |
| 22 | started by the test. For example: |
| 23 | |
| 24 | func TestFoo(t *testing.T) { |
| 25 | go func() { |
| 26 | t.Fatal("oops") // error: (*T).Fatal called from non-test goroutine |
| 27 | }() |
| 28 | } |
| 29 | ` |
| 30 | |
| 31 | var Analyzer = &analysis.Analyzer{ |
| 32 | Name: "testinggoroutine", |
| 33 | Doc: Doc, |
| 34 | Requires: []*analysis.Analyzer{inspect.Analyzer}, |
| 35 | Run: run, |
| 36 | } |
| 37 | |
| 38 | var forbidden = map[string]bool{ |
| 39 | "FailNow": true, |
| 40 | "Fatal": true, |
| 41 | "Fatalf": true, |
| 42 | "Skip": true, |
| 43 | "Skipf": true, |
| 44 | "SkipNow": true, |
| 45 | } |
| 46 | |
| 47 | func run(pass *analysis.Pass) (interface{}, error) { |
| 48 | inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) |
| 49 | |
| 50 | if !analysisutil.Imports(pass.Pkg, "testing") { |
| 51 | return nil, nil |
| 52 | } |
| 53 | |
| 54 | // Filter out anything that isn't a function declaration. |
| 55 | onlyFuncs := []ast.Node{ |
| 56 | (*ast.FuncDecl)(nil), |
| 57 | } |
| 58 | |
| 59 | inspect.Nodes(onlyFuncs, func(node ast.Node, push bool) bool { |
| 60 | fnDecl, ok := node.(*ast.FuncDecl) |
| 61 | if !ok { |
| 62 | return false |
| 63 | } |
| 64 | |
| 65 | if !hasBenchmarkOrTestParams(fnDecl) { |
| 66 | return false |
| 67 | } |
| 68 | |
| 69 | // Now traverse the benchmark/test's body and check that none of the |
| 70 | // forbidden methods are invoked in the goroutines within the body. |
| 71 | ast.Inspect(fnDecl, func(n ast.Node) bool { |
| 72 | goStmt, ok := n.(*ast.GoStmt) |
| 73 | if !ok { |
| 74 | return true |
| 75 | } |
| 76 | |
| 77 | checkGoStmt(pass, goStmt) |
| 78 | |
| 79 | // No need to further traverse the GoStmt since right |
| 80 | // above we manually traversed it in the ast.Inspect(goStmt, ...) |
| 81 | return false |
| 82 | }) |
| 83 | |
| 84 | return false |
| 85 | }) |
| 86 | |
| 87 | return nil, nil |
| 88 | } |
| 89 | |
| 90 | func hasBenchmarkOrTestParams(fnDecl *ast.FuncDecl) bool { |
| 91 | // Check that the function's arguments include "*testing.T" or "*testing.B". |
| 92 | params := fnDecl.Type.Params.List |
| 93 | |
| 94 | for _, param := range params { |
| 95 | if _, ok := typeIsTestingDotTOrB(param.Type); ok { |
| 96 | return true |
| 97 | } |
| 98 | } |
| 99 | |
| 100 | return false |
| 101 | } |
| 102 | |
| 103 | func typeIsTestingDotTOrB(expr ast.Expr) (string, bool) { |
| 104 | starExpr, ok := expr.(*ast.StarExpr) |
| 105 | if !ok { |
| 106 | return "", false |
| 107 | } |
| 108 | selExpr, ok := starExpr.X.(*ast.SelectorExpr) |
| 109 | if !ok { |
| 110 | return "", false |
| 111 | } |
| 112 | |
| 113 | varPkg := selExpr.X.(*ast.Ident) |
| 114 | if varPkg.Name != "testing" { |
| 115 | return "", false |
| 116 | } |
| 117 | |
| 118 | varTypeName := selExpr.Sel.Name |
| 119 | ok = varTypeName == "B" || varTypeName == "T" |
| 120 | return varTypeName, ok |
| 121 | } |
| 122 | |
| 123 | // goStmtFunc returns the ast.Node of a call expression |
| 124 | // that was invoked as a go statement. Currently, only |
| 125 | // function literals declared in the same function, and |
| 126 | // static calls within the same package are supported. |
| 127 | func goStmtFun(goStmt *ast.GoStmt) ast.Node { |
| 128 | switch fun := goStmt.Call.Fun.(type) { |
| 129 | case *ast.IndexExpr, *typeparams.IndexListExpr: |
| 130 | x, _, _, _ := typeparams.UnpackIndexExpr(fun) |
| 131 | id, _ := x.(*ast.Ident) |
| 132 | if id == nil { |
| 133 | break |
| 134 | } |
| 135 | if id.Obj == nil { |
| 136 | break |
| 137 | } |
| 138 | if funDecl, ok := id.Obj.Decl.(ast.Node); ok { |
| 139 | return funDecl |
| 140 | } |
| 141 | case *ast.Ident: |
| 142 | // TODO(cuonglm): improve this once golang/go#48141 resolved. |
| 143 | if fun.Obj == nil { |
| 144 | break |
| 145 | } |
| 146 | if funDecl, ok := fun.Obj.Decl.(ast.Node); ok { |
| 147 | return funDecl |
| 148 | } |
| 149 | case *ast.FuncLit: |
| 150 | return goStmt.Call.Fun |
| 151 | } |
| 152 | return goStmt.Call |
| 153 | } |
| 154 | |
| 155 | // checkGoStmt traverses the goroutine and checks for the |
| 156 | // use of the forbidden *testing.(B, T) methods. |
| 157 | func checkGoStmt(pass *analysis.Pass, goStmt *ast.GoStmt) { |
| 158 | fn := goStmtFun(goStmt) |
| 159 | // Otherwise examine the goroutine to check for the forbidden methods. |
| 160 | ast.Inspect(fn, func(n ast.Node) bool { |
| 161 | selExpr, ok := n.(*ast.SelectorExpr) |
| 162 | if !ok { |
| 163 | return true |
| 164 | } |
| 165 | |
| 166 | _, bad := forbidden[selExpr.Sel.Name] |
| 167 | if !bad { |
| 168 | return true |
| 169 | } |
| 170 | |
| 171 | // Now filter out false positives by the import-path/type. |
| 172 | ident, ok := selExpr.X.(*ast.Ident) |
| 173 | if !ok { |
| 174 | return true |
| 175 | } |
| 176 | if ident.Obj == nil || ident.Obj.Decl == nil { |
| 177 | return true |
| 178 | } |
| 179 | field, ok := ident.Obj.Decl.(*ast.Field) |
| 180 | if !ok { |
| 181 | return true |
| 182 | } |
| 183 | if typeName, ok := typeIsTestingDotTOrB(field.Type); ok { |
| 184 | var fnRange analysis.Range = goStmt |
| 185 | if _, ok := fn.(*ast.FuncLit); ok { |
| 186 | fnRange = selExpr |
| 187 | } |
| 188 | pass.ReportRangef(fnRange, "call to (*%s).%s from a non-test goroutine", typeName, selExpr.Sel) |
| 189 | } |
| 190 | return true |
| 191 | }) |
| 192 | } |
| 193 |
Members