diff --git a/gopls/internal/analysis/norangeoverfunc/norangeoverfunc.go b/gopls/internal/analysis/norangeoverfunc/norangeoverfunc.go new file mode 100644 index 00000000000..aa58e89d75b --- /dev/null +++ b/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 +} diff --git a/gopls/internal/settings/analysis.go b/gopls/internal/settings/analysis.go index ff3cdf67f4c..9a41039f352 100644 --- a/gopls/internal/settings/analysis.go +++ b/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}, diff --git a/gopls/internal/test/marker/testdata/diagnostics/range-over-func-67237.txt b/gopls/internal/test/marker/testdata/diagnostics/range-over-func-67237.txt new file mode 100644 index 00000000000..76fb99ac39c --- /dev/null +++ b/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") +}