Golang cmd/go: always pass a GoVersion to cmd/vet

Reported by @alexaandru in https://github.com/golang/go/issues/63888#issuecomment-1933649310

$ cat go.mod
module example.com/m

go 1.22.0
$ cat main.go
package main

func main() {
    for i := range 10 {
        go func() { println(i) }()
    }
}

No output (as expected) when running as a package:

$ go1.22.0 vet example.com/m

False positive when running on a file:

$ go1.22.0 vet main.go
# command-line-arguments
# [command-line-arguments]
./main.go:5:23: loop variable i captured by func literal

go1.22.0 vet -x -json example.com/m gives the GoVersion as "GoVersion": "go1.22.0",. go1.22.0 vet -x -json main.go gives the GoVersion as "GoVersion": "",.

@bcmills

Comment From: bcmills

We set the GoVersion for cmd/vet based on the module containing the package to be built: https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/work/exec.go;l=1168-1174;drc=f81e4986733bc18ec2bef16549534b9029756444 In the absence of that parameter I would expect cmd/vet to default to the most recent Go version, but maybe that's not the case?

At any rate, that does seem to match what we do when we invoke the compiler: we also omit the flag in that case. https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/work/gc.go;l=71-79;drc=b4e7d630bc6fbf654a20a4bebda94a8150811bea

This is kind of an unfortunate consequence of the property that files given on the command line are not considered to be part of a module (see https://go.dev/cl/339170).

That also has some implications for, say, importing internal packages, which we might not have fully taken into account. This may need more thought.

(CC @matloob @samthanawalla)

Comment From: timothy-king

With the range scope change, there are some tools (x/tools/go/ssa, nilness, staticcheck) that need a decision about what the GoVersion of a file is. ssa and vet were defaulting to unknown meant the earliest possible version, e.g. 1. That seemed like a backwards compatible direction at the time. The vet bug report was an inconsistency with the command line file name and the module. The loopclosure.Analyzer can be fixed to interpret "" as an 'unknown' version and can suppress these. ssa needs to choose a semantics though. Guessing the toolchain version for a file would produce inconsistent results between runs. (More likely to guess more up to date?)

This may need more thought.

+1 Feel free to pull me into the conversation as needed.

Comment From: changkun

I was trying to remove some of the x := x parts from my existing code.

It turns out that golangci-lint invokes go vet, and remains to report loop variable i captured by func literal with Go 1.22, and some weird _ = x needs to stay in the code.

Any suggestions we can get rid of this in 1.22.0 release?

Comment From: timothy-king

@changkun I don't think this is directly related to this issue, and I will need to ask several follow-up questions. Please open a new issue and cc me.

Comment From: bcmills

Some interesting thought-experiments to consider: - If I run go run /tmp/foo.go, there isn't a “real” module corresponding to that file at all. - If go run ./nested/foo.go, and ./nested is a different (nested) module (or outside of my workspace), it may be the case that that module isn't in the build list at all (in which case it would be weird to report that module as containing the file), and it may also be the case that the go or toolchain directive in that module's go.mod file is actually higher than the toolchain that we're currently running.

In discussing with @rsc, @matloob, and @samthanawalla, we're thinking maybe the thing to do for now is to use the language version from either the go.work file or the main module's go.mod file.

On the other hand, there is currently a special case for resolving imports of internal paths, which works only if the directory containing the files package is within a main module. Maybe we can do something similar for the Go version. (https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/load/pkg.go;l=1539-1548;drc=36b14a78b58924a8aea22c8949c3b8a4b7045d8b)

Comment From: timothy-king

In discussing with @rsc, @matloob, and @samthanawalla, we're thinking maybe the thing to do for now is to use the language version from either the go.work file or the main module's go.mod file.

Any discussion for when you were thinking this should be done? Part of 1.22.1 ? If it is after, maybe we should update cmd/vet to use the same logic in the interim? Having vet differ from run seems like a bad state of affairs.

Comment From: rsc

Does vet differ from the compiler here? Presumably the go command passes the same version to both. If it does differ, why does it differ?

Comment From: bcmills

I think the compiler defaults to the highest go version rather than an older one.

Comment From: gopherbot

Change https://go.dev/cl/567435 mentions this issue: cmd/go: set the GoVersion for files listed on the commandline

Comment From: gopherbot

Change https://go.dev/cl/567635 mentions this issue: internal/versions: updates the meaning of FileVersions.

Comment From: gopherbot

Change https://go.dev/cl/591136 mentions this issue: cmd/go: set the GoVersion for files listed on the commandline with vet

Comment From: gopherbot

Change https://go.dev/cl/593156 mentions this issue: cmd/go: set GoVersion for files on the command line with vet

Comment From: gopherbot

Change https://go.dev/cl/593315 mentions this issue: cmd/go: set GoVersion for files on the command line with vet

Comment From: gopherbot

Change https://go.dev/cl/593295 mentions this issue: cmd/go: set GoVersion for files on the command line with vet

Comment From: samthanawalla

This is fixed now. The GoVersion will always be passed for cmd/vet

Created #68159 for changing which version will be passed for files listed on the command line going forward.