Skip to content
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.

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

cmd/vet: more clearly document go tool vet on a single file #23916

Closed
ooesili opened this issue Feb 19, 2018 · 11 comments

Comments

Projects
None yet
4 participants
@ooesili
Copy link

commented Feb 19, 2018

What did you do?

After upgrading from 1.9.4 to 1.10, I ran go vet against a single test file.

$ go vet vetissue_test.go

I have simplified the code that gave me the original issue and uploaded it
here.

What did you expect to see?

No output, and a successful exit code. This command succeeded on Go 1.9.4.

What did you see instead?

$ go vet vetissue_test.go
# command-line-arguments
./vetissue_test.go:6:6: undefined: value

What's happening here is that go vet is complaining about not being able to
find the definition of an identifier that was declared in a different file in
the same package.

It looks like this change in behaviour was introduced in an effort to run go vet in parallel with go test, specifically in https://golang.org/cl/74355
which causes go vet to fail when the type check fails.

I'm not really sure what the best course of action is here. My thought is that
when running go vet against a single file, you can never be guaranteed to have
all of the type information you need, so we could ignore type check failures in
vet against single files.

System details

go version go1.10 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/wmerkel/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/wmerkel/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/52/h8r09kdd5t17prlr9whr4cs4pm06q9/T/go-build420935568=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.10 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.10
uname -v: Darwin Kernel Version 17.4.0: Sun Dec 17 09:19:54 PST 2017; root:xnu-4570.41.2~1/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.13.3
BuildVersion:	17D47
lldb --version: lldb-900.0.64
  Swift-4.0

@ooesili ooesili changed the title cmd/vet: behavior of vetting a single file has change from 1.9.4 to 1.10 cmd/vet: behavior of vetting a single file has changed from 1.9.4 to 1.10 Feb 19, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2018

To run vet on a single file, use go tool vet.

@ooesili

This comment has been minimized.

Copy link
Author

commented Feb 19, 2018

Will go tool vet return the same results as running go vet on the package the file is in? I saw this #16086 (comment) and am curious about what checks might be skipped if the type-check information is not available.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2018

It's true that some checks are only available if type checking information is available. That's not a change from 1.9, though. Offhand I'm not sure which vet tests are affected.

@carterjones

This comment has been minimized.

Copy link

commented Feb 19, 2018

I confirmed the behavior differs between go vet and go tool vet (see fork here: https://github.com/carterjones/vetissue):

$ go vet other_file.go             
# command-line-arguments
./other_file.go:6:5: undefined: value
$ go tool vet other_file.go 
$ go version               
go version go1.10 darwin/amd64
@ooesili

This comment has been minimized.

Copy link
Author

commented Feb 19, 2018

So I guess the breaking change already happened, and there is a workaround (go tool vet instead of go vet). My only asks would be that this be documented somewhere, and we maintain a position on what's allowed to break in the future. It makes sense that over time go vet will get smarter and fail when it previously didn't but I don't want this to happen again.

Can we commit to saying go vet should only be run against packages, and that go tool vet will always work against single files?

carterjones added a commit to carterjones/pre-commit-golang that referenced this issue Feb 19, 2018

fix go vet check
See golang/go#23916 for bug details.
@mvdan

This comment has been minimized.

Copy link
Member

commented Feb 19, 2018

Can we commit to saying go vet should only be run against packages, and that go tool vet will always work against single files?

$ go help vet
usage: go vet [-n] [-x] [build flags] [vet flags] [packages]

Vet runs the Go vet command on the packages named by the import paths.
[...]
$ go tool vet -h
Usage of vet:
        vet [flags] directory...
        vet [flags] files... # Must be a single package
[...]

I understand that this change of behavior of go vet in 1.10 can be confusing, but I belive it's already mentioned in the changelog clearly enough: https://golang.org/doc/go1.10#vet

@mvdan

This comment has been minimized.

Copy link
Member

commented Feb 19, 2018

The fact that go tool vet can work without type information (i.e. without full packages) isn't terribly obvious from reading its usage text, though. A change there might be useful.

@ooesili

This comment has been minimized.

Copy link
Author

commented Feb 19, 2018

usage: go vet [-n] [-x] [build flags] [vet flags] [packages]

Considering this, wouldn't it make sense for go vet to explicitly not accept individual files as arguments? Since it doesn't work anyways, we could give users a better error message.

@mvdan

This comment has been minimized.

Copy link
Member

commented Feb 19, 2018

From go help packages:

As a special case, if the package list is a list of .go files from a single directory, the command is applied to a single synthesized package made up of exactly those files, ignoring any build constraints in those files and ignoring any other files in the directory.

Both accept files. The question is whether it's a requirement that they form a complete package or not.

oschwald added a commit to maxmind/Code-TidyAll-Plugin-Go that referenced this issue Mar 10, 2018

Use `go tool vet`
As of 1.10, `go vet` no longer works correctly on individual files. As
such, we use `go tool vet` instead. See
golang/go#23916 for more information.

@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Mar 28, 2018

@ianlancetaylor ianlancetaylor changed the title cmd/vet: behavior of vetting a single file has changed from 1.9.4 to 1.10 cmd/vet: more clearly document go tool vet on a single file Mar 28, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2018

It's not clear to me what to do here. I feel like there is documentation for how the tool works at https://golang.org/cmd/vet. It's true that vet changed from 1.9 to 1.10, and that was documented in the release notes. The question now is: what, if anything, needs to change in the cmd/vet and/or cmd/go docs? It would be very helpful if people who think that there should be a change could send in a CL. Thanks.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2018

No response, so I'm going to close this issue. Please comment if you disagree.

@andresmgot andresmgot referenced this issue Mar 29, 2019

Closed

Comments require Spaces at the beginning #1012

0 of 3 tasks complete

@SataQiu SataQiu referenced this issue Apr 8, 2019

Merged

Change 'go vet' to 'go tool vet' #1019

2 of 3 tasks complete

rargulati added a commit to keep-network/pre-commit-golang that referenced this issue Apr 18, 2019

Ensure we vet the entire project at the same time
Rather than running go vet on a single file at a time, we are required
to run it on the entire project. This is because `go tool vet` and `go
vet` differ in behavior. `go tool vet` is able to analyze and infer
types in a single file. `go vet` is unable to do so (and requires the
context of the entire project`. We change the behavior to run `go vet`
on the entire project.

More captured in this issue:
golang/go#23916

rargulati added a commit to keep-network/pre-commit-golang that referenced this issue Apr 18, 2019

Merge pull request #2 from keep-network/fix-incorrect-go-vet
Ensure we vet the entirety of the project

Rather than running go vet on a single file at a time, we are required
to run it on the entire project. This is because `go tool vet` and 
`go vet` differ in behavior. `go tool vet` is able to analyze and infer
types in a single file. `go vet` is unable to do so (and requires the
context of the entire project`. We change the behavior to run `go vet`
on the entire project.

More captured in this issue:
golang/go#23916
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.