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

vim-go highlighting of gopls diagnostics not updating on write #2753

Closed
miscing opened this issue Mar 1, 2020 · 20 comments · Fixed by #2779
Closed

vim-go highlighting of gopls diagnostics not updating on write #2753

miscing opened this issue Mar 1, 2020 · 20 comments · Fixed by #2779

Comments

@miscing
Copy link

miscing commented Mar 1, 2020

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

Highlighting seems to work incorrectly, cannot think of a way reproduce issue other than stating that highlighting does not update on ":w" since last update. For some reason quitting and re-entering file does work. Writing to file fails to pickup both new errors and remove fixed errors

What did you expect to happen?

Highlighting to disappear as per usual behavior before update. Also new syntax errors should highligh automatically after a write to file. Now requires exiting and re-entering.

What happened instead?

Incorrect highlighting persists unless one exits and re-opens vim.

Configuration (MUST fill this out):

vim-go version:

v.1.22

vimrc you used to reproduce:

vimrc
call plug#begin()
Plug 'majutsushi/tagbar' "F7 toggles ctag bar
Plug 'fatih/vim-go', { 'do': ':GoUpdateBinaries' } "GO vim stuff, tutorial in same dudes github

" Activate features
Plug 'tpope/vim-commentary' "gc{motion} to comment out, gcc=comment line
Plug 'tpope/vim-surround' "Surround stuff with stuff
Plug 'ctrlpvim/ctrlp.vim' "fuzzy search firectory
" List ends here. Plugins become visible to Vim after this call.
call plug#end()

set number "line numbers
set mouse=a "mouse support

syntax on
colorscheme molokai

set list
set listchars=tab:|·,trail:_,extends:>,precedes:<,nbsp:~

Vim version (first three lines from :version):

NVIM v0.4.3
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3

Go version (go version):

go1.13.6 linux/amd64

Go environment

go env Output:

gopls version

gopls version Output:
golang.org/x/tools/gopls v0.3.2
@bhcleek
Copy link
Collaborator

bhcleek commented Mar 1, 2020

Can you be more specific? I'll need a way to replicate what you're seeing if this has any chance of getting fixed.

A few things I'mo wondering:

  • does this have anything to do with whether or not saving adds imports (e.g. if you g:go_fmt_command is goimports and there's a missing import.)
  • Does this happen with both Vim and Neovim?
  • Does this happen on current master?
  • Does writing the file twice have any effect?

@davidlubomirov
Copy link

davidlubomirov commented Mar 1, 2020

I have the exact same issue.
After an update of all binaries:GoUpdateBinaries, now after saving a file, errors are not highlighting.

How I caught the issue:

  1. I'm opening a file with 2 errors, both of the same time - call to non-existing function in a package
  2. After fix on the first highlighted error and save the file:w, the second highlight is disappearing

I've tried running all type of commands in order to get it working. But the highlighting is not showing.
The only way to resolve it - close and re-open VIM, but again it's working only until I save a file :w

@bhcleek, I can answer the following:

  • Does this happen with both Vim and Neovim? - Yes
  • Does writing the file twice have any effect? - Yes

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 2, 2020

I'm having a hard time duplicating what's being described here. Here's a quick animated gif from my machine of everything working as expected. What am I doing wrong to duplicate what you're describing?

recording

@miscing
Copy link
Author

miscing commented Mar 2, 2020

govim-issue

Here is the issue. With some further testing it seems it is only broken in my go/src directory, any go file outside it works normally. No idea why.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 2, 2020

@miscing I can't duplicate this in $GOPATH either.

Does this problem still exist for you on master?

Also, can you run vim with let g:go_debug=['lsp'] and provide the output of the gopls debug window after saving the file?

@miscing
Copy link
Author

miscing commented Mar 2, 2020

It still persists on master.

debug output:
sent: Content-Length: 288^M^@^M^@{"method": "textDocument/didChange", "jsonrpc": "2.0", "params": {"contentChanges": [{"text": "package main\n\nimport "fmt"\n\nfunc main() {\n\n\t// hello := "hello"\n\tfmt.Println(hello)\n}\n"}], "textDocument": {"uri": "file:///[REMOVED:GOPATH]/go/src/hello/main.go", "version": 6}}}

sent: Content-Length: 288^M^@^M^@{"method": "textDocument/didChange", "jsonrpc": "2.0", "params": {"contentChanges": [{"text": "package main\n\nimport "fmt"\n\nfunc main() {\n\n\t// hello := "hello"\n\tfmt.Println(hello)\n}\n"}], "textDocument": {"uri": "file:///[REMOVED:GOPATH]/go/src/hello/main.go", "version": 7}}}

go pls check works fine inside GOPATH.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 2, 2020

There should be a lot more output than that.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 2, 2020

That log doesn't show any publishDiagnostics notifications from gopls. Did you fully replicate the issue in the Vim session that that log is from?

@miscing
Copy link
Author

miscing commented Mar 2, 2020

govim-issue.txt

Another attempt. Thanks for the patience.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 2, 2020

@miscing those logs show that the diagnostics were cleared after your edits.

The first diagnostic that identified the problematic code:

received: Content-Length: 298
�
�{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file://REMOVED/go/src/hello/main.go","version":1,"diagnostics":[{"range":{"start":{"line":6,"character":13},"end":{"line":6,"character":18}},"severity":1,"source":"compiler","message":"undeclared name: hello"}]}}

followed by writing your changes:

sent: Content-Length: 285
�
�{"method": "textDocument/didChange", "jsonrpc": "2.0", "params": {"contentChanges": [{"text": "package main\n\nimport \"fmt\"\n\nfunc main() {\n\n\thello := \"hello\"\n\tfmt.Println(hello)\n}\n"}], "textDocument": {"uri": "file://REMOVED/go/src/hello/main.go", "version": 2}}}
sent: Content-Length: 285
�
�{"method": "textDocument/didChange", "jsonrpc": "2.0", "params": {"contentChanges": [{"text": "package main\n\nimport \"fmt\"\n\nfunc main() {\n\n\thello := \"hello\"\n\tfmt.Println(hello)\n}\n"}], "textDocument": {"uri": "file://REMOVED/go/src/hello/main.go", "version": 3}}}

followed by the diagnostics that show that there are no errors:

received: Content-Length: 153
�
�{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file://REMOVED/go/src/hello/main.go","version":2,"diagnostics":[]}}

I'm still unable to duplicate what you're describing. The last screen recording you posted doesn't show the command you're typing, which is what I'd need to be able to see to understand when the file is being written so that I can correlate that with what's highlighted.

@davidlubomirov
Copy link

@bhcleek

I've created a small project outside GOPATH.

NOTE: I have other projects defined outside GOPATH, but this problem never showed before the binaries update I did couple of days ago.

@miscing
Copy link
Author

miscing commented Mar 3, 2020

github.com/miscing/govim-highlighting-issue

Did the same, one directory has gif of issue inside gopath and other gif of no issue outside gopath.
Used nvim commands and vim config file included.

gif of issue and logs included here too:
issue
issue-logs.txt

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 3, 2020

Thank you for your patience.

I can duplicate this now and will have a fix soon.

bhcleek added a commit that referenced this issue Mar 5, 2020
@bhcleek
Copy link
Collaborator

bhcleek commented Mar 5, 2020

A quick progress report:

I'm making headway, but am currently fighting getting some useful tests working so I can validate my fixes; we have to be careful with this to avoid introducing a regression.

In the meantime, you might consider either disabling the gopls diagnostic highlighting entirely or going back to revision that doesn't have this problem (252602d ?) or v1.22.

@davidlubomirov
Copy link

@bhcleek I guess only checkout to v1.22 is not enough.

What else I need to execute ?

@bhcleek bhcleek changed the title vim-go highlighting not updating on write vim-go highlighting of gopls diagnostics not updating on write Mar 6, 2020
@bhcleek
Copy link
Collaborator

bhcleek commented Mar 6, 2020

@davidlubomirov That should be enough, but you have to check it out to the right place. Without knowing how you manage your Vim plugins, it's impossible for me to say precisely what you need to do.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 7, 2020

I've made progress on the tests, so I'll be able to focus on implementing a fix.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 14, 2020

The solution for this issue was delayed, because my computer took a dive shortly after my last progress update.

I have good tests and am working on a fix, but I am having a hard time getting one in place. Interestingly, I can duplicate the issue pretty easily. I'm trying to figure out if this is a bug in Vim or not. Either way, using gopls for g:go_fmt_command helps avoid this issue, but comes with a tradeoff: :GoImports has to be called explicitly when g:go_fmt_command is set to gopls, because it doesn't adjust the imports yet.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 14, 2020

I have the tests passing. Now I need to write one more to deal with another situation that the code was trying to workaround when formatting or imports are changed on write.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 19, 2020

I'm finally nearly done with this. It was surprisingly tricky to get right, and I suspect there is still an edge case that I'm going to try to verify and hopefully resolve before I merge the PR after I put it up.

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.

3 participants