cmd/vet: support "vet a.go a_test.go b.go" when a regular package and a test package #22530
Comments
|
What do the files example_test.go and ticker.go look like? |
ianlancetaylor
added
the
NeedsInvestigation
|
Change https://golang.org/cl/75190 mentions this issue: |
|
Example is included in the test I just pushed @ianlancetaylor |
|
Thanks. I'm not convinced that your fix is correct. The docs for cmd/vet say that when you use vet with a list of files on the command line, all of the files must be in the same package. Your example does not do that. I think a better fix would be to detect this case and report an error. I think your CL is trying to patch a specific case of a general problem, but the general problem will just crop up in different ways. |
|
While your are correct it appears that the checker already explicitly tries
to deal with the case of a test and normal package in the same directory so
I don’t believe you can avoid this specific case
…On Wed, 1 Nov 2017 at 21:52, Ian Lance Taylor ***@***.***> wrote:
Thanks. I'm not convinced that your fix is correct. The docs for cmd/vet
say that when you use vet with a list of files on the command line, all of
the files must be in the same package. Your example does not do that. I
think a better fix would be to detect this case and report an error. I
think your CL is trying to patch a specific case of a general problem, but
the general problem will just crop up in different ways.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#22530 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGXL1tTSm-UT8VSMfJjHkXHO3at3wFMks5syOg0gaJpZM4QOuax>
.
|
|
As far as I can see it only does that in the case where you run vet on a directory, not on an explicit list of files. Am I missing something? |
|
extendScope was the precedence I found when looking into this issue. You also have the definition of basePkg and arg processing |
|
An example of something that breaks due to this is gometalinter which calls If we where trying to make it work when pointing it to files that are not in the same package directory then yes it would trying to fix something that its not designed to do, however given the information I have I believe this is an expected use case. The fact that the example in the vet description page suffers from this issue depending on the order that the files are returned from glob further supports this, even though it does state they must be in the same package.
|
|
It sounds like you are arguing that we should change the way that cmd/vet is currently documented to work: when invoked with a list of file names, the files must all be in either the same package or in a test package. If we implement that, then we should make that case work, much as it does when we run vet on a single directory. I don't see any reason to expect that simply sorting the files will make all cases work, even though it fixes your case. We should change vet to handle it correctly by building whatever data structures it builds when running vet on a directory. But before you put any work into that we should decide that that is what we want to have happen. I'll repurpose this issue. |
ianlancetaylor
changed the title
ianlancetaylor
added
the
NeedsDecision
gopherbot
removed
the
NeedsInvestigation
|
Having a quick look at how the dir case handles this, it does so by running a check on all files which match the main package, then it runs a separate pass on the files which match _test the the base package passed in. It looks like this could be replicated to file set case, but I from what I've seen I believe that the sort already achieves most of, if not all of this as the key thing is identifying and using the correct "main" package. |
|
If we're going to do it, let's do it right. |
|
Please try using "go vet" instead of "go tool vet". The latter is now essentially only for developers of vet to help debugging, like "go tool compile". |
|
I can confirm that Should we just update the examples to use |
|
@stevenh Good point, I sent https://golang.org/cl/76390 . |
|
Change https://golang.org/cl/76390 mentions this issue: |
Updates #22530 Change-Id: I161b5e706483744321e6089f747bd761310774eb Reviewed-on: https://go-review.googlesource.com/76390 Reviewed-by: Rob Pike <r@golang.org>
What version of Go are you using (
go version)?go version go1.9.2 freebsd/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env)?GOARCH="amd64"
GOBIN="/data/go/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOOS="freebsd"
GOPATH="/data/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/freebsd_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build969955960=/tmp/go-build -gno-record-gcc-switches"
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"
What did you do?
What did you expect to see?
No errors
What did you see instead?
The issue is order dependent so if name the files in the opposite order then no error occurs.
The problem is that the pkg used by the checks is the first package from the listed files.
The text was updated successfully, but these errors were encountered: