linter: loopvarcapture should check interface methods

pkg/cmd/roachtest/tests/kv.go

@@ -757,6 +757,7 @@ func registerKVScalability(r registry.Registry) {
 
 		const maxPerNodeConcurrency = 64
 		for i := nodes; i <= nodes*maxPerNodeConcurrency; i += nodes {
+			i := i // capture loop variable
 			c.Wipe(ctx, c.Range(1, nodes))
 			c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.Range(1, nodes))
 

pkg/testutils/lint/passes/loopvarcapture/BUILD.bazel

@@ -25,6 +25,18 @@ go_test(
     data = glob(["testdata/**"]) + [
         "@go_sdk//:files",
     ],
+    # N.B. We must disable CGO owing to a rather obscure failure [1].
+    #      It's surmised that the root cause is the missing CGO metadata
+    #      inside the test's runfiles directory. By disabling CGO, we're
+    #      allowing the go type checker to create a dummy object [2] without
+    #      causing a (type) resolution error.
+    #
+    # [1] https://github.com/golang/go/issues/36547
+    # [2] https://github.com/golang/tools/blob/master/go/packages/packages.go#L1056-L1064
+    #
+    env = {
+        "CGO_ENABLED": "0",
+    },
     deps = [
         ":loopvarcapture",
         "//pkg/build/bazel",

pkg/testutils/lint/passes/loopvarcapture/loopvarcapture.go

@@ -405,9 +405,8 @@ func (v *Visitor) visitLoopClosure(closure *ast.FuncLit) {
 // whether that call is being made to one of the functions in the
 // GoRoutineFunctions slice.
 func (v *Visitor) isGoRoutineFunction(call *ast.CallExpr) bool {
-	callee := typeutil.StaticCallee(v.pass.TypesInfo, call)
-	// call to a builtin
-	if callee == nil {
+	callee, ok := typeutil.Callee(v.pass.TypesInfo, call).(*types.Func)
+	if !ok {
 		return false
 	}
 	pkg := callee.Pkg()

pkg/testutils/lint/passes/loopvarcapture/loopvarcapture_test.go

@@ -24,6 +24,7 @@ var extraGoRoutineFunctions = []loopvarcapture.Function{
 	{Pkg: "example.org/concurrency", Type: "Group", Name: "Go"}, // test non-pointer receiver
 	{Pkg: "example.org/concurrency", Name: "Go"},                // test a package-level function
 	{Pkg: "example.org/concurrency", Name: "GoWithError"},       // test a function with a return value
+	{Pkg: "p", Type: "Monitor", Name: "Go"},                     // test an interface method
 }
 
 func init() {
@@ -35,17 +36,16 @@ func init() {
 func TestAnalyzer(t *testing.T) {
 	skip.UnderStress(t)
 
-	// The test fails unless RunDespiteErrors is set to true.
-	// This is not fully understood, something to do with missing metadata
-	// for the "C" pseudo-package.
-	// See comments on #84867 and https://github.com/golang/go/issues/36547.
 	testAnalyzer := *loopvarcapture.Analyzer
-	testAnalyzer.RunDespiteErrors = true
 	originalGoRoutineFunctions := loopvarcapture.GoRoutineFunctions
 	loopvarcapture.GoRoutineFunctions = append(originalGoRoutineFunctions, extraGoRoutineFunctions...)
 	defer func() { loopvarcapture.GoRoutineFunctions = originalGoRoutineFunctions }()
 
 	testdata := datapathutils.TestDataPath(t)
 	analysistest.TestData = func() string { return testdata }
-	analysistest.Run(t, testdata, &testAnalyzer, "p")
+	results := analysistest.Run(t, testdata, &testAnalyzer, "p")
+	// Perform a sanity check--we expect analysis results.
+	if len(results) == 0 {
+		t.Fatal("analysis results are empty")
+	}
 }

pkg/testutils/lint/passes/loopvarcapture/testdata/src/p/p.go

@@ -11,6 +11,7 @@
 package p
 
 import (
+	"context"
 	"fmt"
 	"math/rand"
 	"net"
@@ -430,3 +431,45 @@ func RespectsNolintComments() {
 		wg.Wait()
 	}
 }
+
+// regression test for https://github.com/cockroachdb/cockroach/issues/102678
+func registerKVScalability() {
+	runScalability := func(ctx context.Context, percent int) {
+		nodes := 3
+
+		const maxPerNodeConcurrency = 64
+		for i := nodes; i <= nodes*maxPerNodeConcurrency; i += nodes {
+			fmt.Println("running workload")
+
+			m := NewMonitor(ctx)
+			m.Go(func(ctx context.Context) error {
+				cmd := fmt.Sprintf("./workload run kv --init --read-percent=%d "+
+					"--splits=1000 --duration=1m "+fmt.Sprintf("--concurrency=%d", i), // want `loop variable 'i' captured by reference`
+					percent, nodes)
+
+				return fmt.Errorf("failed to run workload: %s", cmd)
+			})
+		}
+	}
+	runScalability(nil, 0)
+}
+
+type Monitor interface {
+	Go(fn func(context.Context) error)
+}
+
+type monitorImpl struct {
+	ctx context.Context
+}
+
+func (m monitorImpl) Go(fn func(context.Context) error) {
+	panic("implement me")
+}
+
+func NewMonitor(ctx context.Context) Monitor {
+	return newMonitor(ctx)
+}
+
+func newMonitor(ctx context.Context) *monitorImpl {
+	return &monitorImpl{ctx: ctx}
+}