golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.31k stars 17.58k forks source link

x/tools/gopls: provide function that gives list of candidate imports matching pattern #32749

Open myitcv opened 5 years ago

myitcv commented 5 years ago

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

$ go version
go version devel +44c9354c5a Fri Jun 21 05:21:30 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20190620191750-1fa568393b23
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20190620191750-1fa568393b23

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-build670036660=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Much like gopls offers completion candidates, it should provide a function that allows editors to tab-complete/fuzzy match/whatever packages to import.

For example, in govim we want to provide the command GOVIMAddImport such that when we type:

:GOVIMAddImport en<Tab>

(where <Tab> is us looking to complete the import), we should be able to call gopls and get a list of import candidates matching en (according to some algorithm)

Reference: https://github.com/myitcv/govim/issues/317


cc @stamblerre @ianthehat

stamblerre commented 5 years ago

I think this fits into the functionality covered by https://github.com/golang/go/issues/31906, or is there anything else that we should add to that issue?

myitcv commented 5 years ago

@stamblerre - I think they're different. In this case I'm looking to provide a command that can be run in normal mode (which is the mode in which you can issue commands to Vim).

:GOVIMAddImport en<Tab>

Reason being, not everyone does/will think that completion is the way to add an import.

But also because this will likely become the primary way of adding named imports, e.g.:

import qt "github.com/frankban/quicktest"

Would be added via:

:GOVIMAddImport github.com/frankban/quicktest=qt

or similar.

stamblerre commented 5 years ago

What LSP request would this command map to?

myitcv commented 5 years ago

That's really part of the reason for me raising the issue. The short answer is: I don't know šŸ˜„

Is there not any scope for an LSP implementation providing additional methods, beyond the spec?

stamblerre commented 5 years ago

Not that I'm aware of, though we can certainly raise this with https://github.com/microsoft/language-server-protocol. What's the justification behind users wanting to manually add imports rather than using goimports-type behavior?

myitcv commented 5 years ago

As i mentioned in https://github.com/golang/go/issues/32749#issuecomment-505129525, because this will be the primary (only?) way of adding a named import.

stamblerre commented 5 years ago

My best guess for how this could work with the current state of gopls is that you automatically add an import using a quick fix, and then you rename it (I believe @suzmue intends to support that behavior, but it hasn't been added yet). govim would have to do the task of combining those 2 together.

myitcv commented 5 years ago

But this won't work where you already have an identifier in scope which has the name of the package being imported, which is exactly why I'd be looking to have a named import.

myitcv commented 5 years ago

Another case where this is required (just stumbled across it again, which reminded me): where you import a package for its side effects.

suzmue commented 5 years ago

It seems like it would require a change to the lsp spec to provide the exact function you are looking for, but there are a few ways I could see something similar potentially fitting in to gopls:

  1. autocomplete in import spec (import qt "gith<>)
  2. try to resolve imports by ignoring package name and provide the renamed package import (fix: Add import qt "github.com/pkg")
  3. Provide multiple quick fixes to choose from that would satisfy the missing import.

With finer control about what import fixes are applied, which we are working on supporting, these seem like things that would be possible.

myitcv commented 5 years ago

@suzmue - the description talks about providing a Vim command. Vim operates in different modes. Completion is possible in insert mode, at the point of the cursor. Commands operate in normal mode: the cursor position is not within document for such commands.

In any case, the intention is to call such a command when elsewhere in the code; i.e. I don't want to have to jump up to the import declarations to perform a completion on an import, only to then jump back.

Making a request to the LSP spec team sounds like a big step for something that I would think is better suited to an experiment for now. Is there a means by which we can extend the API of gopls for experimental methods such as this?

bhcleek commented 5 years ago

@myitcv vim-go provides this kind of functionality. I can help you understand how it works if needed.

myitcv commented 5 years ago

Thanks @bhcleek - understanding how to do this is not where I'm stuck šŸ˜„. It's that I think this logic belongs in gopls, which is ultimately the source of truth for questions about syntax, types, packages, modules etc.

Of course each editor (plugin) could implement this sort of thing, but that kind of defeats the point of a language server to my mind.

As I mentioned in https://github.com/golang/go/issues/32749#issuecomment-508566031, there are likely to be ideas/requirements like this that fall outside of the LSP spec. It makes sense to experiment with these at different levels:

  1. in editor (plugins), if possible
  2. gopls via a side-API, not part of the LSP spec
  3. proposing changes to the LSP spec

For this particular requirement, because it is not covered by the LSP spec, I'm suggesting we explore avenue 2, as a test case if you will.

stamblerre commented 5 years ago

We have not yet considered a plan for features outside of LSP in gopls, and I don't expect that will be a high priority for us in the near future. However, this is definitely a possibility we should consider.

Still, I'm not convinced that this feature is necessarily one that we need to provide separately from existing gopls features, since it is effectively a combination of quick fix import + rename.

In the meantime, I would definitely consider filing an issue against the LSP specification, as other languages may also be interested in this type of feature. It takes a while to make changes in the LSP spec, so definitely worth filing the issue, even if its not something they're interested in doing.

myitcv commented 4 years ago

On the back of some work to add Symbol support to govim (which incidentally gave rise to #38273 and #37237) it just struck me that the Symbol method can give us exactly what we need here.

e.g. using the proposed advanced query syntax:

kind::4 ^encoding

where:

would return the results:

json
hex
...