Close #1494 - Prefer displaying higher severity problems for cursor messages, balloons, and highlights

8 files changed, 156 insertions, 68 deletions

diff --git a/autoload/ale/sign.vim b/autoload/ale/sign.vim
index 1c439bc8..d1139c03 100644
--- a/autoload/ale/sign.vim
+++ b/autoload/ale/sign.vim
@@ -60,55 +60,35 @@ execute 'sign define ALEInfoSign text=' . g:ale_sign_info
 \   . ' texthl=ALEInfoSign linehl=ALEInfoLine'
 sign define ALEDummySign
 
-let s:error_priority = 1
-let s:warning_priority = 2
-let s:info_priority = 3
-let s:style_error_priority = 4
-let s:style_warning_priority = 5
-
 function! ale#sign#GetSignName(sublist) abort
-    let l:priority = s:style_warning_priority
+    let l:priority = g:ale#util#style_warning_priority
 
     " Determine the highest priority item for the line.
     for l:item in a:sublist
-        if l:item.type is# 'I'
-            let l:item_priority = s:info_priority
-        elseif l:item.type is# 'W'
-            if get(l:item, 'sub_type', '') is# 'style'
-                let l:item_priority = s:style_warning_priority
-            else
-                let l:item_priority = s:warning_priority
-            endif
-        else
-            if get(l:item, 'sub_type', '') is# 'style'
-                let l:item_priority = s:style_error_priority
-            else
-                let l:item_priority = s:error_priority
-            endif
-        endif
+        let l:item_priority = ale#util#GetItemPriority(l:item)
 
-        if l:item_priority < l:priority
+        if l:item_priority > l:priority
             let l:priority = l:item_priority
         endif
     endfor
 
-    if l:priority is# s:error_priority
+    if l:priority is# g:ale#util#error_priority
         return 'ALEErrorSign'
     endif
 
-    if l:priority is# s:warning_priority
+    if l:priority is# g:ale#util#warning_priority
         return 'ALEWarningSign'
     endif
 
-    if l:priority is# s:style_error_priority
+    if l:priority is# g:ale#util#style_error_priority
         return 'ALEStyleErrorSign'
     endif
 
-    if l:priority is# s:style_warning_priority
+    if l:priority is# g:ale#util#style_warning_priority
         return 'ALEStyleWarningSign'
     endif
 
-    if l:priority is# s:info_priority
+    if l:priority is# g:ale#util#info_priority
         return 'ALEInfoSign'
     endif
 
diff --git a/autoload/ale/util.vim b/autoload/ale/util.vim
index e0b31e2c..d160b34c 100644
--- a/autoload/ale/util.vim
+++ b/autoload/ale/util.vim
@@ -33,6 +33,32 @@ function! ale#util#GetFunction(string_or_ref) abort
     return a:string_or_ref
 endfunction
 
+let g:ale#util#error_priority = 5
+let g:ale#util#warning_priority = 4
+let g:ale#util#info_priority = 3
+let g:ale#util#style_error_priority = 2
+let g:ale#util#style_warning_priority = 1
+
+function! ale#util#GetItemPriority(item) abort
+    if a:item.type is# 'I'
+        return g:ale#util#info_priority
+    endif
+
+    if a:item.type is# 'W'
+        if get(a:item, 'sub_type', '') is# 'style'
+            return g:ale#util#style_warning_priority
+        endif
+
+        return g:ale#util#warning_priority
+    endif
+
+    if get(a:item, 'sub_type', '') is# 'style'
+        return g:ale#util#style_error_priority
+    endif
+
+    return g:ale#util#error_priority
+endfunction
+
 " Compare two loclist items for ALE, sorted by their buffers, filenames, and
 " line numbers and column numbers.
 function! ale#util#LocItemCompare(left, right) abort
@@ -70,6 +96,23 @@ function! ale#util#LocItemCompare(left, right) abort
         return 1
     endif
 
+    " When either of the items lacks a problem type, then the two items should
+    " be considered equal. This is important for loclist jumping.
+    if !has_key(a:left, 'type') || !has_key(a:right, 'type')
+        return 0
+    endif
+
+    let l:left_priority = ale#util#GetItemPriority(a:left)
+    let l:right_priority = ale#util#GetItemPriority(a:right)
+
+    if l:left_priority < l:right_priority
+        return -1
+    endif
+
+    if l:left_priority > l:right_priority
+        return 1
+    endif
+
     return 0
 endfunction
 
@@ -139,6 +182,17 @@ function! ale#util#BinarySearch(loclist, buffer, line, column) abort
                 let l:index += 1
             endwhile
 
+            " Scan forwards to find the last item on the column for the item
+            " we found, which will have the most serious problem.
+            let l:item_column = a:loclist[l:index].col
+
+            while l:index < l:max
+            \&& a:loclist[l:index + 1].bufnr == a:buffer
+            \&& a:loclist[l:index + 1].lnum == a:line
+            \&& a:loclist[l:index + 1].col == l:item_column
+                let l:index += 1
+            endwhile
+
             return l:index
         endif
     endwhile
diff --git a/test/test_balloon_messages.vader b/test/test_balloon_messages.vader
index 8f4415ae..d0724c21 100644
--- a/test/test_balloon_messages.vader
+++ b/test/test_balloon_messages.vader
@@ -7,10 +7,19 @@ Before:
 
   let g:ale_buffer_info[bufnr('')] = {'loclist': [
   \ {
+  \   'bufnr': bufnr('%'),
+  \   'lnum': 1,
+  \   'col': 10,
+  \   'linter_name': 'eslint',
+  \   'type': 'W',
+  \   'text': 'Ignore me.',
+  \ },
+  \ {
   \   'bufnr': bufnr(''),
   \   'lnum': 1,
   \   'col': 10,
   \   'text': 'Missing semicolon. (semi)',
+  \   'type': 'E',
   \ },
   \ {
   \   'bufnr': bufnr(''),
diff --git a/test/test_cursor_warnings.vader b/test/test_cursor_warnings.vader
index 19592217..24652909 100644
--- a/test/test_cursor_warnings.vader
+++ b/test/test_cursor_warnings.vader
@@ -2,6 +2,7 @@ Before:
   Save g:ale_echo_msg_format
   Save g:ale_echo_cursor
 
+  " We should prefer the error message at column 10 instead of the warning.
   let g:ale_buffer_info = {
   \ bufnr('%'): {
   \   'loclist': [
@@ -12,6 +13,17 @@ Before:
   \       'vcol': 0,
   \       'linter_name': 'eslint',
   \       'nr': -1,
+  \       'type': 'W',
+  \       'code': 'semi',
+  \       'text': 'Ignore me.',
+  \     },
+  \     {
+  \       'lnum': 1,
+  \       'col': 10,
+  \       'bufnr': bufnr('%'),
+  \       'vcol': 0,
+  \       'linter_name': 'eslint',
+  \       'nr': -1,
   \       'type': 'E',
   \       'code': 'semi',
   \       'text': 'Missing semicolon.',
diff --git a/test/test_loclist_binary_search.vader b/test/test_loclist_binary_search.vader
index 5558191c..219fb314 100644
--- a/test/test_loclist_binary_search.vader
+++ b/test/test_loclist_binary_search.vader
@@ -47,3 +47,20 @@ Execute(Searches should work with just one item):
   let g:loclist = [{'bufnr': 1, 'lnum': 3, 'col': 10}]
 
   AssertEqual 0, ale#util#BinarySearch(g:loclist, 1, 3, 2)
+
+Execute(Searches should return the last item on a single column):
+  let g:loclist = [
+  \ {'bufnr': 1, 'lnum': 1, 'col': 10, 'type': 'W'},
+  \ {'bufnr': 1, 'lnum': 1, 'col': 10, 'type': 'E'},
+  \ {'bufnr': 1, 'lnum': 1, 'col': 11, 'type': 'W'},
+  \ {'bufnr': 1, 'lnum': 1, 'col': 11, 'type': 'E'},
+  \ {'bufnr': 1, 'lnum': 1, 'col': 20, 'type': 'W'},
+  \ {'bufnr': 1, 'lnum': 1, 'col': 20, 'type': 'E'},
+  \]
+
+  " We should return the index for the last item at some column to the right.
+  AssertEqual 1, ale#util#BinarySearch(g:loclist, 1, 1, 1)
+  " We should return the index for the last item at the column we are on.
+  AssertEqual 3, ale#util#BinarySearch(g:loclist, 1, 1, 11)
+  " We should prefer items to the left of the cursor, over the right.
+  AssertEqual 3, ale#util#BinarySearch(g:loclist, 1, 1, 19)
diff --git a/test/test_loclist_jumping.vader b/test/test_loclist_jumping.vader
index 5e18499e..da9a1f57 100644
--- a/test/test_loclist_jumping.vader
+++ b/test/test_loclist_jumping.vader
@@ -2,15 +2,15 @@ Before:
   let g:ale_buffer_info = {
   \ bufnr(''): {
   \   'loclist': [
-  \     {'bufnr': bufnr('') - 1, 'lnum': 3, 'col': 2},
-  \     {'bufnr': bufnr(''), 'lnum': 1, 'col': 2},
-  \     {'bufnr': bufnr(''), 'lnum': 1, 'col': 3},
-  \     {'bufnr': bufnr(''), 'lnum': 2, 'col': 1},
-  \     {'bufnr': bufnr(''), 'lnum': 2, 'col': 2},
-  \     {'bufnr': bufnr(''), 'lnum': 2, 'col': 3},
-  \     {'bufnr': bufnr(''), 'lnum': 2, 'col': 6},
-  \     {'bufnr': bufnr(''), 'lnum': 2, 'col': 700},
-  \     {'bufnr': bufnr('') + 1, 'lnum': 3, 'col': 2},
+  \     {'type': 'E', 'bufnr': bufnr('') - 1, 'lnum': 3, 'col': 2},
+  \     {'type': 'E', 'bufnr': bufnr(''), 'lnum': 1, 'col': 2},
+  \     {'type': 'E', 'bufnr': bufnr(''), 'lnum': 1, 'col': 3},
+  \     {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 1},
+  \     {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 2},
+  \     {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 3},
+  \     {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 6},
+  \     {'type': 'E', 'bufnr': bufnr(''), 'lnum': 2, 'col': 700},
+  \     {'type': 'E', 'bufnr': bufnr('') + 1, 'lnum': 3, 'col': 2},
   \   ],
   \ },
   \}
@@ -81,7 +81,7 @@ Execute(We should be able to jump when the error line is blank):
   " Add a blank line at the end.
   call setline(1, getline('.', '$') + [''])
   " Add a problem on the blank line.
-  call add(g:ale_buffer_info[bufnr('%')].loclist, {'bufnr': bufnr(''), 'lnum': 3, 'col': 1})
+  call add(g:ale_buffer_info[bufnr('%')].loclist, {'type': 'E', 'bufnr': bufnr(''), 'lnum': 3, 'col': 1})
 
   AssertEqual 0, len(getline(3))
   AssertEqual [2, 8], TestJump('before', 0, [3, 1])
diff --git a/test/test_loclist_sorting.vader b/test/test_loclist_sorting.vader
index 157b2a25..376e743a 100644
--- a/test/test_loclist_sorting.vader
+++ b/test/test_loclist_sorting.vader
@@ -25,3 +25,19 @@ Execute(loclist item should be sorted):
   \   {'bufnr': 2, 'lnum': 1, 'col': 2},
   \   {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2},
   \], 'ale#util#LocItemCompare')
+
+Execute(Items should be sorted in by their problem priority when they lie on the same column):
+  AssertEqual [
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W', 'sub_type': 'style'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E', 'sub_type': 'style'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'I'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E'},
+  \ ],
+  \ sort([
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'I'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'E', 'sub_type': 'style'},
+  \   {'bufnr': 1, 'lnum': 1, 'col': 1, 'type': 'W', 'sub_type': 'style'},
+  \], 'ale#util#LocItemCompare')
diff --git a/test/test_quickfix_deduplication.vader b/test/test_quickfix_deduplication.vader
index 0dff3f2e..9cb8b931 100644
--- a/test/test_quickfix_deduplication.vader
+++ b/test/test_quickfix_deduplication.vader
@@ -9,42 +9,42 @@ Execute:
   " Equal problems should be de-duplicated.
   let g:ale_buffer_info = {
   \ '1': {'loclist': [
-  \   {'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'},
-  \   {'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'},
-  \   {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'},
-  \   {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'},
-  \   {'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'},
-  \   {'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'},
-  \   {'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'},
-  \   {'bufnr': 3, 'lnum': 1, 'col': 1, 'text': 'foo'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'},
+  \   {'type': 'E', 'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 3, 'lnum': 1, 'col': 1, 'text': 'foo'},
   \ ]},
   \ '2': {'loclist': [
-  \   {'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'},
-  \   {'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'},
-  \   {'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'},
-  \   {'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'},
-  \   {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'another error'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'another error'},
   \ ]},
   \}
 
   AssertEqual
   \ [
-  \   {'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'},
-  \   {'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'},
-  \   {'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'},
-  \   {'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'},
-  \   {'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'},
-  \   {'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'},
-  \   {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'another error'},
-  \   {'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'},
-  \   {'bufnr': 3, 'lnum': 1, 'col': 1, 'text': 'foo'},
+  \   {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 4, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': -1, 'filename': 'b', 'lnum': 5, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': -1, 'filename': 'c', 'lnum': 3, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 2, 'col': 10, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 3, 'col': 2, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 4, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 1, 'lnum': 5, 'col': 5, 'text': 'x'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 2, 'text': 'foo'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 1, 'col': 5, 'text': 'bar'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'another error'},
+  \   {'type': 'E', 'bufnr': 2, 'lnum': 5, 'col': 5, 'text': 'foo'},
+  \   {'type': 'E', 'bufnr': 3, 'lnum': 1, 'col': 1, 'text': 'foo'},
   \ ],
   \ ale#list#GetCombinedList()