Fix #964 - Remove signs when multiple signs end up on a single line

ESLint --config option causes it to not detect node_modules?

ale_linters/go/gosimple.vim

@@ -6,4 +6,5 @@ call ale#linter#Define('go', {
 \   'executable': 'gosimple',
 \   'command': 'gosimple %t',
 \   'callback': 'ale#handlers#unix#HandleAsWarning',
+\   'output_stream': 'both'
 \})

ale_linters/go/staticcheck.vim

@@ -6,4 +6,5 @@ call ale#linter#Define('go', {
 \   'executable': 'staticcheck',
 \   'command': 'staticcheck %t',
 \   'callback': 'ale#handlers#unix#HandleAsWarning',
+\   'output_stream': 'both'
 \})

autoload/ale/fixers/eslint.vim

@@ -2,22 +2,23 @@
 " Description: Fixing files with eslint.
 
 function! s:FindConfig(buffer) abort
-    for l:filename in [
-    \   '.eslintrc.js',
-    \   '.eslintrc.yaml',
-    \   '.eslintrc.yml',
-    \   '.eslintrc.json',
-    \   '.eslintrc',
-    \   'package.json',
-    \]
-        let l:config = ale#path#FindNearestFile(a:buffer, l:filename)
+    for l:path in ale#path#Upwards(expand('#' . a:buffer . ':p:h'))
+        for l:basename in [
+        \   '.eslintrc.js',
+        \   '.eslintrc.yaml',
+        \   '.eslintrc.yml',
+        \   '.eslintrc.json',
+        \   '.eslintrc',
+        \]
+            let l:config = ale#path#Simplify(l:path . '/' . l:basename)
 
-        if !empty(l:config)
-            return l:config
-        endif
+            if filereadable(l:config)
+                return l:config
+            endif
+        endfor
     endfor
 
-    return ''
+    return ale#path#FindNearestFile(a:buffer, 'package.json')
 endfunction
 
 function! ale#fixers#eslint#Fix(buffer) abort
@@ -30,7 +31,7 @@ function! ale#fixers#eslint#Fix(buffer) abort
 
     return {
     \   'command': ale#node#Executable(a:buffer, l:executable)
-    \       . ' --config ' . ale#Escape(l:config)
+    \       . ' -c ' . ale#Escape(l:config)
     \       . ' --fix %t',
     \   'read_temporary_file': 1,
     \}

autoload/ale/list.vim

@@ -98,10 +98,10 @@ function! s:SetListsImpl(timer_id, buffer, loclist) abort
 
         if g:ale_set_quickfix
             if !ale#list#IsQuickfixOpen()
-                execute 'copen ' . str2nr(ale#Var(a:buffer, 'list_window_size'))
+                silent! execute 'copen ' . str2nr(ale#Var(a:buffer, 'list_window_size'))
             endif
         elseif g:ale_set_loclist
-            execute 'lopen ' . str2nr(ale#Var(a:buffer, 'list_window_size'))
+            silent! execute 'lopen ' . str2nr(ale#Var(a:buffer, 'list_window_size'))
         endif
 
         " If focus changed, restore it (jump to the last window).

autoload/ale/sign.vim

@@ -184,16 +184,6 @@ function! s:GroupLoclistItems(buffer, loclist) abort
     return l:grouped_items
 endfunction
 
-function! ale#sign#SetSignColumnHighlight(has_problems) abort
-    highlight clear SignColumn
-
-    if a:has_problems
-        highlight link SignColumn ALESignColumnWithErrors
-    else
-        highlight link SignColumn ALESignColumnWithoutErrors
-    endif
-endfunction
-
 function! s:UpdateLineNumbers(buffer, current_sign_list, loclist) abort
     let l:line_map = {}
     let l:line_numbers_changed = 0
@@ -224,24 +214,32 @@ function! s:BuildSignMap(current_sign_list, grouped_items) abort
     let l:sign_offset = g:ale_sign_offset
 
     for [l:line, l:sign_id, l:name] in a:current_sign_list
-        let l:sign_map[l:line] = {
-        \   'current_id': l:sign_id,
-        \   'current_name': l:name,
+        let l:sign_info = get(l:sign_map, l:line, {
+        \   'current_id_list': [],
+        \   'current_name_list': [],
         \   'new_id': 0,
         \   'new_name': '',
         \   'items': [],
-        \}
+        \})
 
+        " Increment the sign offset for new signs, by the maximum sign ID.
         if l:sign_id > l:sign_offset
             let l:sign_offset = l:sign_id
         endif
+
+        " Remember the sign names and IDs in separate Lists, so they are easy
+        " to work with.
+        call add(l:sign_info.current_id_list, l:sign_id)
+        call add(l:sign_info.current_name_list, l:name)
+
+        let l:sign_map[l:line] = l:sign_info
     endfor
 
     for l:group in a:grouped_items
         let l:line = l:group[0].lnum
         let l:sign_info = get(l:sign_map, l:line, {
-        \   'current_id': 0,
-        \   'current_name': '',
+        \   'current_id_list': [],
+        \   'current_name_list': [],
         \   'new_id': 0,
         \   'new_name': '',
         \   'items': [],
@@ -250,11 +248,18 @@ function! s:BuildSignMap(current_sign_list, grouped_items) abort
         let l:sign_info.new_name = ale#sign#GetSignName(l:group)
         let l:sign_info.items = l:group
 
-        if l:sign_info.current_name isnot# l:sign_info.new_name
+        let l:index = index(
+        \   l:sign_info.current_name_list,
+        \   l:sign_info.new_name
+        \)
+
+        if l:index >= 0
+            " We have a sign with this name already, so use the same ID.
+            let l:sign_info.new_id = l:sign_info.current_id_list[l:index]
+        else
+            " This sign name replaces the previous name, so use a new ID.
             let l:sign_info.new_id = l:sign_offset + 1
             let l:sign_offset += 1
-        else
-            let l:sign_info.new_id = l:sign_info.current_id
         endif
 
         let l:sign_map[l:line] = l:sign_info
@@ -288,7 +293,7 @@ function! ale#sign#GetSignCommands(buffer, was_sign_set, sign_map) abort
                 let l:item.sign_id = l:info.new_id
             endfor
 
-            if l:info.new_id isnot l:info.current_id
+            if index(l:info.current_id_list, l:info.new_id) < 0
                 call add(l:command_list, 'sign place '
                 \   . (l:info.new_id)
                 \   . ' line=' . l:line_str
@@ -301,12 +306,14 @@ function! ale#sign#GetSignCommands(buffer, was_sign_set, sign_map) abort
 
     " Remove signs without new IDs.
     for l:info in values(a:sign_map)
-        if l:info.current_id && l:info.current_id isnot l:info.new_id
-            call add(l:command_list, 'sign unplace '
-            \   . (l:info.current_id)
-            \   . ' buffer=' . a:buffer
-            \)
-        endif
+        for l:current_id in l:info.current_id_list
+            if l:current_id isnot l:info.new_id
+                call add(l:command_list, 'sign unplace '
+                \   . l:current_id
+                \   . ' buffer=' . a:buffer
+                \)
+            endif
+        endfor
     endfor
 
     " Remove the dummy sign to close the sign column if we need to.
@@ -347,7 +354,19 @@ function! ale#sign#SetSigns(buffer, loclist) abort
     \   l:sign_map,
     \)
 
+    " Change the sign column color if the option is on.
+    if g:ale_change_sign_column_color && !empty(a:loclist)
+        highlight clear SignColumn
+        highlight link SignColumn ALESignColumnWithErrors
+    endif
+
     for l:command in l:command_list
         silent! execute l:command
     endfor
+
+    " Reset the sign column color when there are no more errors.
+    if g:ale_change_sign_column_color && empty(a:loclist)
+        highlight clear SignColumn
+        highlight link SignColumn ALESignColumnWithoutErrors
+    endif
 endfunction

test/fixers/test_eslint_fixer_callback.vader

@@ -13,22 +13,45 @@ Execute(The path to eslint.js should be run on Unix):
   \   'read_temporary_file': 1,
   \   'command':
   \     ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
-  \     . ' --config ' . ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
+  \     . ' -c ' . ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
   \     . ' --fix %t',
   \ },
   \ ale#fixers#eslint#Fix(bufnr(''))
 
-Execute(The eslint fixer with eslint.js should be run with node on Windows):
-  call ale#test#SetFilename('../eslint-test-files/react-app/subdir/testfile.js')
-  let g:ale_has_override['win32'] = 1
+Execute(The lower priority configuration file in a nested directory should be preferred):
+  call ale#test#SetFilename('../eslint-test-files/react-app/subdir-with-config/testfile.js')
 
-  " We have to execute the file with node.
   AssertEqual
   \ {
   \   'read_temporary_file': 1,
-  \   'command': ale#Escape('node.exe') . ' '
-  \     . ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
-  \     . ' --config ' . ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
+  \   'command':
+  \     ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
+  \     . ' -c ' . ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/subdir-with-config/.eslintrc'))
+  \     . ' --fix %t',
+  \ },
+  \ ale#fixers#eslint#Fix(bufnr(''))
+
+Execute(package.json should be used as a last resort):
+  call ale#test#SetFilename('../eslint-test-files/react-app/subdir-with-package-json/testfile.js')
+
+  AssertEqual
+  \ {
+  \   'read_temporary_file': 1,
+  \   'command':
+  \     ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
+  \     . ' -c ' . ale#Escape(simplify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
+  \     . ' --fix %t',
+  \ },
+  \ ale#fixers#eslint#Fix(bufnr(''))
+
+  call ale#test#SetFilename('../eslint-test-files/package.json')
+
+  AssertEqual
+  \ {
+  \   'read_temporary_file': 1,
+  \   'command':
+  \     ale#Escape(simplify(g:dir . '/../eslint-test-files/node_modules/.bin/eslint'))
+  \     . ' -c ' . ale#Escape(simplify(g:dir . '/../eslint-test-files/package.json'))
   \     . ' --fix %t',
   \ },
   \ ale#fixers#eslint#Fix(bufnr(''))

test/sign/test_sign_column_highlighting.vader

@@ -1,4 +1,6 @@
 Before:
+  Save g:ale_change_sign_column_color
+
   function! ParseHighlight(name) abort
     redir => l:output
       silent execute 'highlight ' . a:name
@@ -20,14 +22,34 @@ Before:
   let g:sign_highlight = ParseHighlight('SignColumn')
 
 After:
+  Restore
+
   delfunction ParseHighlight
   call SetHighlight('SignColumn', g:sign_highlight)
   delfunction SetHighlight
   unlet! g:sign_highlight
 
+  sign unplace *
+
+Execute(The SignColumn highlight shouldn't be changed if the option is off):
+  let g:ale_change_sign_column_color = 0
+  let b:sign_highlight = ParseHighlight('SignColumn')
+
+  call ale#sign#SetSigns(bufnr(''), [
+  \ {'bufnr': bufnr(''), 'lnum': 1, 'col': 1, 'type': 'W', 'text': 'x'},
+  \])
+  AssertEqual b:sign_highlight, ParseHighlight('SignColumn')
+
+  call ale#sign#SetSigns(bufnr(''), [])
+  AssertEqual b:sign_highlight, ParseHighlight('SignColumn')
+
 Execute(The SignColumn highlight should be set and reset):
-  call ale#sign#SetSignColumnHighlight(1)
+  let g:ale_change_sign_column_color = 1
+
+  call ale#sign#SetSigns(bufnr(''), [
+  \ {'bufnr': bufnr(''), 'lnum': 1, 'col': 1, 'type': 'W', 'text': 'x'},
+  \])
   AssertEqual 'links to ALESignColumnWithErrors', ParseHighlight('SignColumn')
 
-  call ale#sign#SetSignColumnHighlight(0)
+  call ale#sign#SetSigns(bufnr(''), [])
   AssertEqual 'links to ALESignColumnWithoutErrors', ParseHighlight('SignColumn')

test/sign/test_sign_placement.vader

@@ -266,3 +266,14 @@ Execute(It should be possible to clear signs with empty lists):
 
 Execute(No exceptions should be thrown when setting signs for invalid buffers):
   call ale#sign#SetSigns(123456789, [{'lnum': 15, 'col': 2, 'type': 'W', 'text': 'e'}])
+
+Execute(Signs should be removed when lines have multiple sign IDs on them):
+  " We can fail to remove signs if there are multiple signs on one line,
+  " say after deleting lines in Vim, etc.
+  exec 'sign place 1000347 line=3 name=ALEErrorSign buffer=' . bufnr('')
+  exec 'sign place 1000348 line=3 name=ALEWarningSign buffer=' . bufnr('')
+  exec 'sign place 1000349 line=10 name=ALEErrorSign buffer=' . bufnr('')
+  exec 'sign place 1000350 line=10 name=ALEWarningSign buffer=' . bufnr('')
+
+  call ale#sign#SetSigns(bufnr(''), [])
+  AssertEqual [], ParseSigns()