[tools] go/analysis: testinggoroutine inspects defined/anonymous functions
Cuong Manh Le (Gerrit)
Cuong Manh Le has uploaded this change for review.
go/analysis: testinggoroutine inspects defined/anonymous functions
Currently, testinggoroutine only performs inspection for functions
literal "go func() {}".
For defined functions or anonymous functions, like "go f(t)", it does
not traverse the function body, thus not reporting error, if any, for
those kind of functions.
To fix this, when seeing defined/anonymous functions, we should get the
function definition node then inspect it.
Fixes golang/go#47470
Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
---
M go/analysis/passes/testinggoroutine/testdata/src/a/a.go
M go/analysis/passes/testinggoroutine/testinggoroutine.go
2 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/go/analysis/passes/testinggoroutine/testdata/src/a/a.go b/go/analysis/passes/testinggoroutine/testdata/src/a/a.go
index 5fe9041..7fd2059 100644
--- a/go/analysis/passes/testinggoroutine/testdata/src/a/a.go
+++ b/go/analysis/passes/testinggoroutine/testdata/src/a/a.go
@@ -36,19 +36,6 @@
}
}
-func BenchmarkBadFatalf(b *testing.B) {
- var wg sync.WaitGroup
- defer wg.Wait()
-
- for i := 0; i < b.N; i++ {
- wg.Add(1)
- go func(id int) {
- defer wg.Done()
- b.Fatalf("TestFailed: id = %v\n", id) // want "call to .+B.+Fatalf from a non-test goroutine"
- }(i)
- }
-}
-
func TestBadFatal(t *testing.T) {
var wg sync.WaitGroup
defer wg.Wait()
@@ -62,6 +49,32 @@
}
}
+func f(t *testing.T) {
+ t.Fatal("TestFailed")
+}
+
+func TestBadFatalIssue47470(t *testing.T) {
+ go f(t) // want "call to .+T.+Fatal from a non-test goroutine"
+
+ g := func(t *testing.T) {
+ t.Fatal("TestFailed")
+ }
+ go g(t) // want "call to .+T.+Fatal from a non-test goroutine"
+}
+
+func BenchmarkBadFatalf(b *testing.B) {
+ var wg sync.WaitGroup
+ defer wg.Wait()
+
+ for i := 0; i < b.N; i++ {
+ wg.Add(1)
+ go func(id int) {
+ defer wg.Done()
+ b.Fatalf("TestFailed: id = %v\n", id) // want "call to .+B.+Fatalf from a non-test goroutine"
+ }(i)
+ }
+}
+
func BenchmarkBadFatal(b *testing.B) {
var wg sync.WaitGroup
defer wg.Wait()
diff --git a/go/analysis/passes/testinggoroutine/testinggoroutine.go b/go/analysis/passes/testinggoroutine/testinggoroutine.go
index d2b9a56..ff3148b 100644
--- a/go/analysis/passes/testinggoroutine/testinggoroutine.go
+++ b/go/analysis/passes/testinggoroutine/testinggoroutine.go
@@ -122,8 +122,16 @@
// checkGoStmt traverses the goroutine and checks for the
// use of the forbidden *testing.(B, T) methods.
func checkGoStmt(pass *analysis.Pass, goStmt *ast.GoStmt) {
+ var fun ast.Node = goStmt.Call
+ switch goStmt.Call.Fun.(type) {
+ case *ast.Ident:
+ id := goStmt.Call.Fun.(*ast.Ident)
+ fun = id.Obj.Decl.(ast.Node)
+ case *ast.FuncLit:
+ fun = goStmt.Call.Fun
+ }
// Otherwise examine the goroutine to check for the forbidden methods.
- ast.Inspect(goStmt, func(n ast.Node) bool {
+ ast.Inspect(fun, func(n ast.Node) bool {
selExpr, ok := n.(*ast.SelectorExpr)
if !ok {
return true
@@ -147,7 +155,12 @@
return true
}
if typeName, ok := typeIsTestingDotTOrB(field.Type); ok {
- pass.ReportRangef(selExpr, "call to (*%s).%s from a non-test goroutine", typeName, selExpr.Sel)
+ if _, ok := fun.(*ast.FuncLit); ok {
+ pass.ReportRangef(selExpr, "call to (*%s).%s from a non-test goroutine", typeName, selExpr.Sel)
+ } else {
+ pass.ReportRangef(goStmt, "call to (*%s).%s from a non-test goroutine", typeName, selExpr.Sel)
+ }
+
}
return true
})
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Emmanuel Odeke, Michael Matloob.
Patch set 1:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Emmanuel Odeke, Michael Matloob.
Cuong Manh Le uploaded patch set #2 to this change.
go/analysis: testinggoroutine inspects defined/anonymous functions
Currently, testinggoroutine only performs inspection for functions
literal "go func() {}".
For defined functions or anonymous functions, like "go f(t)", it does
not traverse the function body, thus not reporting error, if any, for
those kind of functions.
To fix this, when seeing defined/anonymous functions, we should get the
function definition node then inspect it.
Fixes golang/go#47470
Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
---
M go/analysis/passes/testinggoroutine/testdata/src/a/a.go
M go/analysis/passes/testinggoroutine/testinggoroutine.go
2 files changed, 41 insertions(+), 15 deletions(-)
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Emmanuel Odeke, Michael Matloob.
Patch set 2:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Emmanuel Odeke, Michael Matloob.
Cuong Manh Le uploaded patch set #3 to this change.
go/analysis: testinggoroutine inspects defined/anonymous functions
Currently, testinggoroutine only performs inspection for functions
literal "go func() {}".
For defined functions or anonymous functions, like "go f(t)", it does
not traverse the function body, thus not reporting error, if any, for
those kind of functions.
To fix this, when seeing defined/anonymous functions, we should get the
function definition node then inspect it.
Fixes golang/go#47470
Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
---
M go/analysis/passes/testinggoroutine/testdata/src/a/a.go
M go/analysis/passes/testinggoroutine/testinggoroutine.go
2 files changed, 40 insertions(+), 15 deletions(-)
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Emmanuel Odeke, Michael Matloob.
Patch set 3:Run-TryBot +1
kokoro (Gerrit)
Attention is currently required from: Rebecca Stambler, Emmanuel Odeke, Michael Matloob.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/6bb6a549-77f8-4c10-a5d7-06f40eeb0585
Patch set 2:gopls-CI +1
kokoro (Gerrit)
Attention is currently required from: Rebecca Stambler, Emmanuel Odeke, Michael Matloob.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/b2c817b1-8883-4483-a79a-36d5867211d1
Patch set 3:gopls-CI +1
Tim King (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le, Emmanuel Odeke, Michael Matloob.
2 comments:
File go/analysis/passes/testinggoroutine/testinggoroutine.go:
Patch Set #3, Line 125:
var fun ast.Node = goStmt.CallSuggestion: have a function that returns fun from goStmt. This will allow us a place to document what is currently supported.
We do not want to support arbitrary callgraph analysis here. Extending this from function literals declared in the same function to static calls within the same package is fine. We should document these limitations. We can maybe do a tiny bit more (method values with similar constraints?) but if so we should be deliberate.
id := goStmt.Call.Fun.(*ast.Ident)
fun = id.Obj.Decl.(ast.Node)Can these casts ever fail? If so we need to back out cleanly.
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le, Emmanuel Odeke, Michael Matloob.
Cuong Manh Le uploaded patch set #4 to this change.
go/analysis: testinggoroutine inspects defined/anonymous functions
Currently, testinggoroutine only performs inspection for functions
literal "go func() {}".
For defined functions or anonymous functions, like "go f(t)", it does
not traverse the function body, thus not reporting error, if any, for
those kind of functions.
To fix this, when seeing defined/anonymous functions, we should get the
function definition node then inspect it.
Fixes golang/go#47470
Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
---
M go/analysis/passes/testinggoroutine/testdata/src/a/a.go
M go/analysis/passes/testinggoroutine/testinggoroutine.go
2 files changed, 51 insertions(+), 15 deletions(-)
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King, Emmanuel Odeke, Michael Matloob.
Patch set 4:Run-TryBot +1
2 comments:
File go/analysis/passes/testinggoroutine/testinggoroutine.go:
Patch Set #3, Line 125:
var fun ast.Node = goStmt.Call
Suggestion: have a function that returns fun from goStmt. […]
I think using method values, which has *testing.T as parameter seems rare in practice, we can add support for it later, if any users raised the bar. My motivation to add support for static calls within the same package because I saw this problem in my friends project, and go vet do not catch this, but staticcheck does.
id := goStmt.Call.Fun.(*ast.Ident)
fun = id.Obj.Decl.(ast.Node)Can these casts ever fail? If so we need to back out cleanly.
Done
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King, Emmanuel Odeke.
Cuong Manh Le removed Michael Matloob from this change.
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King, Emmanuel Odeke.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/a81e4619-b0e3-4eb5-8d3c-c9e8c1a68ce3
Patch set 4:gopls-CI +1
Tim King (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le, Emmanuel Odeke.
3 comments:
File go/analysis/passes/testinggoroutine/testdata/src/a/a.go:
Patch Set #4, Line 57:
// want "call to .+T.+Fatal from a non-test goroutine"What is the full error message here?
At the moment, I am wondering whether this message is sufficiently clear? This is now somewhat distant from "f". Maybe the message needs clarification? Maybe something needs to mention "f"? Would that be too long?
Very curious on other reviewers' thoughts on this.
File go/analysis/passes/testinggoroutine/testinggoroutine.go:
Patch Set #4, Line 132:
fun = funDeclnit: `return funDecl`
This allows for removing a stateful variable fun. You can then use other types or shorter variables if this helps.
Patch Set #4, Line 135:
goStmt.Call.Funnit: return goStmt.Call.Fun
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le, Emmanuel Odeke.
Cuong Manh Le uploaded patch set #5 to this change.
go/analysis: testinggoroutine inspects defined/anonymous functions
Currently, testinggoroutine only performs inspection for functions
literal "go func() {}".
For defined functions or anonymous functions, like "go f(t)", it does
not traverse the function body, thus not reporting error, if any, for
those kind of functions.
To fix this, when seeing defined/anonymous functions, we should get the
function definition node then inspect it.
Fixes golang/go#47470
Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
---
M go/analysis/passes/testinggoroutine/testdata/src/a/a.go
M go/analysis/passes/testinggoroutine/testinggoroutine.go
2 files changed, 50 insertions(+), 15 deletions(-)
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King, Emmanuel Odeke.
Patch set 5:Run-TryBot +1
3 comments:
File go/analysis/passes/testinggoroutine/testdata/src/a/a.go:
Patch Set #4, Line 57:
// want "call to .+T.+Fatal from a non-test goroutine"What is the full error message here?
It's "call to (*T).Fatalf from a non-test goroutine"
Maybe we can just do: "call to (*T).Fatalf from a non-test goroutine f"
File go/analysis/passes/testinggoroutine/testinggoroutine.go:
Patch Set #4, Line 132:
fun = funDecl
nit: `return funDecl` […]
Done
Patch Set #4, Line 135:
goStmt.Call.Fun
nit: return goStmt.Call. […]
Done
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King, Emmanuel Odeke.
1 comment:
File go/analysis/passes/testinggoroutine/testdata/src/a/a.go:
Patch Set #4, Line 57:
// want "call to .+T.+Fatal from a non-test goroutine"
> What is the full error message here? […]
FYI, staticcheck also does not mention "f" in the message, but it does include the stack trace to t.Fatal call:
```
testdata/src/a/a.go:57:2: the goroutine calls T.Fatal, which must be called in the same goroutine as the test (SA2002)
testdata/src/a/a.go:53:2: call to T.Fatal
```
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King, Emmanuel Odeke.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/c2f8dbb2-a7a4-4ca5-9118-ed475df5377f
Patch set 5:gopls-CI +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King, Emmanuel Odeke.
1 comment:
Patchset:
a gentle ping @odeke-em 😊
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le, Tim King.
Patch set 5:Run-TryBot +1Code-Review +2
10 comments:
Commit Message:
Patch Set #5, Line 7:
go/analysis: testinggoroutine inspects defined/anonymous functionsgo/analysis/passes/testinggoroutine: also inspect defined/anonymous functions
performs inspection for functions
literal "go func() {}".
inspects function literals invoked as "go func() {}"
For defined functions or anonymous functions, like "go f(t)", it does
not traverse the function body, thus not reporting error, if any, for
those kind of functions.
To fix this, when seeing defined/anonymous functions, we should get the
function definition node then inspect it.
For defined or anonymous functions like "go f(t)" it didn't
traverse the function body. To fix this, on encountering those
kinds of functions, retrieve the definition node then inspect.
Patchset:
Thank you for this change Cuong, and thank you for the review Tim! LGTM, Cuong with some suggestions, PTAL.
File go/analysis/passes/testinggoroutine/testdata/src/a/a.go:
Patch Set #4, Line 57:
// want "call to .+T.+Fatal from a non-test goroutine"
FYI, staticcheck also does not mention "f" in the message, but it does include the stack trace to t. […]
I am also inclined to leave it as it, line numbers IMHO are sufficient, plus we explain the entire context of the issue is explained.
File go/analysis/passes/testinggoroutine/testinggoroutine.go:
goStmtFun returns function declaration node of the
// call expression in go statement.// goStmtFunc returns the ast.Node of a call expression
// that was invoked as a go statement.}
return goStmt.CallMinor nit, please use default:
fn
fn
Patch Set #5, Line 168:
var rng analysis.Range = goStmtvar fnRange analysis.Range = goStmt
rng usually stands for "Random Number Generator"
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le, Tim King.
Cuong Manh Le uploaded patch set #6 to this change.
go/analysis/passes/testinggoroutine: also inspect defined/anonymous functions
Currently, testinggoroutine only inspects functions literal invoked as
"go func(){ ... }()".
For defined or anonymous functions like "go f(t)" it didn't traverse the
function body. To fix this, on encountering those kinds of functions,
retrieve the definition node then inspect it.
Fixes golang/go#47470
Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
---
M go/analysis/passes/testinggoroutine/testdata/src/a/a.go
M go/analysis/passes/testinggoroutine/testinggoroutine.go
2 files changed, 50 insertions(+), 15 deletions(-)
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King.
Patch set 6:Run-TryBot +1
8 comments:
Commit Message:
Patch Set #5, Line 7:
go/analysis: testinggoroutine inspects defined/anonymous functions
go/analysis/passes/testinggoroutine: also inspect defined/anonymous functions
Done
performs inspection for functions
literal "go func() {}".inspects function literals invoked as "go func() {}"
Done
For defined functions or anonymous functions, like "go f(t)", it does
not traverse the function body, thus not reporting error, if any, for
those kind of functions.
To fix this, when seeing defined/anonymous functions, we should get the
function definition node then inspect it.
For defined or anonymous functions like "go f(t)" it didn't […]
Done
File go/analysis/passes/testinggoroutine/testinggoroutine.go:
goStmtFun returns function declaration node of the
// call expression in go statement.
// goStmtFunc returns the ast.Node of a call expression […]
Done
}
return goStmt.CallMinor nit, please use default:
Use default, you still have to add the final return outside of the switch-case, because we don't always return in case *ast.Ident
fn
Done
fn
Done
Patch Set #5, Line 168:
var rng analysis.Range = goStmt
var fnRange analysis.Range = goStmt […]
Done
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
kokoro (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/8ae160ba-3349-4760-8847-73a5bdd917f2
Patch set 6:gopls-CI +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler, Tim King.
1 comment:
Patchset:
Hi Tim, a gently ping for final review.
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Tim King (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le.
2 comments:
Patchset:
Hi Tim, a gently ping for final review.
Code looks fine. I am going to recommend waiting to submit this though. We are under the Go 1.17 code freeze. The release is expected to happen next week (https://groups.google.com/g/golang-dev/c/O3ae8bhs0ZU/m/A1fTnom5AQAJ). There is a chance someone updates go/tools in the vendor directories of the release. That would pull this change in.
I think we can wait for a week or two. How does that sound?
File go/analysis/passes/testinggoroutine/testdata/src/a/a.go:
Patch Set #4, Line 57:
// want "call to .+T.+Fatal from a non-test goroutine"
I am also inclined to leave it as it, line numbers IMHO are sufficient, plus we explain the entire c […]
SG. The message is plausibly clear enough from the context. If it is not, we can improve it later.
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Patchset:
Code looks fine. I am going to recommend waiting to submit this though. We are under the Go 1. […]
SGTM!
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Patchset:
Hi Tim, is it ok to merge now?
The tree for 1.18 is opened in golang/go now.
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Tim King (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le.
1 comment:
Patchset:
Hi Tim, is it ok to merge now? […]
Rebase + rerun trybots for thoroughness?
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Rebecca Stambler.
Patch set 7:Run-TryBot +1
kokoro (Gerrit)
Attention is currently required from: Rebecca Stambler.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/14fce639-23f1-424c-b447-3925d806c99f
Patch set 7:gopls-CI +1
Tim King (Gerrit)
Attention is currently required from: Rebecca Stambler, Cuong Manh Le.
Patch set 7:Code-Review +2
1 comment:
Patchset:
Rebase + rerun trybots for thoroughness?
Done
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.
Tim King (Gerrit)
Tim King submitted this change.
go/analysis/passes/testinggoroutine: also inspect defined/anonymous functions
Currently, testinggoroutine only inspects functions literal invoked as
"go func(){ ... }()".
For defined or anonymous functions like "go f(t)" it didn't traverse the
function body. To fix this, on encountering those kinds of functions,
retrieve the definition node then inspect it.
Fixes golang/go#47470
Change-Id: I83b6eb3bf2689c66aee32f13d34002aa3cd175b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/338529
Trust: Cuong Manh Le <cuong.m...@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.m...@gmail.com>
gopls-CI: kokoro <noreply...@google.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Tim King <tak...@google.com>
Reviewed-by: Emmanuel Odeke <emma...@orijtech.com>
---
M go/analysis/passes/testinggoroutine/testdata/src/a/a.go
M go/analysis/passes/testinggoroutine/testinggoroutine.go
2 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/go/analysis/passes/testinggoroutine/testdata/src/a/a.go b/go/analysis/passes/testinggoroutine/testdata/src/a/a.go
index 5fe9041..c211ec3 100644
--- a/go/analysis/passes/testinggoroutine/testdata/src/a/a.go
+++ b/go/analysis/passes/testinggoroutine/testdata/src/a/a.go
@@ -36,19 +36,6 @@
}
}
-func BenchmarkBadFatalf(b *testing.B) {
- var wg sync.WaitGroup
- defer wg.Wait()
-
- for i := 0; i < b.N; i++ {
- wg.Add(1)
- go func(id int) {
- defer wg.Done()
- b.Fatalf("TestFailed: id = %v\n", id) // want "call to .+B.+Fatalf from a non-test goroutine"
- }(i)
- }
-}
-
func TestBadFatal(t *testing.T) {
var wg sync.WaitGroup
defer wg.Wait()
@@ -62,6 +49,32 @@
}
}
+func f(t *testing.T, _ string) {
+ t.Fatal("TestFailed")
+}
+
+func TestBadFatalIssue47470(t *testing.T) {
+ go f(t, "failed test 1") // want "call to .+T.+Fatal from a non-test goroutine"
+
+ g := func(t *testing.T, _ string) {
+ t.Fatal("TestFailed")
+ }
+ go g(t, "failed test 2") // want "call to .+T.+Fatal from a non-test goroutine"
+}
+
+func BenchmarkBadFatalf(b *testing.B) {
+ var wg sync.WaitGroup
+ defer wg.Wait()
+
+ for i := 0; i < b.N; i++ {
+ wg.Add(1)
+ go func(id int) {
+ defer wg.Done()
+ b.Fatalf("TestFailed: id = %v\n", id) // want "call to .+B.+Fatalf from a non-test goroutine"
+ }(i)
+ }
+}
+
func BenchmarkBadFatal(b *testing.B) {
var wg sync.WaitGroup
defer wg.Wait()
diff --git a/go/analysis/passes/testinggoroutine/testinggoroutine.go b/go/analysis/passes/testinggoroutine/testinggoroutine.go
index d2b9a56..800bef5 100644
--- a/go/analysis/passes/testinggoroutine/testinggoroutine.go
+++ b/go/analysis/passes/testinggoroutine/testinggoroutine.go
@@ -119,11 +119,29 @@
return varTypeName, ok
}
+// goStmtFunc returns the ast.Node of a call expression
+// that was invoked as a go statement. Currently, only
+// function literals declared in the same function, and
+// static calls within the same package are supported.
+func goStmtFun(goStmt *ast.GoStmt) ast.Node {
+ switch goStmt.Call.Fun.(type) {
+ case *ast.Ident:
+ id := goStmt.Call.Fun.(*ast.Ident)
+ if funDecl, ok := id.Obj.Decl.(ast.Node); ok {
+ return funDecl
+ }
+ case *ast.FuncLit:
+ return goStmt.Call.Fun
+ }
+ return goStmt.Call
+}
+
// checkGoStmt traverses the goroutine and checks for the
// use of the forbidden *testing.(B, T) methods.
func checkGoStmt(pass *analysis.Pass, goStmt *ast.GoStmt) {
+ fn := goStmtFun(goStmt)
// Otherwise examine the goroutine to check for the forbidden methods.
- ast.Inspect(goStmt, func(n ast.Node) bool {
+ ast.Inspect(fn, func(n ast.Node) bool {
selExpr, ok := n.(*ast.SelectorExpr)
if !ok {
return true
@@ -147,7 +165,11 @@
return true
}
if typeName, ok := typeIsTestingDotTOrB(field.Type); ok {
- pass.ReportRangef(selExpr, "call to (*%s).%s from a non-test goroutine", typeName, selExpr.Sel)
+ var fnRange analysis.Range = goStmt
+ if _, ok := fn.(*ast.FuncLit); ok {
+ fnRange = selExpr
+ }
+ pass.ReportRangef(fnRange, "call to (*%s).%s from a non-test goroutine", typeName, selExpr.Sel)
}
return true
})
To view, visit change 338529. To unsubscribe, or for help writing mail filters, visit settings.