go/analysis/passes/loopclosure: restore exact Go 1.20 logic, with future logic now only accessible via internal flag

Author: thepudds

go/analysis/passes/internal/loopclosure/main.go

@@ -0,0 +1,20 @@
+// 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.
+
+// The loopclosure command applies the golang.org/x/tools/go/analysis/passes/loopclosure
+// analysis to the specified packages of Go source code.
+// It enables additional experimental checking that is disabled by default in Go 1.20.
+//
+package main
+
+import (
+	"golang.org/x/tools/go/analysis/passes/loopclosure"
+	"golang.org/x/tools/go/analysis/singlechecker"
+	"golang.org/x/tools/internal/analysisinternal"
+)
+
+func main() {
+	analysisinternal.LoopclosureGo121 = true
+	singlechecker.Main(loopclosure.Analyzer)
+}

go/analysis/passes/loopclosure/loopclosure.go

@@ -18,6 +18,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
+	"golang.org/x/tools/internal/analysisinternal"
 )
 
 const Doc = `check references to loop variables from within nested functions
@@ -87,6 +88,108 @@ var Analyzer = &analysis.Analyzer{
 }
 
 func run(pass *analysis.Pass) (interface{}, error) {
+	// Check if we are enabling additional experimental logic.
+	if !analysisinternal.LoopclosureGo121 {
+		return runGo120(pass)
+	}
+	return runGo121(pass)
+}
+
+// runGo120 runs the analyzer with logic intended for Go 1.20 cmd/vet.
+// TODO: delete this once the Go 1.21 dev cycle has started.
+func runGo120(pass *analysis.Pass) (interface{}, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+	nodeFilter := []ast.Node{
+		(*ast.RangeStmt)(nil),
+		(*ast.ForStmt)(nil),
+	}
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		// Find the variables updated by the loop statement.
+		vars := make(map[types.Object]int)
+		addVar := func(expr ast.Expr) {
+			if id, _ := expr.(*ast.Ident); id != nil {
+				if obj := pass.TypesInfo.ObjectOf(id); obj != nil {
+					// For runGo121, we use a count to track when to remove
+					// elements from the vars map.
+					// For runGo120, we do not, but set the proper count anyway.
+					vars[obj] = 1
+				}
+			}
+		}
+		var body *ast.BlockStmt
+		switch n := n.(type) {
+		case *ast.RangeStmt:
+			body = n.Body
+			addVar(n.Key)
+			addVar(n.Value)
+		case *ast.ForStmt:
+			body = n.Body
+			switch post := n.Post.(type) {
+			case *ast.AssignStmt:
+				// e.g. for p = head; p != nil; p = p.next
+				for _, lhs := range post.Lhs {
+					addVar(lhs)
+				}
+			case *ast.IncDecStmt:
+				// e.g. for i := 0; i < n; i++
+				addVar(post.X)
+			}
+		}
+		if vars == nil {
+			return
+		}
+
+		// Inspect statements to find function literals that may be run outside of
+		// the current loop iteration.
+		//
+		// For go, defer, and errgroup.Group.Go, we ignore all but the last
+		// statement, where "last" is defined recursively.
+		// See runGo120 for an alternative approach.
+		v := visitor{last: func(v visitor, last ast.Stmt) {
+			var stmts []ast.Stmt
+			switch s := last.(type) {
+			case *ast.GoStmt:
+				stmts = litStmts(s.Call.Fun)
+			case *ast.DeferStmt:
+				stmts = litStmts(s.Call.Fun)
+			case *ast.ExprStmt: // check for errgroup.Group.Go
+				if call, ok := s.X.(*ast.CallExpr); ok {
+					stmts = litStmts(goInvoke(pass.TypesInfo, call))
+				}
+			}
+			for _, stmt := range stmts {
+				reportCaptured(pass, vars, stmt)
+			}
+		}}
+		v.visit(body.List)
+
+		// Also check for testing.T.Run (with T.Parallel).
+		// We consider every t.Run statement in the loop body, because there is
+		// no commonly used mechanism for synchronizing parallel subtests.
+		// It is of course theoretically possible to synchronize parallel subtests,
+		// though such a pattern is likely to be exceedingly rare as it would be
+		// fighting against the test runner.
+		for _, s := range body.List {
+			switch s := s.(type) {
+			case *ast.ExprStmt:
+				if call, ok := s.X.(*ast.CallExpr); ok {
+					for _, stmt := range parallelSubtest(pass.TypesInfo, call) {
+						reportCaptured(pass, vars, stmt)
+					}
+				}
+			}
+		}
+	})
+	return nil, nil
+}
+
+// runGo121 runs the analyzer with additional experimental logic
+// that is not intended for Go 1.20 cmd/vet, including examining
+// statements following a go, defer or errgroup.Group.Go statement
+// to determine if they cannot delay start of execution of the
+// go or defer.
+func runGo121(pass *analysis.Pass) (interface{}, error) {
 	// We do two passes with inspect: first, a simpler walk
 	// looking for problems with testing.T.Run with T.Parallel, and
 	// second, a more involved walk to examine last statements

go/analysis/passes/loopclosure/loopclosure_test.go

@@ -9,14 +9,25 @@ import (
 
 	"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) {
 	testdata := analysistest.TestData()
-	tests := []string{"a", "golang.org/...", "subtests", "go121"}
+	tests := []string{"a", "golang.org/...", "subtests"}
 	if typeparams.Enabled {
 		tests = append(tests, "typeparams")
 	}
 	analysistest.Run(t, testdata, loopclosure.Analyzer, tests...)
+
+	// Enable experimental checking that is disabled by default in Go 1.20.
+	defer func(go121Test bool) {
+		analysisinternal.LoopclosureGo121 = go121Test
+	}(analysisinternal.LoopclosureGo121)
+	analysisinternal.LoopclosureGo121 = true
+
+	// Re-run everything, plus the go121 tests
+	tests = append(tests, "go121")
+	analysistest.Run(t, testdata, loopclosure.Analyzer, tests...)
 }

internal/analysisinternal/analysis.go

@@ -19,6 +19,10 @@ import (
 // in Go 1.18+.
 var DiagnoseFuzzTests bool = false
 
+// LoopclosureGo121 controls whether the 'loopclosure' analyzer performs additional
+// diagnoses that are disabled in Go 1.20.
+var LoopclosureGo121 bool = false
+
 func TypeErrorEndPos(fset *token.FileSet, src []byte, start token.Pos) token.Pos {
 	// Get the end position for the type error.
 	offset, end := fset.PositionFor(start, false).Offset, start