gopls/internal/analysis: disable ssa/ir analyzers on range-over-func

This change disables analyzers that cannot yet safely process
go1.23 range-over-func statements, including buildssa and buildir.
(This is done by poking in an additional Analyzer.Requires edge
on a new temporary analyzer that fails when it sees a range-over-func.)

We plan to revert this change when ssa and ir support the new feature,
but this CL will unblock uses of it in the standard library which
would otherwise cause gopls' tests to crash.

Updates golang/go#67237
Updates dominikh/go-tools#1494

Change-Id: Ibed2a88da94fb84234b4410b6bc7562a493287ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/583778
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

gopls/internal/analysis/norangeoverfunc/norangeoverfunc.go

@@ -0,0 +1,50 @@
+// Copyright 2024 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 norangeoverfunc
+
+// TODO(adonovan): delete this when #67237 and dominikh/go-tools#1494 are fixed.
+
+import (
+	_ "embed"
+	"fmt"
+	"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"
+)
+
+var Analyzer = &analysis.Analyzer{
+	Name: "norangeoverfunc",
+	Doc: `norangeoverfunc fails if a package uses go1.23 range-over-func
+
+Require it from any analyzer that cannot yet safely process this new feature.`,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	filter := []ast.Node{(*ast.RangeStmt)(nil)}
+
+	// TODO(adonovan): opt: short circuit if not using go1.23.
+
+	var found *ast.RangeStmt
+	inspect.Preorder(filter, func(n ast.Node) {
+		if found == nil {
+			stmt := n.(*ast.RangeStmt)
+			if _, ok := pass.TypesInfo.TypeOf(stmt.X).Underlying().(*types.Signature); ok {
+				found = stmt
+			}
+		}
+	})
+	if found != nil {
+		return nil, fmt.Errorf("package %q uses go1.23 range-over-func; cannot build SSA or IR (#67237)",
+			pass.Pkg.Path())
+	}
+
+	return nil, nil
+}

gopls/internal/settings/analysis.go

@@ -12,6 +12,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/atomic"
 	"golang.org/x/tools/go/analysis/passes/atomicalign"
 	"golang.org/x/tools/go/analysis/passes/bools"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
 	"golang.org/x/tools/go/analysis/passes/buildtag"
 	"golang.org/x/tools/go/analysis/passes/cgocall"
 	"golang.org/x/tools/go/analysis/passes/composite"
@@ -49,6 +50,7 @@ import (
 	"golang.org/x/tools/gopls/internal/analysis/fillreturns"
 	"golang.org/x/tools/gopls/internal/analysis/infertypeargs"
 	"golang.org/x/tools/gopls/internal/analysis/nonewvars"
+	"golang.org/x/tools/gopls/internal/analysis/norangeoverfunc"
 	"golang.org/x/tools/gopls/internal/analysis/noresultvalues"
 	"golang.org/x/tools/gopls/internal/analysis/simplifycompositelit"
 	"golang.org/x/tools/gopls/internal/analysis/simplifyrange"
@@ -59,6 +61,7 @@ import (
 	"golang.org/x/tools/gopls/internal/analysis/unusedvariable"
 	"golang.org/x/tools/gopls/internal/analysis/useany"
 	"golang.org/x/tools/gopls/internal/protocol"
+	"honnef.co/go/tools/staticcheck"
 )
 
 // Analyzer augments a [analysis.Analyzer] with additional LSP configuration.
@@ -104,6 +107,33 @@ func (a *Analyzer) String() string { return a.analyzer.String() }
 var DefaultAnalyzers = make(map[string]*Analyzer) // initialized below
 
 func init() {
+	// Emergency workaround for #67237 to allow standard library
+	// to use range over func: disable SSA-based analyses of
+	// go1.23 packages that use range-over-func.
+	suppressOnRangeOverFunc := func(a *analysis.Analyzer) {
+		a.Requires = append(a.Requires, norangeoverfunc.Analyzer)
+	}
+	suppressOnRangeOverFunc(buildssa.Analyzer)
+	// buildir is non-exported so we have to scan the Analysis.Requires graph to find it.
+	var buildir *analysis.Analyzer
+	for _, a := range staticcheck.Analyzers {
+		for _, req := range a.Analyzer.Requires {
+			if req.Name == "buildir" {
+				buildir = req
+			}
+		}
+
+		// Temporarily disable SA4004 CheckIneffectiveLoop as
+		// it crashes when encountering go1.23 range-over-func
+		// (#67237, dominikh/go-tools#1494).
+		if a.Analyzer.Name == "SA4004" {
+			suppressOnRangeOverFunc(a.Analyzer)
+		}
+	}
+	if buildir != nil {
+		suppressOnRangeOverFunc(buildir)
+	}
+
 	// The traditional vet suite:
 	analyzers := []*Analyzer{
 		{analyzer: appends.Analyzer, enabled: true},

gopls/internal/test/marker/testdata/diagnostics/range-over-func-67237.txt

@@ -0,0 +1,77 @@
+
+This test verifies that SSA-based analyzers don't run on packages that
+use range-over-func. This is an emergency fix of #67237 (for buildssa)
+until we land https://go.dev/cl/555075.
+
+Similarly, it is an emergency fix of dominikh/go-tools#1494 (for
+buildir) until that package is similarly fixed for go1.23.
+
+Explanation:
+- Package p depends on q and r, and analyzers buildssa and buildir
+  depend on norangeoverfunc.
+- Analysis pass norangeoverfunc@q fails, thus norangeoverfunc@p is not
+  executed; but norangeoverfunc@r is ok
+- nilness requires buildssa, which is not facty, so it can run on p and r.
+- SA1025 requires buildir, which is facty, so SA1025 can run only on r.
+
+-- flags --
+-min_go=go1.23
+
+-- settings.json --
+{
+	"staticcheck": true
+}
+
+-- go.mod --
+module example.com
+
+go 1.23
+
+-- p/p.go --
+package p // a dependency uses range-over-func, so nilness runs but SA1025 cannot (buildir is facty)
+
+import (
+	_ "example.com/q"
+	_ "example.com/r"
+	"fmt"
+)
+
+func _(ptr *int) {
+	if ptr == nil {
+		println(*ptr) //@diag(re"[*]ptr", re"nil dereference in load")
+	}
+	_ = fmt.Sprintf("%s", "abc") // no SA1025 finding
+}
+
+-- q/q.go --
+package q // uses range-over-func, so no diagnostics from nilness or SA1025
+
+import "fmt"
+
+type iterSeq[T any] func(yield func(T) bool)
+
+func _(seq iterSeq[int]) {
+	for x := range seq {
+		println(x)
+	}
+
+	_  = fmt.Sprintf("%s", "abc")
+}
+
+func _(ptr *int) {
+	if ptr == nil {
+		println(*ptr) // no nilness finding
+	}
+}
+
+-- r/r.go --
+package r // does not use range-over-func, so nilness and SA1025 report diagnosticcs
+
+import "fmt"
+
+func _(ptr *int) {
+	if ptr == nil {
+		println(*ptr) //@diag(re"[*]ptr", re"nil dereference in load")
+	}
+	_ = fmt.Sprintf("%s", "abc") //@diag(re"fmt", re"no need to use fmt.Sprintf")
+}