x/tools/go/analysis/passes/testinggoroutine: do not ignore calls in goroutines (false negative)

[Antonboom]

$ go vet -testinggoroutine some_test.go

func TestSomething(t *testing.T)
	go func() {
		helper(t)       // No warning :(
	}()
}

func helper(t *testing.T) {
        t.Helper()
	t.Fatal()
}

[findleyr]

Thanks. We discussed this in triage: This could be done (via two passes over the package), but might not be worth the complexity. Seems like a low priority.

[gopherbot]

Change https://go.dev/cl/541155 mentions this issue: go/analysis/passes/testinggoroutine: report by enclosing regions

[ccoVeille]

Change https://go.dev/cl/541155 mentions this issue: go/analysis/passes/testinggoroutine: report by enclosing regions

I found back this old issue, and found there was something made about this issue with 1733061

The code is still there
https://github.com/golang/tools/blob/master/go/analysis/passes/testinggoroutine/testinggoroutine.go

The commit only refers to "Updates #63849" it's not about fixing it.

So I'm curious, what did this commit was about in the context on this issue.

I'm asking because apparently the issue is still there (status open) and I can replicate the issue in the code.

Thanks

[ccoVeille]

Maybe here is the difference

This is detected via changes made with 1733061

func TestSomething(t *testing.T) {
	go fail(t)
}

func fail(tb *testing.T) {
	tb.Helper()

	tb.FailNow()
}

This is not detected

func TestSomething(t *testing.T) {
	go func() {
		fail(t)
	}()
}

func fail(tb *testing.T) {
	tb.Helper()
	tb.FailNow()
}

The second pattern (also the exact one reported in the example of this issue) is obviously the most frequent

[timothy-king]

As you correctly point out, this example is not yet handled. So the issue has not yet been closed.

FWIW the first pattern is simpler to detect. If someone wants to contribute an extension to the checker to detect the second that is sufficiently accurate, performant, etc, they can contribute PR.