Skip to content

Commit ae303ab

Browse files
findleyrgopherbot
authored andcommitted
gopls/internal/analysis/modernize: replace WithCancel with t.Cancel
This CL adds a modernizer to replace calls to context.WithCancel with calls to t.Cancel, where t is the *testing.T (or B, or F) for the relevant surrounding test function. Example: func TestFoo(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel ... } => func TestFoo(t *testing.T) { ctx := t.Context() } Also, factor out an analysisinternal.IsPointerNamed helper to assist with identifying pointers to named types. This slightly alters the behavior of the bloop pass, as it was previously tolerant implicitly referenced testing.B variables, but that seems unimportant. Updates golang/go#70815 Change-Id: Id10b5feb85a43e71d5ad740198d27135e8a3e6cf Reviewed-on: https://go-review.googlesource.com/c/tools/+/641440 Auto-Submit: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 60643c0 commit ae303ab

File tree

12 files changed

+432
-13
lines changed

12 files changed

+432
-13
lines changed

gopls/doc/analyzers.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,8 @@ existing code by using more modern features of Go, such as:
480480
from the maps package, added in go1.21;
481481
- replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),
482482
added in go1.19;
483+
- replacing uses of context.WithCancel in tests with t.Context, added in
484+
go1.24;
483485

484486
Default: on.
485487

gopls/internal/analysis/modernize/bloop.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"golang.org/x/tools/go/types/typeutil"
1717
"golang.org/x/tools/internal/analysisinternal"
1818
"golang.org/x/tools/internal/astutil/cursor"
19-
"golang.org/x/tools/internal/typesinternal"
2019
)
2120

2221
// bloop updates benchmarks that use "for range b.N", replacing it
@@ -90,7 +89,7 @@ func bloop(pass *analysis.Pass) {
9089
if cmp, ok := n.Cond.(*ast.BinaryExpr); ok && cmp.Op == token.LSS {
9190
if sel, ok := cmp.Y.(*ast.SelectorExpr); ok &&
9291
sel.Sel.Name == "N" &&
93-
isTestingB(info.TypeOf(sel.X)) {
92+
analysisinternal.IsPointerToNamed(info.TypeOf(sel.X), "testing", "B") {
9493

9594
delStart, delEnd := n.Cond.Pos(), n.Cond.End()
9695

@@ -133,7 +132,7 @@ func bloop(pass *analysis.Pass) {
133132
n.Key == nil &&
134133
n.Value == nil &&
135134
sel.Sel.Name == "N" &&
136-
isTestingB(info.TypeOf(sel.X)) {
135+
analysisinternal.IsPointerToNamed(info.TypeOf(sel.X), "testing", "B") {
137136

138137
pass.Report(analysis.Diagnostic{
139138
// Highlight "range b.N".
@@ -152,10 +151,6 @@ func bloop(pass *analysis.Pass) {
152151
}
153152
}
154153

155-
func isTestingB(t types.Type) bool {
156-
return analysisinternal.IsTypeNamed(typesinternal.Unpointer(t), "testing", "B")
157-
}
158-
159154
// uses reports whether the subtree cur contains a use of obj.
160155
func uses(info *types.Info, cur cursor.Cursor, obj types.Object) bool {
161156
for curId := range cur.Preorder((*ast.Ident)(nil)) {

gopls/internal/analysis/modernize/doc.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,6 @@
2323
// from the maps package, added in go1.21;
2424
// - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),
2525
// added in go1.19;
26+
// - replacing uses of context.WithCancel in tests with t.Context, added in
27+
// go1.24;
2628
package modernize

gopls/internal/analysis/modernize/modernize.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ func run(pass *analysis.Pass) (any, error) {
6363
mapsloop(pass)
6464
minmax(pass)
6565
sortslice(pass)
66+
testingContext(pass)
6667

6768
// TODO(adonovan):
6869
// - more modernizers here; see #70815.

gopls/internal/analysis/modernize/modernize_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,6 @@ func Test(t *testing.T) {
2020
"mapsloop",
2121
"minmax",
2222
"sortslice",
23+
"testingcontext",
2324
)
2425
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package testingcontext
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package testingcontext
2+
3+
import (
4+
"context"
5+
6+
"testing"
7+
)
8+
9+
func Test(t *testing.T) {
10+
ctx, cancel := context.WithCancel(context.Background()) // want "context.WithCancel can be modernized using t.Context"
11+
defer cancel()
12+
_ = ctx
13+
14+
func() {
15+
ctx, cancel := context.WithCancel(context.Background()) // Nope. scope of defer is not the testing func.
16+
defer cancel()
17+
_ = ctx
18+
}()
19+
20+
{
21+
ctx, cancel := context.WithCancel(context.TODO()) // want "context.WithCancel can be modernized using t.Context"
22+
defer cancel()
23+
_ = ctx
24+
var t int // not in scope of the call to WithCancel
25+
_ = t
26+
}
27+
28+
{
29+
ctx := context.Background()
30+
ctx, cancel := context.WithCancel(context.Background()) // Nope. ctx is redeclared.
31+
defer cancel()
32+
_ = ctx
33+
}
34+
35+
{
36+
var t int
37+
ctx, cancel := context.WithCancel(context.Background()) // Nope. t is shadowed.
38+
defer cancel()
39+
_ = ctx
40+
_ = t
41+
}
42+
43+
t.Run("subtest", func(t2 *testing.T) {
44+
ctx, cancel := context.WithCancel(context.Background()) // want "context.WithCancel can be modernized using t2.Context"
45+
defer cancel()
46+
_ = ctx
47+
})
48+
}
49+
50+
func TestAlt(t2 *testing.T) {
51+
ctx, cancel := context.WithCancel(context.Background()) // want "context.WithCancel can be modernized using t2.Context"
52+
defer cancel()
53+
_ = ctx
54+
}
55+
56+
func Testnot(t *testing.T) {
57+
ctx, cancel := context.WithCancel(context.Background()) // Nope. Not a test func.
58+
defer cancel()
59+
_ = ctx
60+
}
61+
62+
func Benchmark(b *testing.B) {
63+
ctx, cancel := context.WithCancel(context.Background()) // want "context.WithCancel can be modernized using b.Context"
64+
defer cancel()
65+
_ = ctx
66+
67+
b.Run("subtest", func(b2 *testing.B) {
68+
ctx, cancel := context.WithCancel(context.Background()) // want "context.WithCancel can be modernized using b2.Context"
69+
defer cancel()
70+
_ = ctx
71+
})
72+
}
73+
74+
func Fuzz(f *testing.F) {
75+
ctx, cancel := context.WithCancel(context.Background()) // want "context.WithCancel can be modernized using f.Context"
76+
defer cancel()
77+
_ = ctx
78+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package testingcontext
2+
3+
import (
4+
"context"
5+
6+
"testing"
7+
)
8+
9+
func Test(t *testing.T) {
10+
ctx := t.Context()
11+
_ = ctx
12+
13+
func() {
14+
ctx, cancel := context.WithCancel(context.Background()) // Nope. scope of defer is not the testing func.
15+
defer cancel()
16+
_ = ctx
17+
}()
18+
19+
{
20+
ctx := t.Context()
21+
_ = ctx
22+
var t int // not in scope of the call to WithCancel
23+
_ = t
24+
}
25+
26+
{
27+
ctx := context.Background()
28+
ctx, cancel := context.WithCancel(context.Background()) // Nope. ctx is redeclared.
29+
defer cancel()
30+
_ = ctx
31+
}
32+
33+
{
34+
var t int
35+
ctx, cancel := context.WithCancel(context.Background()) // Nope. t is shadowed.
36+
defer cancel()
37+
_ = ctx
38+
_ = t
39+
}
40+
41+
t.Run("subtest", func(t2 *testing.T) {
42+
ctx := t2.Context()
43+
_ = ctx
44+
})
45+
}
46+
47+
func TestAlt(t2 *testing.T) {
48+
ctx := t2.Context()
49+
_ = ctx
50+
}
51+
52+
func Testnot(t *testing.T) {
53+
ctx, cancel := context.WithCancel(context.Background()) // Nope. Not a test func.
54+
defer cancel()
55+
_ = ctx
56+
}
57+
58+
func Benchmark(b *testing.B) {
59+
ctx := b.Context()
60+
_ = ctx
61+
62+
b.Run("subtest", func(b2 *testing.B) {
63+
ctx := b2.Context()
64+
_ = ctx
65+
})
66+
}
67+
68+
func Fuzz(f *testing.F) {
69+
ctx := f.Context()
70+
_ = ctx
71+
}

0 commit comments

Comments
 (0)