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

x/tools/gopls: diagnostics not returned for dependencies #32859

Open
myitcv opened this issue Jun 29, 2019 · 4 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Jun 29, 2019

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

$ go version
go version devel +623d653db7 Sat Jun 29 13:17:15 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20190628222527-fb37f6ba8261
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.2-0.20190628222527-fb37f6ba8261

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/myitcv/gostuff/src/github.com/myitcv/govim/cmd/govim/.bin"
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build123661815=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Consider the following setup:

-- go.mod --
module mod.com

-- main.go --
package main

import _ "mod.com/p"

func main() {
}
-- p/p.go --
package p

var x Type

If we open main.go we do not get back any diagnostics for the dependency package mod.com/p, despite there being a compile error that prevents the main package from compiling:

# mod.com/p
p/p.go:3:7: undefined: Type

If we then open p/p.go we do get the diagnostic with the above type check error.

What did you expect to see?

The diagnostic for p/p.go to be returned when we open main.go, because it's an error in a dependency of the package to which main.go belongs.

What did you see instead?

As above.


cc @stamblerre @ianthehat

@gopherbot gopherbot added this to the Unreleased milestone Jun 29, 2019
@stamblerre stamblerre changed the title x/tools/cmd/gopls: diagnostics not returned for dependencies x/tools/gopls: diagnostics not returned for dependencies Jul 2, 2019
@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Aug 31, 2019

@stamblerre I think this also belongs in the gopls v1.0.0, principally because of the dissonance with the compiler output. Thoughts?

@myitcv myitcv modified the milestones: Unreleased, gopls v1.0 Sep 11, 2019
@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Sep 11, 2019

On a related note, other packages in the main module also do not have diagnostics reported until a file from that package is opened. This creates something of a continuity problem with the compiler, especially when running go test $m/... for some main module.

This can clearly be argued both ways.

Is there perhaps an argument therefore for making this configurable?

Despite this comment about other packages in the main module, I think dependencies' diagnostics should always be reported.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Oct 30, 2019

This remains an issue as of b2a7f28a184a6e7aea33316b2d8a321cc9d7e138.

Based on discussion with @ianthehat, I think the "right" thing to do is to return diagnostics for the packages and their transitive dependencies of open files. But analysis results should only be returned for main module package files.

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Oct 31, 2019

That sounds like a reasonable approach to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.