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: unimported completions not (consistently) offered #38764

Closed
myitcv opened this issue Apr 30, 2020 · 10 comments
Closed

x/tools/gopls: unimported completions not (consistently) offered #38764

myitcv opened this issue Apr 30, 2020 · 10 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Apr 30, 2020

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

$ go version
go version devel +b1b67841d1 Tue Apr 28 21:46:16 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200430040329-4b814e061378
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20200430040329-4b814e061378

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=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
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-build754553919=/tmp/go-build -gno-record-gcc-switches"

What did you do?

These steps are reduced from a more complex example:

export GOPATH=$(mktemp -d) 
cd $(mktemp -d) 
go mod init example.com/hello
go get -d github.com/rogpeppe/go-internal@v1.5.2
go get -d cuelang.org/go@v0.1.3-0.20200424192631-12927e83d318
go mod download
echo -e "package main\n\nfunc main() {\n\tpar\n}" > main.go

Then attempt completion after par in the main.go file

What did you expect to see?

Offered completion candidates for:

parse  "text/template/parse"
parser "go/parser"
par    "github.com/rogpeppe/go-internal/par"
parser "cuelang.org/go/cue/parser"

What did you see instead?

During repeated calls to completion, most of the time I only saw:

parse  "text/template/parse"
parser "go/parser"

Sometimes:

parse  "text/template/parse"
parser "go/parser"
par    "github.com/rogpeppe/go-internal/par"

But the cuelang.org/go/cue/parser package was never a candidate.

See the attached log file for the sequence of events: gopls_20200430_1402_02_716145697.log


cc @stamblerre @heschik

FYI @leitzler

@gopherbot gopherbot added this to the Unreleased milestone Apr 30, 2020
@gopherbot gopherbot added the Tools label Apr 30, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Apr 30, 2020
@heschik
Copy link
Contributor

@heschik heschik commented Apr 30, 2020

I can't reproduce this. There should be a total of 5 completion results (https://cs.opensource.google/go/tools/+/master:internal/lsp/source/completion.go;drc=8463f397d07cfd2b3f1442fb1daa5e6bc2178a6e;l=743) arbitrarily chosen from any packages that match the prefix. I often see the cue parser package.

@myitcv
Copy link
Member Author

@myitcv myitcv commented Apr 30, 2020

I can reproduce this consistently locally. Is there anything I can do to help debug?

arbitrarily chosen from any packages that match the prefix

I think we should improve this logic to prefer those packages in modules "closest" to the go.mod requirements, but that's perhaps a separate matter.

@heschik
Copy link
Contributor

@heschik heschik commented Apr 30, 2020

Ranking is #38104.

Actually, I'm confused. I just looked at your log, and I see cuelang.org/go/cue/parser in response 3 and 16.

@myitcv
Copy link
Member Author

@myitcv myitcv commented Apr 30, 2020

Ah, I see what's happening here. This is actually my expectations being wrong because of the random results. Apologies.

Is there any way we can increase the limit from 5? Where does that limit come from?

@heschik
Copy link
Contributor

@heschik heschik commented Apr 30, 2020

It's totally arbitrary, so of course it can be increased. See the description of https://golang.org/cl/218879 for more details. It may be that people are more willing to scroll through a list of completions than we expected, but I still generally think you should just keep typing?

@myitcv
Copy link
Member Author

@myitcv myitcv commented Apr 30, 2020

but I still generally think you should just keep typing?

What if, as is the case here, you have > 5 packages called parser? You can't type any more and instead you are reliant on getting lucky that the one you want appears in the window of 5.

Or did I miss your point?

@heschik
Copy link
Contributor

@heschik heschik commented Apr 30, 2020

You could type parser.Pif you have even the faintest idea that you're looking for a Parse function, and it will give many more results for that.

Feel free to send a CL to increase the 5, but you're probably going to have to explain why you're confident that there won't be 10 (or however many) parser packages.

@myitcv
Copy link
Member Author

@myitcv myitcv commented Apr 30, 2020

You could type parser.Pif you have even the faintest idea that you're looking for a Parse function, and it will give many more results for that

Doesn't that only work if you have deep completions turned on?

@myitcv
Copy link
Member Author

@myitcv myitcv commented May 1, 2020

Doesn't that only work if you have deep completions turned on?

Turns this works without deep completions.

So I think the only open question is whether we want to make the logic around returning 5 random results for the par example above smarter.

@heschik
Copy link
Contributor

@heschik heschik commented May 1, 2020

Not without a compelling rationale. Closing.

@heschik heschik closed this May 1, 2020
@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.1 May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.