Skip to content

go/types: add Package.GoVersion method #61175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
rsc opened this issue Jul 5, 2023 · 9 comments
Closed

go/types: add Package.GoVersion method #61175

rsc opened this issue Jul 5, 2023 · 9 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted release-blocker
Milestone
Go1.21

Comments

@rsc
Copy link
Contributor

rsc commented Jul 5, 2023

For #61174, it would be good for individual checkers to be able to find out the Go version associated with a given package. The code will start tracking this, but we should export it as well. Strictly speaking this is not necessary for Go 1.21, but I suggest we include it now, since it will be necessary for Go 1.22 and it will be easier for tool authors if all the related changes are in the same Go version.

The new method is:

// GoVersion returns the minimum Go version required by this package.
// If the minimum version is unknown, GoVersion returns the empty string.
// Individual source files may specify a different minimum Go version,
// as reported in the [go/ast.File.GoVersion] field.
func (pkg *Package) GoVersion() string { return pkg.goVersion }

The goVersion is set during type-checking, copied from Config.GoVersion.
@rsc rsc added this to the Proposal milestone Jul 5, 2023
@rsc
Copy link
Contributor Author

rsc commented Jul 5, 2023

I take back it not being necessary. Checkers like cgocall re-typecheck certain files and need to be able to propagate the language version. Adding this method means we can use:

diff --git a/go/analysis/passes/cgocall/cgocall.go b/go/analysis/passes/cgocall/cgocall.go
index afff0d82d..1c9723fea 100644
--- a/go/analysis/passes/cgocall/cgocall.go
+++ b/go/analysis/passes/cgocall/cgocall.go
@@ -268,8 +268,9 @@ func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*a
 		Importer: importerFunc(func(path string) (*types.Package, error) {
 			return importMap[path], nil
 		}),
-		Sizes: sizes,
-		Error: func(error) {}, // ignore errors (e.g. unused import)
+		Sizes:     sizes,
+		Error:     func(error) {}, // ignore errors (e.g. unused import)
+		GoVersion: pkg.GoVersion(),
 	}
 
 	// It's tempting to record the new types in the

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/507975 mentions this issue: go/types: record Config.GoVersion for reporting in Package.GoVersion method

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/507880 mentions this issue: go/analysis: pass package's Go version to type checker

@rsc rsc added this to Proposals Jul 5, 2023
@rsc rsc moved this to Incoming in Proposals Jul 5, 2023
@findleyr
Copy link
Member

findleyr commented Jul 5, 2023

This makes sense to me.

I don't think it's strictly necessary, since it would be possible for any caller (such as an analysis driver) to keep track of its own inputs. However, since the version is unambiguously associated with the resulting package, it make sense to capture it in the package directly.

@rsc
Copy link
Contributor Author

rsc commented Jul 5, 2023

At the very least it seems to be necessary for
https://go-review.googlesource.com/c/tools/+/507880/2/go/analysis/passes/cgocall/cgocall.go which wants to re-typecheck some extra files using the same config as the original package.

@rsc
Copy link
Contributor Author

rsc commented Jul 5, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Likely Accept in Proposals Jul 5, 2023
@rsc
Copy link
Contributor Author

rsc commented Jul 12, 2023

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals Jul 12, 2023
@rsc rsc changed the title proposal: go/types: add Package.GoVersion method go/types: add Package.GoVersion method Jul 12, 2023
@rsc rsc modified the milestones: Proposal, Backlog Jul 12, 2023
bradfitz pushed a commit to tailscale/go that referenced this issue Jul 15, 2023
@rsc @bradfitz
go/types: record Config.GoVersion for reporting in Package.GoVersion …
e9af5ae 
…method

Clients of go/types, such as analyzers, may need to know which
specific Go version a package is written for. Record that information
in the Package and expose it using the new GoVersion method.

Update parseGoVersion to handle the new Go versions that may
be passed around starting in Go 1.21.0: versions like "go1.21.0"
and "go1.21rc2". This is not strictly necessary today, but it adds some
valuable future-proofing.

While we are here, change NewChecker from panicking on invalid
version to saving an error for returning later from Files.
Go versions are now likely to be coming from a variety of sources,
not just hard-coded in calls to NewChecker, making a panic
inappropriate.

For golang#61174.
Fixes golang#61175.

Change-Id: Ibe41fe207c1b6e71064b1fe448ac55776089c541
Reviewed-on: https://go-review.googlesource.com/c/go/+/507975
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@findleyr findleyr mentioned this issue Jul 19, 2023
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/511096 mentions this issue: go/types, types2: update documentation for GoVersion

@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Jul 21, 2023
gopherbot pushed a commit that referenced this issue Jul 21, 2023
@findleyr
go/types, types2: update documentation for GoVersion
12f3d68 
Update the documentation for Config.GoVersion to reflect the changes
made for #61175.

Change-Id: I9f3fbcf8ee88e52d6a5e7cf80dad3d2fb5313893
Reviewed-on: https://go-review.googlesource.com/c/go/+/511096
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/511699 mentions this issue: [release-branch.go1.21] go/types, types2: update documentation for GoVersion

gopherbot pushed a commit that referenced this issue Jul 24, 2023
@findleyr
[release-branch.go1.21] go/types, types2: update documentation for Go…
8472fcb 
…Version

Update the documentation for Config.GoVersion to reflect the changes
made for #61175.

Change-Id: I9f3fbcf8ee88e52d6a5e7cf80dad3d2fb5313893
Reviewed-on: https://go-review.googlesource.com/c/go/+/511096
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
(cherry picked from commit 12f3d68)
Reviewed-on: https://go-review.googlesource.com/c/go/+/511699
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
bradfitz pushed a commit to tailscale/go that referenced this issue Aug 2, 2023
@findleyr @bradfitz
[release-branch.go1.21] go/types, types2: update documentation for Go…
db64f13 
…Version

Update the documentation for Config.GoVersion to reflect the changes
made for golang#61175.

Change-Id: I9f3fbcf8ee88e52d6a5e7cf80dad3d2fb5313893
Reviewed-on: https://go-review.googlesource.com/c/go/+/511096
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
(cherry picked from commit 12f3d68)
Reviewed-on: https://go-review.googlesource.com/c/go/+/511699
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
@picatz picatz mentioned this issue Jan 19, 2024
Merged
awly pushed a commit to tailscale/go that referenced this issue Feb 7, 2024
@rsc @awly
go/types: record Config.GoVersion for reporting in Package.GoVersion …
55ed560 
…method

Clients of go/types, such as analyzers, may need to know which
specific Go version a package is written for. Record that information
in the Package and expose it using the new GoVersion method.

Update parseGoVersion to handle the new Go versions that may
be passed around starting in Go 1.21.0: versions like "go1.21.0"
and "go1.21rc2". This is not strictly necessary today, but it adds some
valuable future-proofing.

While we are here, change NewChecker from panicking on invalid
version to saving an error for returning later from Files.
Go versions are now likely to be coming from a variety of sources,
not just hard-coded in calls to NewChecker, making a panic
inappropriate.

For golang#61174.
Fixes golang#61175.

Change-Id: Ibe41fe207c1b6e71064b1fe448ac55776089c541
Reviewed-on: https://go-review.googlesource.com/c/go/+/507975
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@golang golang locked and limited conversation to collaborators Jul 20, 2024
@rsc rsc removed this from Proposals Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Accepted release-blocker
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@rsc @dmitshur @gopherbot @findleyr