x/tools/cmd/gotype: -a flag doesn't include XTestGoFiles

[myitcv]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

$ go version
go version go1.7.4 linux/amd64

$ (command cd $(go list -f '{{.Dir}}' golang.org/x/tools/cmd/gotype) && git rev-parse HEAD)
f8ed2e405fdcbb42c55233531ad98721e6d1cb3e

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GORACE=""
GOROOT="/home/myitcv/gos"
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build484566709=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

cat <<EOD > pkg_test.go
package pkg_test

import "testing"

func TestBanana(t *testing.T) {
        x := banana.Fetch()
}
EOD
gotype -a .

What did you expect to see?

pkg_test.go:6:7: undeclared name: banana
pkg_test.go:6:2: x declared but not used

What did you see instead?

Nothing

If instead we change to use the package name:

cat <<EOD > pkg_test.go
package pkg

import "testing"

func TestBanana(t *testing.T) {
        x := banana.Fetch()
}
EOD
gotype -a .

then the output is as expected.

Is the intention that gotype be consistent with cmd/go's definition of Go files? Should therefore XTestGoFiles be considered along with TestGoFiles?

Happy to help contribute if you think this is the right fix.

cc @alandonovan

[alandonovan]

The XTestGoFiles define a different package from the GoFiles and TestGoFiles. gotype would have to run the type-checker a second time to process these files. The best way to do that is to use the golang.org/x/tools/go/loader package, but it seems out of scope for an undocumented diagnostic tool such as gotype.

cc: @griesemer

[josharian]

gotype may be an undocumented diagnostic tool, but it seems many people (myself included) use it when they want a fast answer to the question "does this typecheck?", which is not an uncommon question.

[alandonovan]

On 6 February 2017 at 15:23, Josh Bleecher Snyder ***@***.***> wrote: gotype may be an undocumented diagnostic tool, but it seems many people (myself included) use it when they want a fast answer to the question "does this typecheck?", which is not an uncommon question.

You could try using ssadump for that purpose; it does the right thing for XTestGoFiles, and it also understands cgo. I'll leave it up to Robert to decide what, if anything, to do about gotype.

[griesemer]

@myitcv gotype is not taking into account package-external test files. As is, it can't easily do so because it requires the package to be tested to be installed first (which may not be the case). This is because external test files are not part of the package, they are a separate package. This is related to #11415 . Without that, we cannot easily fix this one.

I'm not saying gotype shouldn't or couldn't do what you are suggesting, but the existing behavior is not a bug, it's intended.

Changed to unplanned. This is not urgent. If anybody wants to look into it, that's fine, but it's not going to be simple without #11415 (which is planned for 1.9).

[myitcv]

@alandonovan @griesemer thanks for the comments, helped steer me in the right direction.

You could try using ssadump for that purpose

@josharian don't know about your use case, but I need gotype (or equivalent) to be very fast, hence I think this rules out ssadump (vs gotype) because it uses go/loader; at least in some rough experiments it and gotype correspond with the kind of performance @alandonovan mentioned here

As is, it can't easily do so because it requires the package to be tested to be installed first (which may not be the case)

In my case I know this will be the case. I have a little helper that aggressively ensures all transitive dependencies (test included) of a "package spec of interest" are go install-ed (helps for other things like gocode, go vet pre #11415, etc)

So what I might do for now is fork tools and modify gotype to do exactly as @alandonovan suggested above; type check the xtest package in a second "pass" after the main package itself succeeds.

I'm not saying gotype shouldn't or couldn't do what you are suggesting, but the existing behavior is not a bug, it's intended.

...

This is not urgent.

Totally agree. Thanks both for taking the time to comment in any case.

[myitcv]

@griesemer I've just mailed https://go-review.googlesource.com/#/c/36992/

Not a perfect fix I agree given your comments about #11415, but perhaps it suffices for now?

[gopherbot]

CL https://golang.org/cl/36992 mentions this issue.