Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
Antonboom opened this issue Oct 31, 2023 · 2 comments
Open

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

Antonboom opened this issue Oct 31, 2023 · 2 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@Antonboom
Copy link 

$ 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()
}
@Antonboom Antonboom mentioned this issue Oct 31, 2023
Closed
@bcmills bcmills changed the title go/analysis/passes/testinggoroutine: do not ignore calls in goroutines (false negative) golang.org/x/tools/go/analysis/passes/testinggoroutine: do not ignore calls in goroutines (false negative) Oct 31, 2023
@bcmills bcmills added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Oct 31, 2023
@findleyr
Copy link
Contributor

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.

@seankhliao seankhliao changed the title golang.org/x/tools/go/analysis/passes/testinggoroutine: do not ignore calls in goroutines (false negative) x/tools/go/analysis/passes/testinggoroutine: do not ignore calls in goroutines (false negative) Oct 31, 2023
@gopherbot gopherbot added this to the Unreleased milestone Oct 31, 2023
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 31, 2023
@Antonboom Antonboom mentioned this issue Nov 2, 2023
Merged
@gopherbot
Copy link

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

gopherbot pushed a commit to golang/tools that referenced this issue Nov 21, 2023
@timothy-king
go/analysis/passes/testinggoroutine: report by enclosing regions
1733061 
This adds support for t.Run() statements. The new version
performs two passes. The first pass collects all of the
`go callee` and `t.Run(name, callee)` statements. The second pass
inspects each callee FuncDecl or FuncLit for a call to a
tb.Forbidden() function.

When the enclosing callee function called from an t.Run() statement,
it reports when tb is declared outside of the body of the
enclosing function.

Fixes golang/go#63799
Updates golang/go#63849
Updates golang/go#48124

Change-Id: I10d5f2a0af9b985126dbfcc98eaab97757a7f71d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/541155
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Tim King <taking@google.com>
@a-nogikh a-nogikh mentioned this issue Nov 28, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@bcmills @gopherbot @seankhliao @Antonboom @findleyr