internal/versions: updates the meaning of FileVersions.
For Go >=1.22, FileVersions returns an unknown [invalid] Future version
when the file versions would be invalid. This better matches
go/types.
For Go <=1.21, FileVersions returns either the runtime.Version()
or the Future version.
Adds AtLeast and Before for doing comparisons with Future.
Updates golang/go#65612
Fixes golang/go#66007
Change-Id: I93ff1681b0f9117765614a20d82642749597307c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/567635
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/go/analysis/passes/loopclosure/loopclosure.go b/go/analysis/passes/loopclosure/loopclosure.go
index 7b78939..fe05eda 100644
--- a/go/analysis/passes/loopclosure/loopclosure.go
+++ b/go/analysis/passes/loopclosure/loopclosure.go
@@ -55,9 +55,8 @@
switch n := n.(type) {
case *ast.File:
// Only traverse the file if its goversion is strictly before go1.22.
- goversion := versions.Lang(versions.FileVersions(pass.TypesInfo, n))
- // goversion is empty for older go versions (or the version is invalid).
- return goversion == "" || versions.Compare(goversion, "go1.22") < 0
+ goversion := versions.FileVersion(pass.TypesInfo, n)
+ return versions.Before(goversion, versions.Go1_22)
case *ast.RangeStmt:
body = n.Body
addVar(n.Key)
diff --git a/go/analysis/passes/loopclosure/loopclosure_test.go b/go/analysis/passes/loopclosure/loopclosure_test.go
index 386f532..c8aab48 100644
--- a/go/analysis/passes/loopclosure/loopclosure_test.go
+++ b/go/analysis/passes/loopclosure/loopclosure_test.go
@@ -17,6 +17,9 @@
)
func Test(t *testing.T) {
+ // legacy loopclosure test expectations are incorrect > 1.21.
+ testenv.SkipAfterGo1Point(t, 21)
+
testdata := analysistest.TestData()
analysistest.Run(t, testdata, loopclosure.Analyzer,
"a", "golang.org/...", "subtests", "typeparams")
diff --git a/go/analysis/passes/loopclosure/testdata/src/a/a.go b/go/analysis/passes/loopclosure/testdata/src/a/a.go
index 7a7f05f..eb4d2a6 100644
--- a/go/analysis/passes/loopclosure/testdata/src/a/a.go
+++ b/go/analysis/passes/loopclosure/testdata/src/a/a.go
@@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-// This file contains tests for the loopclosure checker.
+// This file contains legacy tests for the loopclosure checker.
+// Legacy expectations are incorrect after go1.22.
package testdata
diff --git a/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go b/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go
index 50283ec..faf9838 100644
--- a/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go
+++ b/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go
@@ -2,8 +2,9 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-// This file contains tests that the loopclosure analyzer detects leaked
+// This file contains legacy tests that the loopclosure analyzer detects leaked
// references via parallel subtests.
+// Legacy expectations are incorrect after go1.22.
package subtests
diff --git a/go/analysis/passes/loopclosure/testdata/src/typeparams/typeparams.go b/go/analysis/passes/loopclosure/testdata/src/typeparams/typeparams.go
index 55e129c..ef5b143 100644
--- a/go/analysis/passes/loopclosure/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/loopclosure/testdata/src/typeparams/typeparams.go
@@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-// This file contains tests for the loopclosure checker.
+// This file contains legacy tests for the loopclosure checker for GoVersion <go1.22.
+// Expectations are incorrect after go1.22.
//go:build go1.18
@@ -45,16 +46,16 @@
a P
}
-func (t T[P]) Go(func() error) { }
+func (t T[P]) Go(func() error) {}
func _(g T[errgroup.Group]) {
var s []int
for i, v := range s {
// "T.a" is method "(*...errgroup.Group).Go".
g.a.Go(func() error {
- print(i) // want "loop variable i captured by func literal"
- print(v) // want "loop variable v captured by func literal"
+ print(i) // want "loop variable i captured by func literal"
+ print(v) // want "loop variable v captured by func literal"
return nil
})
}
-}
\ No newline at end of file
+}
diff --git a/go/ssa/builder.go b/go/ssa/builder.go
index 9588406..72e906c 100644
--- a/go/ssa/builder.go
+++ b/go/ssa/builder.go
@@ -1747,8 +1747,7 @@
// Use forStmtGo122 instead if it applies.
if s.Init != nil {
if assign, ok := s.Init.(*ast.AssignStmt); ok && assign.Tok == token.DEFINE {
- afterGo122 := versions.Compare(fn.goversion, "go1.21") > 0
- if afterGo122 {
+ if versions.AtLeast(fn.goversion, versions.Go1_22) {
b.forStmtGo122(fn, s, label)
return
}
@@ -2243,7 +2242,7 @@
}
}
- afterGo122 := versions.Compare(fn.goversion, "go1.21") > 0
+ afterGo122 := versions.AtLeast(fn.goversion, versions.Go1_22)
if s.Tok == token.DEFINE && !afterGo122 {
// pre-go1.22: If iteration variables are defined (:=), this
// occurs once outside the loop.
diff --git a/go/ssa/create.go b/go/ssa/create.go
index 4545c17..f8f584a 100644
--- a/go/ssa/create.go
+++ b/go/ssa/create.go
@@ -245,7 +245,7 @@
if len(files) > 0 {
// Go source package.
for _, file := range files {
- goversion := versions.Lang(versions.FileVersions(p.info, file))
+ goversion := versions.Lang(versions.FileVersion(p.info, file))
for _, decl := range file.Decls {
membersFromDecl(p, decl, goversion)
}
diff --git a/internal/versions/features.go b/internal/versions/features.go
new file mode 100644
index 0000000..b53f178
--- /dev/null
+++ b/internal/versions/features.go
@@ -0,0 +1,43 @@
+// Copyright 2023 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 versions
+
+// This file contains predicates for working with file versions to
+// decide when a tool should consider a language feature enabled.
+
+// GoVersions that features in x/tools can be gated to.
+const (
+ Go1_18 = "go1.18"
+ Go1_19 = "go1.19"
+ Go1_20 = "go1.20"
+ Go1_21 = "go1.21"
+ Go1_22 = "go1.22"
+)
+
+// Future is an invalid unknown Go version sometime in the future.
+// Do not use directly with Compare.
+const Future = ""
+
+// AtLeast reports whether the file version v comes after a Go release.
+//
+// Use this predicate to enable a behavior once a certain Go release
+// has happened (and stays enabled in the future).
+func AtLeast(v, release string) bool {
+ if v == Future {
+ return true // an unknown future version is always after y.
+ }
+ return Compare(Lang(v), Lang(release)) >= 0
+}
+
+// Before reports whether the file version v is strictly before a Go release.
+//
+// Use this predicate to disable a behavior once a certain Go release
+// has happened (and stays enabled in the future).
+func Before(v, release string) bool {
+ if v == Future {
+ return false // an unknown future version happens after y.
+ }
+ return Compare(Lang(v), Lang(release)) < 0
+}
diff --git a/internal/versions/toolchain.go b/internal/versions/toolchain.go
new file mode 100644
index 0000000..377bf7a
--- /dev/null
+++ b/internal/versions/toolchain.go
@@ -0,0 +1,14 @@
+// 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 versions
+
+// toolchain is maximum version (<1.22) that the go toolchain used
+// to build the current tool is known to support.
+//
+// When a tool is built with >=1.22, the value of toolchain is unused.
+//
+// x/tools does not support building with go <1.18. So we take this
+// as the minimum possible maximum.
+var toolchain string = Go1_18
diff --git a/internal/versions/toolchain_go119.go b/internal/versions/toolchain_go119.go
new file mode 100644
index 0000000..f65beed
--- /dev/null
+++ b/internal/versions/toolchain_go119.go
@@ -0,0 +1,14 @@
+// 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.
+
+//go:build go1.19
+// +build go1.19
+
+package versions
+
+func init() {
+ if Compare(toolchain, Go1_19) < 0 {
+ toolchain = Go1_19
+ }
+}
diff --git a/internal/versions/toolchain_go120.go b/internal/versions/toolchain_go120.go
new file mode 100644
index 0000000..1a9efa1
--- /dev/null
+++ b/internal/versions/toolchain_go120.go
@@ -0,0 +1,14 @@
+// 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.
+
+//go:build go1.20
+// +build go1.20
+
+package versions
+
+func init() {
+ if Compare(toolchain, Go1_20) < 0 {
+ toolchain = Go1_20
+ }
+}
diff --git a/internal/versions/toolchain_go121.go b/internal/versions/toolchain_go121.go
new file mode 100644
index 0000000..b7ef216
--- /dev/null
+++ b/internal/versions/toolchain_go121.go
@@ -0,0 +1,14 @@
+// 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.
+
+//go:build go1.21
+// +build go1.21
+
+package versions
+
+func init() {
+ if Compare(toolchain, Go1_21) < 0 {
+ toolchain = Go1_21
+ }
+}
diff --git a/internal/versions/types_go121.go b/internal/versions/types_go121.go
index a7b7920..b4345d3 100644
--- a/internal/versions/types_go121.go
+++ b/internal/versions/types_go121.go
@@ -12,9 +12,19 @@
"go/types"
)
-// FileVersions always reports the a file's Go version as the
-// zero version at this Go version.
-func FileVersions(info *types.Info, file *ast.File) string { return "" }
+// FileVersion returns a language version (<=1.21) derived from runtime.Version()
+// or an unknown future version.
+func FileVersion(info *types.Info, file *ast.File) string {
+ // In x/tools built with Go <= 1.21, we do not have Info.FileVersions
+ // available. We use a go version derived from the toolchain used to
+ // compile the tool by default.
+ // This will be <= go1.21. We take this as the maximum version that
+ // this tool can support.
+ //
+ // There are no features currently in x/tools that need to tell fine grained
+ // differences for versions <1.22.
+ return toolchain
+}
-// InitFileVersions is a noop at this Go version.
+// InitFileVersions is a noop when compiled with this Go version.
func InitFileVersions(*types.Info) {}
diff --git a/internal/versions/types_go122.go b/internal/versions/types_go122.go
index 7b9ba89..e818063 100644
--- a/internal/versions/types_go122.go
+++ b/internal/versions/types_go122.go
@@ -12,10 +12,27 @@
"go/types"
)
-// FileVersions maps a file to the file's semantic Go version.
-// The reported version is the zero version if a version cannot be determined.
-func FileVersions(info *types.Info, file *ast.File) string {
- return info.FileVersions[file]
+// FileVersions returns a file's Go version.
+// The reported version is an unknown Future version if a
+// version cannot be determined.
+func FileVersion(info *types.Info, file *ast.File) string {
+ // In tools built with Go >= 1.22, the Go version of a file
+ // follow a cascades of sources:
+ // 1) types.Info.FileVersion, which follows the cascade:
+ // 1.a) file version (ast.File.GoVersion),
+ // 1.b) the package version (types.Config.GoVersion), or
+ // 2) is some unknown Future version.
+ //
+ // File versions require a valid package version to be provided to types
+ // in Config.GoVersion. Config.GoVersion is either from the package's module
+ // or the toolchain (go run). This value should be provided by go/packages
+ // or unitchecker.Config.GoVersion.
+ if v := info.FileVersions[file]; IsValid(v) {
+ return v
+ }
+ // Note: we could instead return runtime.Version() [if valid].
+ // This would act as a max version on what a tool can support.
+ return Future
}
// InitFileVersions initializes info to record Go versions for Go files.
diff --git a/internal/versions/types_test.go b/internal/versions/types_test.go
index 0ffdd46..59f6d18 100644
--- a/internal/versions/types_test.go
+++ b/internal/versions/types_test.go
@@ -52,11 +52,11 @@
if got, want := versions.GoVersion(pkg), item.pversion; versions.Compare(got, want) != 0 {
t.Errorf("GoVersion()=%q. expected %q", got, want)
}
- if got := versions.FileVersions(info, nil); got != "" {
+ if got := versions.FileVersion(info, nil); got != "" {
t.Errorf(`FileVersions(nil)=%q. expected ""`, got)
}
for i, test := range item.tests {
- if got, want := versions.FileVersions(info, files[i]), test.want; got != want {
+ if got, want := versions.FileVersion(info, files[i]), test.want; got != want {
t.Errorf("FileVersions(%s)=%q. expected %q", test.fname, got, want)
}
}
diff --git a/internal/versions/versions.go b/internal/versions/versions.go
index f898284..8d1f745 100644
--- a/internal/versions/versions.go
+++ b/internal/versions/versions.go
@@ -4,7 +4,9 @@
package versions
-import "strings"
+import (
+ "strings"
+)
// Note: If we use build tags to use go/versions when go >=1.22,
// we run into go.dev/issue/53737. Under some operations users would see an
diff --git a/internal/versions/versions_test.go b/internal/versions/versions_test.go
index 997de2a..dbc1c55 100644
--- a/internal/versions/versions_test.go
+++ b/internal/versions/versions_test.go
@@ -5,8 +5,13 @@
package versions_test
import (
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "go/types"
"testing"
+ "golang.org/x/tools/internal/testenv"
"golang.org/x/tools/internal/versions"
)
@@ -137,3 +142,115 @@
}
}
+
+func TestKnown(t *testing.T) {
+ for _, v := range [...]string{
+ versions.Go1_18,
+ versions.Go1_19,
+ versions.Go1_20,
+ versions.Go1_21,
+ versions.Go1_22,
+ } {
+ if !versions.IsValid(v) {
+ t.Errorf("Expected known version %q to be valid.", v)
+ }
+ if v != versions.Lang(v) {
+ t.Errorf("Expected known version %q == Lang(%q).", v, versions.Lang(v))
+ }
+ }
+}
+
+func TestAtLeast(t *testing.T) {
+ for _, item := range [...]struct {
+ v, release string
+ want bool
+ }{
+ {versions.Future, versions.Go1_22, true},
+ {versions.Go1_22, versions.Go1_22, true},
+ {"go1.21", versions.Go1_22, false},
+ {"invalid", versions.Go1_22, false},
+ } {
+ if got := versions.AtLeast(item.v, item.release); got != item.want {
+ t.Errorf("AtLeast(%q, %q)=%v. wanted %v", item.v, item.release, got, item.want)
+ }
+ }
+}
+
+func TestBefore(t *testing.T) {
+ for _, item := range [...]struct {
+ v, release string
+ want bool
+ }{
+ {versions.Future, versions.Go1_22, false},
+ {versions.Go1_22, versions.Go1_22, false},
+ {"go1.21", versions.Go1_22, true},
+ {"invalid", versions.Go1_22, true}, // invalid < Go1_22
+ } {
+ if got := versions.Before(item.v, item.release); got != item.want {
+ t.Errorf("Before(%q, %q)=%v. wanted %v", item.v, item.release, got, item.want)
+ }
+ }
+}
+
+func TestFileVersions122(t *testing.T) {
+ testenv.NeedsGo1Point(t, 22)
+
+ const source = `
+ package P
+ `
+ fset := token.NewFileSet()
+ f, err := parser.ParseFile(fset, "hello.go", source, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ for _, conf := range []types.Config{
+ {GoVersion: versions.Go1_22},
+ {}, // GoVersion is unset.
+ } {
+ info := &types.Info{}
+ versions.InitFileVersions(info)
+
+ _, err = conf.Check("P", fset, []*ast.File{f}, info)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ v := versions.FileVersion(info, f)
+ if !versions.AtLeast(v, versions.Go1_22) {
+ t.Errorf("versions.AtLeast(%q, %q) expected to hold", v, versions.Go1_22)
+ }
+
+ if versions.Before(v, versions.Go1_22) {
+ t.Errorf("versions.AtLeast(%q, %q) expected to be false", v, versions.Go1_22)
+ }
+
+ if conf.GoVersion == "" && v != versions.Future {
+ t.Error("Expected the FileVersion to be the Future when conf.GoVersion is unset")
+ }
+ }
+}
+
+func TestFileVersions121(t *testing.T) {
+ testenv.SkipAfterGo1Point(t, 21)
+
+ // If <1.22, info and file are ignored.
+ v := versions.FileVersion(nil, nil)
+ oneof := map[string]bool{
+ versions.Go1_18: true,
+ versions.Go1_19: true,
+ versions.Go1_20: true,
+ versions.Go1_21: true,
+ }
+ if !oneof[v] {
+ t.Errorf("FileVersion(...)=%q expected to be a known go version <1.22", v)
+ }
+
+ if versions.AtLeast(v, versions.Go1_22) {
+ t.Errorf("versions.AtLeast(%q, %q) expected to be false", v, versions.Go1_22)
+ }
+
+ if !versions.Before(v, versions.Go1_22) {
+ t.Errorf("versions.Before(%q, %q) expected to hold", v, versions.Go1_22)
+ }
+}
