Skip to content

Commit 16b9742

Browse files
committed
go/analysis/passes/loopclosure: use go/types' object resolution
Object resolution in go/types is more accurate than the purely syntactic object resolution in go/parser (for example, golang/go#45160), and spans multiple files. Use it for more accurate diagnostics in the loopclosure analyzer. Change-Id: I2160876dd4b3fd83aa07a9da971f27c17d7d6043 Reviewed-on: https://go-review.googlesource.com/c/tools/+/432137 Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 81a42f0 commit 16b9742

File tree

4 files changed

+36
-12
lines changed

4 files changed

+36
-12
lines changed

go/analysis/passes/loopclosure/loopclosure.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
5959
}
6060
inspect.Preorder(nodeFilter, func(n ast.Node) {
6161
// Find the variables updated by the loop statement.
62-
var vars []*ast.Ident
62+
var vars []types.Object
6363
addVar := func(expr ast.Expr) {
64-
if id, ok := expr.(*ast.Ident); ok {
65-
vars = append(vars, id)
64+
if id, _ := expr.(*ast.Ident); id != nil {
65+
if obj := pass.TypesInfo.ObjectOf(id); obj != nil {
66+
vars = append(vars, obj)
67+
}
6668
}
6769
}
6870
var body *ast.BlockStmt
@@ -132,17 +134,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
132134

133135
ast.Inspect(lit.Body, func(n ast.Node) bool {
134136
id, ok := n.(*ast.Ident)
135-
if !ok || id.Obj == nil {
137+
if !ok {
136138
return true
137139
}
138-
if pass.TypesInfo.Types[id].Type == nil {
139-
// Not referring to a variable (e.g. struct field name)
140+
obj := pass.TypesInfo.Uses[id]
141+
if obj == nil {
140142
return true
141143
}
142144
for _, v := range vars {
143-
if v.Obj == id.Obj {
144-
pass.ReportRangef(id, "loop variable %s captured by func literal",
145-
id.Name)
145+
if v == obj {
146+
pass.ReportRangef(id, "loop variable %s captured by func literal", id.Name)
146147
}
147148
}
148149
return true

go/analysis/passes/loopclosure/loopclosure_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ package loopclosure_test
77
import (
88
"testing"
99

10-
"golang.org/x/tools/internal/analysisinternal"
11-
"golang.org/x/tools/internal/typeparams"
12-
1310
"golang.org/x/tools/go/analysis/analysistest"
1411
"golang.org/x/tools/go/analysis/passes/loopclosure"
12+
"golang.org/x/tools/internal/analysisinternal"
13+
"golang.org/x/tools/internal/typeparams"
1514
)
1615

1716
func Test(t *testing.T) {

go/analysis/passes/loopclosure/testdata/src/a/a.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"golang.org/x/sync/errgroup"
1111
)
1212

13+
var A int
14+
1315
func _() {
1416
var s []int
1517
for i, v := range s {
@@ -51,6 +53,19 @@ func _() {
5153
println(i, v)
5254
}()
5355
}
56+
57+
// iteration variable declared outside the loop
58+
for A = range s {
59+
go func() {
60+
println(A) // want "loop variable A captured by func literal"
61+
}()
62+
}
63+
// iteration variable declared in a different file
64+
for B = range s {
65+
go func() {
66+
println(B) // want "loop variable B captured by func literal"
67+
}()
68+
}
5469
// If the key of the range statement is not an identifier
5570
// the code should not panic (it used to).
5671
var x [2]int
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright 2022 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 testdata
6+
7+
// B is declared in a separate file to test that object resolution spans the
8+
// entire package.
9+
var B int

0 commit comments

Comments
 (0)