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

Completion returns wrong candidates due to inconsistent file path usage. #2913

Closed
egravert opened this issue Jun 8, 2020 · 7 comments · Fixed by #2914
Closed

Completion returns wrong candidates due to inconsistent file path usage. #2913

egravert opened this issue Jun 8, 2020 · 7 comments · Fixed by #2914

Comments

@egravert
Copy link

egravert commented Jun 8, 2020

Hi all

I'm running into an issue where the completion menu in vim shows unexpected results. I enabled completion within my vimrc to show contextual candidates when I press period (see vimrc below).
Unfortunately, the candidates are almost always wrong.

After tracing through gopls I was eventually able to track down the issue, but need further assistance on how to best fix it.

For context, this is on FreeBSD using zfs for the filesystem. This is relevant due to how the default filesystem is setup. For FreeBSD, /home is a symlink to /usr/home.

By tracing gopls, this is what appears to be happening:

  1. Vim Starts, and vim-go initializes gopls:
[Trace - 12:49:43.694 PM] Sending request 'initialize - (1)'.
Params: {
  "rootUri":"file:///home/messorian/src/messorian/gopls-demo",
  "capabilities": {
                    "workspace":{"workspaceFolders":true,"configuration":true,"didChangeConfiguration":{"dynamicRegistration":true}},
                    "textDocument":{
                      "codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["source.organizeImports"]}}},
                      "completion":{"completionItem":{"snippetSupport":false}},"hover":{"contentFormat":["plaintext"]}
                    }
                  },
  "processId":64400,
  "workspaceFolders":[{
                        "uri":"file:///home/messorian/src/messorian/gopls-demo",
                        "name":"/home/messorian/src/messorian/gopls-demo"
                      }]
}

Note that workspaceFolders and rootUri are both set to /home/

Gopls uses this event to initialize its cache. When files are added to the cache, the file URI is tracked, and is based on the rootUri that was provided by this event.

  1. The developer edits the file
[Trace - 12:49:45.164 PM] Sending notification 'textDocument/didChange'.
Params: {
  "contentChanges":[{ "text":"package main\n\nimport \"fmt\"\n\nfunc main() {\n\tfmt.Println(\"go-vim\")\n\tfmt.Println(\"welcome\")\n\tfm\n}\n"}],
  "textDocument": {
                    "uri": "file:///usr/home/messorian/src/messorian/gopls-demo/main.go",
                    "version":2
                  }
}

The uri provided now points to /usr/home instead.
gopls adds the uri to the file's uri list (See here: fileBase.addURI)

  1. The developer triggers the completion menu (period in my case, ctrl+x,ctrl+o otherwise):
[Trace - 12:49:45.307 PM] Sending request 'textDocument/completion - (4)'.
Params: {
  "textDocument":{
            "uri":"file:///usr/home/messorian/src/messorian/gopls-demo/main.go"},
             "position":{"character":5,"line":7}}

While the uri points to the uri of the last change, gopls is hardcoded to only return the URI of the first URI added to the list (see here).

This results in gopls returning the wrong text document when looking for the identifier closest to the cursor, causing the wrong result to be returned.

I see two way to fix this:

  1. Update vim-go to pass consistent paths to gopls. Either always using the relative path, or always following the symlink (Sorry if I used the wrong terms here).
  2. Update gopls to somehow handle resolving multiple paths to the same file.

For now, I have worked around the issue by hacking gopls to have URI() always return the last added uri.

What did you do? (required: The issue will be closed when not provided)

  1. Open a go source file containing the following code on FreeBSD (already saved to disk):
package main
 
import "fmt"

func main() {
    fmt.Println("go-vim")
}
  1. Add a new line after fmt.Println("go-vim")
  2. type fmt.

What did you expect to happen?

I would expect vim to show the fmt package's exported types in the completion window.

What happened instead?

Only keywords are show.

Configuration (MUST fill this out):

vim-go version:

v1.23

vimrc you used to reproduce:

vimrc
  set nocompatible              " be iMproved, required

execute pathogen#infect()

syntax on
filetype plugin indent on " required

let g:go_fmt_fail_silently = 1
let g:go_fmt_command = "goimports"

let g:go_highlight_functions = 1
let g:go_highlight_methods = 1
let g:go_highlight_structs = 1
let g:go_highlight_interfaces = 1
let g:go_highlight_operators = 1
let g:go_highlight_build_constraints = 1

let g:go_auto_type_info = 1
let g:go_auto_sameids = 1

let g:go_def_mode='gopls'
let g:go_info_mode='gopls'

" enable autocomplete popup on period for go files
au filetype go inoremap . .
set completeopt=menuone,longest

Vim version (first three lines from :version):

  VIM - Vi IMproved 8.2 (2019 Dec 12, compiled May 30 2020 17:48:31)                                                                                                                                              
  Included patches: 1-491                                                                                                                                                                                     
  Compiled by root@121amd64-quarterly-job-11                                                                                                                                                                  
  Huge version with GTK3 GUI.  Features included (+) or not (-):

Go version (go version):

go version go1.14.3 freebsd/amd64

Go environment

go env Output:
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/messorian/.cache/go-build"
GOENV="/home/messorian/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="freebsd"
GOPATH="/home/messorian/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/freebsd_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/home/messorian/src/messorian/gopls-demo/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build970947096=/tmp/go-build -gno-record-gcc-switches"

gopls version

gopls version Output:
golang.org/x/tools/gopls 0.4.1
    golang.org/x/tools/gopls@v0.4.1 h1:0e3BPxGV4B3cd0zdMuccwW72SgmHp92lAjOyxX/ScAw=

@bhcleek
Copy link
Collaborator

bhcleek commented Jun 8, 2020

How did you open the file identified in your step 2, file:///usr/home/messorian/src/messorian/gopls-demo/main.go?

vim-go good about using the paths you give it AFAIK. And given that the rootUri and workspaceFolders.uri use /home, I suspect the root cause here is one of these two possibilities:

  1. You edited the file by opening it with the /usr/home path.
  2. You executed a command that caused vim-go to open a file (e.g. :GoDef or similar), and gopls followed a symlink to its real path and returned that resolved path to vim-go.

It would be helpful if you could provide some clear steps for how to duplicate this in a simple scenario and also if you could provide the LSP debug output by adding g:go_debug=['lsp'] to your vimrc.

@egravert
Copy link
Author

egravert commented Jun 9, 2020

Hi @bhcleek thanks for the help!

I'm starting vim from the current working directory using vim main.go. The file already exists with the default vim-go starter template. I captured a video of the behavior and the debug output for you as well.

Video
vim-go

Log
vim-go-debug.log

@bhcleek
Copy link
Collaborator

bhcleek commented Jun 9, 2020

Thank you @egravert I think the we'll be able to get to the bottom of this pretty quickly. The screen recording and the lsp debug log will help immensely.

The paths being different for the file vs the rootUri is likely an issue that vim-go can resolve.

I haven't yet been able to duplicate the completion candidates that you're seeing, but I'll take a thorough look at the log you provided to see if it offers any clues. In the meantime, can remove your . mapping and see if you get the same result with the usual C-x,C-o key combination to trigger completion?

@bhcleek
Copy link
Collaborator

bhcleek commented Jun 9, 2020

I can duplicate the completion issue. There's no need for you to test without the . mapping. I'll see what I can do to resolve this or whether it's something about gopls itself.

@bhcleek
Copy link
Collaborator

bhcleek commented Jun 9, 2020

I don't think there's a solution for this on the vim-go side. GOPATH with a symlink has similar kinds of issues. The best I could do would be to open the go.mod file, expand it to its real path, and then use that as the working directory.

Ideally, that wouldn't be necessary, because Vim would be able to expand the path with a symlink in it, but Vim doesn't have an option to be able to do that: expanding file paths always provides the real path.

Even if we could handle this particular use case, the community tooling wouldn't support symlinks generally and we'd be left with a variety of related issues. I'll refer you to previous discussions where vim-go users have had questions about symlinks and our answers there (e.g. #2731).

I think your best option maybe to work in /usr/home instead of /home.

bhcleek added a commit to bhcleek/vim-go that referenced this issue Jun 9, 2020
Resolve the module root to the true path. All other path expansions in
Vim are the resolved path, so the module root path should also be the
resolved path.

Fixes fatih#2913
@bhcleek
Copy link
Collaborator

bhcleek commented Jun 9, 2020

I found an easy solution for this after thinking about it some more and considering the options and the tradeoffs.

Thank you for a detailed issue report @egravert !

bhcleek added a commit that referenced this issue Jun 9, 2020
@egravert
Copy link
Author

egravert commented Jun 9, 2020

@bhcleek Thank you for the amazing help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants