go/analysis: extend the loopclosure checker to considering errgroup.Group.Go.

errgroup.Group.Go(f) executes f asynchronously in a Go routine. This Go call is used quite often in Go projects.

Change-Id: I397af118300a25a5c38dbce83fcead974b58cef2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287173
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Trust: Tim King <taking@google.com>

go/analysis/passes/loopclosure/loopclosure.go

@@ -8,22 +8,14 @@ package loopclosure
 
 import (
 	"go/ast"
+	"go/types"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/go/types/typeutil"
 )
 
-// TODO(adonovan): also report an error for the following structure,
-// which is often used to ensure that deferred calls do not accumulate
-// in a loop:
-//
-//	for i, x := range c {
-//		func() {
-//			...reference to i or x...
-//		}()
-//	}
-
 const Doc = `check references to loop variables from within nested functions
 
 This analyzer checks for references to loop variables from within a
@@ -95,16 +87,19 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		if len(body.List) == 0 {
 			return
 		}
-		var last *ast.CallExpr
+		// The function invoked in the last return statement.
+		var fun ast.Expr
 		switch s := body.List[len(body.List)-1].(type) {
 		case *ast.GoStmt:
-			last = s.Call
+			fun = s.Call.Fun
 		case *ast.DeferStmt:
-			last = s.Call
-		default:
-			return
+			fun = s.Call.Fun
+		case *ast.ExprStmt: // check for errgroup.Group.Go()
+			if call, ok := s.X.(*ast.CallExpr); ok {
+				fun = goInvokes(pass.TypesInfo, call)
+			}
 		}
-		lit, ok := last.Fun.(*ast.FuncLit)
+		lit, ok := fun.(*ast.FuncLit)
 		if !ok {
 			return
 		}
@@ -128,3 +123,43 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	})
 	return nil, nil
 }
+
+// goInvokes returns a function expression that would be called asynchronously
+// (but not awaited) in another goroutine as a consequence of the call.
+// For example, given the g.Go call below, it returns the function literal expression.
+//
+//   import "sync/errgroup"
+//   var g errgroup.Group
+//   g.Go(func() error { ... })
+//
+// Currently only "golang.org/x/sync/errgroup.Group()" is considered.
+func goInvokes(info *types.Info, call *ast.CallExpr) ast.Expr {
+	f := typeutil.StaticCallee(info, call)
+	// Note: Currently only supports: golang.org/x/sync/errgroup.Go.
+	if f == nil || f.Name() != "Go" {
+		return nil
+	}
+	recv := f.Type().(*types.Signature).Recv()
+	if recv == nil {
+		return nil
+	}
+	rtype, ok := recv.Type().(*types.Pointer)
+	if !ok {
+		return nil
+	}
+	named, ok := rtype.Elem().(*types.Named)
+	if !ok {
+		return nil
+	}
+	if named.Obj().Name() != "Group" {
+		return nil
+	}
+	pkg := f.Pkg()
+	if pkg == nil {
+		return nil
+	}
+	if pkg.Path() != "golang.org/x/sync/errgroup" {
+		return nil
+	}
+	return call.Args[0]
+}

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

@@ -6,6 +6,8 @@
 
 package testdata
 
+import "golang.org/x/sync/errgroup"
+
 func _() {
 	var s []int
 	for i, v := range s {
@@ -88,3 +90,31 @@ func _() {
 		}()
 	}
 }
+
+// Group is used to test that loopclosure does not match on any type named "Group".
+// The checker only matches on methods "(*...errgroup.Group).Go".
+type Group struct{};
+
+func (g *Group) Go(func() error) {}
+
+func _() {
+	var s []int
+	// errgroup.Group.Go() invokes Go routines
+	g := new(errgroup.Group)
+	for i, v := range s {
+		g.Go(func() error {
+			print(i) // want "loop variable i captured by func literal"
+			print(v) // want "loop variable v captured by func literal"
+			return nil
+		})
+	}
+	// Do not match other Group.Go cases
+	g1 := new(Group)
+	for i, v := range s {
+		g1.Go(func() error {
+			print(i)
+			print(v)
+			return nil
+		})
+	}
+}

go/analysis/passes/loopclosure/testdata/src/golang.org/x/sync/errgroup/errgroup.go

@@ -0,0 +1,12 @@
+// Package errgroup synthesizes Go's package "golang.org/x/sync/errgroup",
+// which is used in unit-testing.
+package errgroup
+
+type Group struct {
+}
+
+func (g *Group) Go(f func() error) {
+	go func() {
+		f()
+	}()
+}