From 16b974289fe50bc5c949118ac08d09af63e093f3 Mon Sep 17 00:00:00 2001
From: Robert Findley <rfindley@google.com>
Date: Tue, 20 Sep 2022 17:19:59 -0400
Subject: [PATCH] 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 <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
---
 go/analysis/passes/loopclosure/loopclosure.go | 19 ++++++++++---------
 .../passes/loopclosure/loopclosure_test.go    |  5 ++---
 .../passes/loopclosure/testdata/src/a/a.go    | 15 +++++++++++++++
 .../passes/loopclosure/testdata/src/a/b.go    |  9 +++++++++
 4 files changed, 36 insertions(+), 12 deletions(-)
 create mode 100644 go/analysis/passes/loopclosure/testdata/src/a/b.go

diff --git a/go/analysis/passes/loopclosure/loopclosure.go b/go/analysis/passes/loopclosure/loopclosure.go
index aed3e43143..645e5895bb 100644
--- a/go/analysis/passes/loopclosure/loopclosure.go
+++ b/go/analysis/passes/loopclosure/loopclosure.go
@@ -59,10 +59,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	}
 	inspect.Preorder(nodeFilter, func(n ast.Node) {
 		// Find the variables updated by the loop statement.
-		var vars []*ast.Ident
+		var vars []types.Object
 		addVar := func(expr ast.Expr) {
-			if id, ok := expr.(*ast.Ident); ok {
-				vars = append(vars, id)
+			if id, _ := expr.(*ast.Ident); id != nil {
+				if obj := pass.TypesInfo.ObjectOf(id); obj != nil {
+					vars = append(vars, obj)
+				}
 			}
 		}
 		var body *ast.BlockStmt
@@ -132,17 +134,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 			ast.Inspect(lit.Body, func(n ast.Node) bool {
 				id, ok := n.(*ast.Ident)
-				if !ok || id.Obj == nil {
+				if !ok {
 					return true
 				}
-				if pass.TypesInfo.Types[id].Type == nil {
-					// Not referring to a variable (e.g. struct field name)
+				obj := pass.TypesInfo.Uses[id]
+				if obj == nil {
 					return true
 				}
 				for _, v := range vars {
-					if v.Obj == id.Obj {
-						pass.ReportRangef(id, "loop variable %s captured by func literal",
-							id.Name)
+					if v == obj {
+						pass.ReportRangef(id, "loop variable %s captured by func literal", id.Name)
 					}
 				}
 				return true
diff --git a/go/analysis/passes/loopclosure/loopclosure_test.go b/go/analysis/passes/loopclosure/loopclosure_test.go
index e00ae21fa8..8bf1d7a7bc 100644
--- a/go/analysis/passes/loopclosure/loopclosure_test.go
+++ b/go/analysis/passes/loopclosure/loopclosure_test.go
@@ -7,11 +7,10 @@ package loopclosure_test
 import (
 	"testing"
 
-	"golang.org/x/tools/internal/analysisinternal"
-	"golang.org/x/tools/internal/typeparams"
-
 	"golang.org/x/tools/go/analysis/analysistest"
 	"golang.org/x/tools/go/analysis/passes/loopclosure"
+	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 func Test(t *testing.T) {
diff --git a/go/analysis/passes/loopclosure/testdata/src/a/a.go b/go/analysis/passes/loopclosure/testdata/src/a/a.go
index 2fc3f21f1c..2a17d16bc8 100644
--- a/go/analysis/passes/loopclosure/testdata/src/a/a.go
+++ b/go/analysis/passes/loopclosure/testdata/src/a/a.go
@@ -10,6 +10,8 @@ import (
 	"golang.org/x/sync/errgroup"
 )
 
+var A int
+
 func _() {
 	var s []int
 	for i, v := range s {
@@ -51,6 +53,19 @@ func _() {
 			println(i, v)
 		}()
 	}
+
+	// iteration variable declared outside the loop
+	for A = range s {
+		go func() {
+			println(A) // want "loop variable A captured by func literal"
+		}()
+	}
+	// iteration variable declared in a different file
+	for B = range s {
+		go func() {
+			println(B) // want "loop variable B captured by func literal"
+		}()
+	}
 	// If the key of the range statement is not an identifier
 	// the code should not panic (it used to).
 	var x [2]int
diff --git a/go/analysis/passes/loopclosure/testdata/src/a/b.go b/go/analysis/passes/loopclosure/testdata/src/a/b.go
new file mode 100644
index 0000000000..d4e5da418e
--- /dev/null
+++ b/go/analysis/passes/loopclosure/testdata/src/a/b.go
@@ -0,0 +1,9 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package testdata
+
+// B is declared in a separate file to test that object resolution spans the
+// entire package.
+var B int