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/internal/lsp: handle module paths below root path #29174

Closed
eliasnaur opened this issue Dec 11, 2018 · 13 comments
Closed

x/tools/internal/lsp: handle module paths below root path #29174

eliasnaur opened this issue Dec 11, 2018 · 13 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls.
Milestone

Comments

@eliasnaur
Copy link
Contributor

eliasnaur commented Dec 11, 2018

I'm trying to get golsp to work with vim through https://github.com/autozimu/LanguageClient-neovim.

Golsp seems to work very well for me when editing Go source code in GOPATH. However, when editing Go source inside a module, golsp reports

"no packages found for </path/to/gofile.go>"

when I attempt the "go to definition" command.

My project structure resembles this template:

project/
 .git/
 src/
   example.com/
     go.mod
     source.go

It seems that the problem is a mismatch between the language server "project path" and the go module path.

The project path is sent through the "initialize" lsp request in the "rootPath" and "rootUri" fields by LanguageClient-neovim. LanguageClient-neovim also starts the golsp executable with the project path as its current directory. Its unclear exactly how the project path is determined, but it seems to be the root of the git project containing the Go source file I'm editing. In my case, "project/".

Now, when golsp attempts to determine the packages containing for the open Go source file it fails because

pkgs, err := packages.Load(v.Config, fmt.Sprintf("file=%s", path))

(from https://github.com/golang/tools/blob/6a3e9aa2ab7749d72d1006ee484271b4a11f96c2/internal/lsp/cache/view.go#L63)

fails to find the current module in golsp's current working directory, "project/".

To test my hypothesis, I added

Dir: "</path/to/project/src/example.com>"

to the packages.Config used above and sure enough, "go to definition" and other lsp features started working again.

I'm sorry if this is a known issue. If so, I hope this issue can be a placeholder for resolving it.

CC @stamblerre.

@gopherbot gopherbot added this to the Unreleased milestone Dec 11, 2018
@stamblerre
Copy link
Contributor

thanks for reporting this @eliasnaur! This is definitely the correct fix; I think none of us just got around to fixing it because we are working in $GOPATH mode. If you'd like to contribute the fix, please go ahead, otherwise I'll be happy to fix it.

@gopherbot
Copy link

Change https://golang.org/cl/153777 mentions this issue: internal/lsp,internal/lsp/cache: find module roots

@myitcv
Copy link
Member

myitcv commented Dec 12, 2018

@eliasnaur - very interesting, we were talking about editors gaining LSP capabilities on yesterday's golang-tools call.

If it's ok with you I'll cc you on a thread I'd planned to kick off about Vim/Neovim integration with the LSP?

@eliasnaur
Copy link
Contributor Author

eliasnaur commented Dec 12, 2018 via email

@gopherbot
Copy link

Change https://golang.org/cl/153863 mentions this issue: internal/lsp: use rootURI as config.Dir in packages.Load

@eliasnaur
Copy link
Contributor Author

eliasnaur commented Dec 12, 2018

Thanks for the CL, but it doesn't fix the problem I reported: the LanguageClient-neovim vim plugin will pass my git project root as rootUri; the module is below the git project root and golsp will fail to locate a package. Who's at fault, golsp or LanguageClient-neovim?

@eliasnaur eliasnaur reopened this Dec 12, 2018
@ianthehat
Copy link

I think it has to be vim plugin, that changes. Using the directory of the file is wrong in too many cases (the most important and common of which is when the file is one from the module cache)
For now, pending more discussions about how editors can describe intent to the lsp, I think the only reasonable thing we can do is trust the rootUri and ask the editors to set it to a sane value. As far as golsp is concerned the rootUri is the directory of the go.mod file.

@eliasnaur
Copy link
Contributor Author

But changing the vim plugin seems to go against the stated goal of the language server protocol: reducing an M*N problem to M + N. Case in point, LanguageClient-neovim doesn't know about golsp or any other particular language server; I mapped go files to the golsp server in my vim configuration.
Furthermore, it seems odd to me that the rootUri is established at the "initialize" request, but a given workspace (git project) might very well contain multiple modules. If so, is the lsp client supposed to re-initialize its lsp server for every file in a different module? Or have a server running for each Go module?

In other words, are you suggesting that every lsp client, including the multiple clients for vim alone, must include Go specific logic to determine rootUri?
And even if your answer is yes, I think it is worth having a fallback in golsp for a (from the point of view of golsp) "wrong" rootUri. I think a "no packages found for </path/to/gofile.go>" is giving up too easily.

@myitcv
Copy link
Member

myitcv commented Dec 13, 2018

I think it has to be vim plugin, that changes

There's a bit of nuance behind this comment given conversations we've had in the past in golang-tools's various channels.

VSCode and Atom are at something of an advantage in this respect because they have the concept of opening a project/directory. Chances are that directory corresponds to root of the main module, but even that's not bullet proof for multi-module repos/sub modules (something that came up on the last call - notes to follow today).

To my knowledge, the closest Vim gets is the concept of a current directory. There is a current directory for the Vim instance itself, then optionally a value per window (if overridden). Arguably, the current directory could be used as the root (more specifically the value that gets set in go/packages.Config.Dir). However, I suspect trying to "overload" this will be problematic because there are likely other vim features/plugins that rely heavily on the current directory, not to mention people's work flows (thinking of autochdir and friends).

The furthest I've got on thinking about this is Vim gaining some sort of state, akin to the current directory, that includes the main module path. Again, this state should be settable at the Vim instance level, as well as locally, per window.

I say "includes the main module path", because I think this state also needs to include:

  • GOOS
  • GOARCH
  • Build tags

All of these need to be set by the editor for calls through the LSP to be successful (think about working on a WASM project, the LSP would otherwise have no idea you mean GOOS=js GOARCH=wasm).

So when @ianthehat said:

I think it has to be vim plugin, that changes

I suspect (please correct me if I'm wrong) this was more a comment along the lines of "this is something that needs to be set/sent/etc from the editor as opposed to determined/etc in the LSP. Whether it happens in the editor's LSP plugin or some other plugin/config is I think to be worked out.

So I think this is more a conversation about editor context than anything in the LSP. Not sure whether it's best to repurpose this issue/switch conversation elsewhere.

cc @bhcleek as an FYI on the Vim points

@stamblerre stamblerre added gopls Issues related to the Go language server, gopls. and removed gopls Issues related to the Go language server, gopls. labels Mar 12, 2019
@fsouza
Copy link
Contributor

fsouza commented Mar 13, 2019

@eliasnaur off-topic, but you can fix this in LanguageClient-neovim using a configuration parameter (use go.mod as the root marker for Go projects). See g:LanguageClient_rootMarkers.

@benweissmann
Copy link

To add another data point: I'm using VSCode and working on a project that contains multiple Go modules. gopls works great as long as I open one of the modules directly, but if I open the parent folder (which it not itself a Go modules, but has sub-directories that are Go modules) then gopls doesn't work (can't use go-to-definition, etc.). It also doesn't work if I add the modules separately as top-level folders in the workspace (using File -> Add Folder To Workspace) -- gopls works with the first folder in the workspace but not the others. If I open up a separate VSCode window for each module, it does work.

@stamblerre
Copy link
Contributor

Duplicate of #30841

@stamblerre stamblerre marked this as a duplicate of #30841 Apr 17, 2019
@stamblerre
Copy link
Contributor

More discussion seems to be happening on #30841, closing in favor of that issue

@golang golang locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls.
Projects
None yet
Development

No branches or pull requests

7 participants