Repository: autozimu/LanguageClient-neovim (language: rust)

Issue:
vim8 gutter is not cleared correctly
- Did you upgrade to latest plugin version? yes
- Did you upgrade to/compile latest binary? Run shell command
  `bin/languageclient --version` to get its version number. `languageclient 0.1.155`
- (Neovim users only) Did you check output of `:checkhealth LanguageClient`?
- Did you check [troubleshooting]?

[troubleshooting]: https://github.com/autozimu/LanguageClient-neovim/blob/next/INSTALL.md#6-troubleshooting

## Describe the bug
The sign buffer is occasionally not cleared correctly resulting in signs showing when the language server reports no errors.

## Environment
- neovim/vim version (`nvim --version` or `vim --version`):
```
VIM - Vi IMproved 8.1 (2018 May 18, compiled Nov  7 2019 22:30:47)
Included patches: 1-2268
Compiled by Arch Linux
Huge version with GTK3 GUI.  Features included (+) or not (-):
+acl               -farsi             -mouse_sysmouse    -tag_any_white
+arabic            +file_in_path      +mouse_urxvt       +tcl/dyn
+autocmd           +find_in_path      +mouse_xterm       +termguicolors
+autochdir         +float             +multi_byte        +terminal
-autoservername    +folding           +multi_lang        +terminfo
+balloon_eval      -footer            -mzscheme          +termresponse
+balloon_eval_term +fork()            +netbeans_intg     +textobjects
+browse            +gettext           +num64             +textprop
++builtin_terms    -hangul_input      +packages          +timers
+byte_offset       +iconv             +path_extra        +title
+channel           +insert_expand     +perl/dyn          +toolbar
+cindent           +job               +persistent_undo   +user_commands
+clientserver      +jumplist          +postscript        +vartabs
+clipboard         +keymap            +printer           +vertsplit
+cmdline_compl     +lambda            +profile           +virtualedit
+cmdline_hist      +langmap           +python/dyn        +visual
+cmdline_info      +libcall           +python3/dyn       +visualextra
+comments          +linebreak         +quickfix          +viminfo
+conceal           +lispindent        +reltime           +vreplace
+cryptv            +listcmds          +rightleft         +wildignore
+cscope            +localmap          +ruby/dyn          +wildmenu
+cursorbind        +lua/dyn           +scrollbind        +windows
+cursorshape       +menu              +signs             +writebackup
+dialog_con_gui    +mksession         +smartindent       +X11
+diff              +modify_fname      +sound             -xfontset
+digraphs          +mouse             +spell             +xim
+dnd               +mouseshape        +startuptime       -xpm
-ebcdic            +mouse_dec         +statusline        +xsmp_interact
+emacs_tags        +mouse_gpm         -sun_workshop      +xterm_clipboard
+eval              -mouse_jsbterm     +syntax            -xterm_save
+ex_extra          +mouse_netterm     +tag_binary        
+extra_search      +mouse_sgr         -tag_old_static    
   system vimrc file: "/etc/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
  system gvimrc file: "/etc/gvimrc"
    user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "~/.vim/gvimrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
    system menu file: "$VIMRUNTIME/menu.vim"
  fall-back for $VIM: "/usr/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK  -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/lib/libffi-3.2.1/include -I/usr/include/harfbuzz -I/usr/include/fribidi -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gio-unix-2.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -pthread  -D_FORTIFY_SOURCE=2  -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -D_REENTRANT  -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1       
Linking: gcc   -L. -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -fstack-protector-strong -rdynamic -Wl,-export-dynamic -Wl,-E -Wl,-rpath,/usr/lib/perl5/5.30/core_perl/CORE  -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -L/usr/local/lib -Wl,--as-needed -o vim   -lgtk-3 -lgdk-3 -lz -lpangocairo-1.0 -lpango-1.0 -lharfbuzz -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0  -lSM -lICE -lXt -lX11 -lXdmcp -lSM -lICE  -lm -ltinfo -lelf -lnsl    -lcanberra  -lacl -lattr -lgpm -ldl   -Wl,-E -Wl,-rpath,/usr/lib/perl5/5.30/core_perl/CORE -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -fstack-protector-strong -L/usr/local/lib  -L/usr/lib/perl5/5.30/core_perl/CORE -lperl -lpthread -ldl -lm -lcrypt -lutil -lc   -L/usr/lib -ltclstub8.6 -ldl -lz -lpthread -lm     
```
- This plugin version (`git rev-parse --short HEAD`): `bfa0c0c`
- This plugin's binary version (`bin/languageclient --version`): `languageclient 0.1.155`
- Minimal vimrc content (A minimal vimrc is the smallest vimrc that could
  reproduce the issue. Refer to an example [here][min-vimrc.vim]):
```vim
call plug#begin('~/.vim/plugged')
" Required for operations modifying multiple buffers like rename.
set hidden

let g:LanguageClient_serverCommands = {
    \ 'c'   : ['clangd'],
    \ 'cpp' : ['clangd'],
    \ }

" vim language client
Plug 'autozimu/LanguageClient-neovim', {
    \ 'branch': 'next',
    \ 'do': 'bash install.sh',
    \ }
call plug#end()
```
- Language server link and version: `clangd version 9.0.0 (tags/RELEASE_900/final)`

## To Reproduce
[video](https://asciinema.org/a/ws99PUo4Yvi76HQbmEnPZ8UKf)

## Current behavior
Diagnostic signs remain when there are no diagnostics to report.

## Expected behavior
The diagnostics should be fully cleared.

## Screenshots
If applicable, add screenshots to help explain your problem.

## Additional context
The issue seems kinda racey as it's not always reproducible. I have yet to reproduce this in `nvim`.


Discussion / hints:
I've had this issue for a long time. Today, I worked around this by clearing all signs, not just the ones that were created by LanguageClient-neovim.
This isn't an ideal approach:

- clearing every single sign would not work well if this was used in combinations with `:make`, Syntastic, or anything else that sets signs
- The root cause might be a race condition which makes this fail to call `:sign unplace <id>` for some of LanguageClient-neovim's own signs. Adding an additional Mutex for any modifications of self.signs and updates of UI methods might help if that really was the root cause.
- This is calling `:sign` more often than is necessary (see above note about race condition)

It might be useful if the maintainers added a mode to always clear signs before adding the new signs

```patch
--- a/src/language_client.rs
+++ b/src/language_client.rs
@@ -2385,7 +2449,7 @@ impl LanguageClient {
         let viewport = self.vim()?.get_viewport(params)?;
 
         // use the most severe diagnostic of each line as the sign
-        let signs_next: Vec<_> = self.update(|state| {
+        let mut signs_next: Vec<_> = self.update(|state| {
             let limit = if let Some(n) = state.diagnosticsSignsMax {
                 n as usize
             } else {
@@ -2433,7 +2497,7 @@ impl LanguageClient {
                 _ => {}
             }
         }
-        for sign in &mut signs_to_add {
+        for sign in &mut signs_next {
             if sign.id == 0 {
                 sign.id = self.update(|state| {
                     state.sign_next_id += 1;
@@ -2442,7 +2506,7 @@ impl LanguageClient {
             }
         }
         self.vim()?
-            .set_signs(&filename, &signs_to_add, &signs_to_delete)?;
+            .set_signs(&filename, &signs_next, &signs_to_delete)?;
         self.update(|state| {
             let signs = state.signs.entry(filename.clone()).or_default();
             // signs might be deleted AND added in the same line to change severity,
--- a/autoload/LanguageClient.vim
+++ b/autoload/LanguageClient.vim
@@ -166,13 +166,11 @@ endfunction
 
 function! s:set_signs(file, signs_to_add, signs_to_delete) abort
     " TODO: Optimize to update sign instead of add + remove sign.
+    execute ':sign unplace * file=' . a:file
     for l:sign in a:signs_to_add
         let l:line = l:sign['line'] + 1
         execute ':sign place ' . l:sign['id'] . ' line=' . l:line . ' name=' . l:sign['name'] . ' file=' . a:file
     endfor
-    for l:sign in a:signs_to_delete
-        execute ':sign unplace ' . l:sign['id']
-    endfor
 endfunction
```
I have just spent some time in neovim and also get this issue.
Same issue in Neovim as well.

Fix patch:
diff --git a/src/language_server_protocol.rs b/src/language_server_protocol.rs
index 35d95faa1..052bffb30 100644
--- a/src/language_server_protocol.rs
+++ b/src/language_server_protocol.rs
@@ -2516,49 +2516,47 @@ impl LanguageClient {
                 .map(|(line, severity)| Sign::new(line, format!("LanguageClient{:?}", severity)))
                 .collect())
         })?;
-        let signs_prev: Vec<_> = self.update(|state| {
-            Ok(state
+        self.update(|state| {
+            let signs_prev: Vec<_> = state
                 .signs
                 .entry(filename.clone())
                 .or_default()
                 .iter()
                 .map(|(_, sign)| sign.clone())
-                .collect())
-        })?;
-        let mut signs_to_add = vec![];
-        let mut signs_to_delete = vec![];
-        let diffs = diff::slice(&signs_next, &signs_prev);
-        for diff in diffs {
-            match diff {
-                diff::Result::Left(s) => {
-                    signs_to_add.push(s.clone());
-                }
-                diff::Result::Right(s) => {
-                    signs_to_delete.push(s.clone());
+                .collect();
+            let mut signs_to_add = vec![];
+            let mut signs_to_delete = vec![];
+            let diffs = diff::slice(&signs_next, &signs_prev);
+            for diff in diffs {
+                match diff {
+                    diff::Result::Left(s) => {
+                        signs_to_add.push(s.clone());
+                    }
+                    diff::Result::Right(s) => {
+                        signs_to_delete.push(s.clone());
+                    }
+                    _ => {}
                 }
-                _ => {}
             }
-        }
-        for sign in &mut signs_to_add {
-            if sign.id == 0 {
-                sign.id = self.update(|state| {
+            for sign in &mut signs_to_add {
+                if sign.id == 0 {
                     state.sign_next_id += 1;
-                    Ok(state.sign_next_id)
-                })?;
+                    sign.id = state.sign_next_id;
+                }
             }
-        }
-        self.vim()?
-            .set_signs(&filename, &signs_to_add, &signs_to_delete)?;
-        self.update(|state| {
+
             let signs = state.signs.entry(filename.clone()).or_default();
             // signs might be deleted AND added in the same line to change severity,
             // so deletions must be before additions
-            for sign in signs_to_delete {
+            for sign in &signs_to_delete {
                 signs.remove(&sign.line);
             }
-            for sign in signs_to_add {
-                signs.insert(sign.line, sign);
+            for sign in &signs_to_add {
+                signs.insert(sign.line, sign.clone());
             }
+            state
+                .vim
+                .set_signs(&filename, &signs_to_add, &signs_to_delete)?;
             Ok(())
         })?;
 


Test patch: