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

Jump to bottom

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

Closed
myitcv opened this issue Jun 29, 2019 · 5 comments
Closed

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

myitcv opened this issue Jun 29, 2019 · 5 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
gopls/v0.4.0

Comments

@myitcv
Copy link
Member

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
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Jun 29, 2019
@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 myitcv mentioned this issue Jul 14, 2019
Closed
@stamblerre stamblerre added help wanted Suggested Issues that may be good for new contributors looking for work to do. and removed Suggested Issues that may be good for new contributors looking for work to do. labels Aug 8, 2019
@myitcv
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

myitcv commented Oct 30, 2019

This remains an issue as of b2a7f28.

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 stamblerre added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 31, 2019
@stamblerre
Copy link
Contributor

That sounds like a reasonable approach to me.

@myitcv
Copy link
Member Author

myitcv commented Jan 20, 2020

I've just done a test against 0cba7a3 with partial revert of CL 214586 applied on top, in the form of CL 215239, and this now appears to be fixed. Thanks very much @stamblerre!

@myitcv myitcv closed this as completed Jan 20, 2020
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Jul 22, 2020
@golang golang locked and limited conversation to collaborators Jul 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
gopls/v0.4.0
Development

No branches or pull requests
3 participants
@myitcv @stamblerre @gopherbot