Merge pull request #1728 from bstaletic/detailed-diag-priority

Sort diagnostics according to severity for detailed diag responses

Author: mergify[bot]

ycmd/completers/cs/cs_completer.py

@@ -316,6 +316,9 @@ def GetDetailedDiagnostic( self, request_data ):
     if not diagnostics:
       raise ValueError( NO_DIAGNOSTIC_MESSAGE )
 
+    # Prefer errors to warnings and warnings to infos.
+    diagnostics.sort( key = _CsDiagnosticToLspSeverity )
+
     closest_diagnostic = None
     distance_to_closest_diagnostic = 999
 
@@ -968,3 +971,11 @@ def _ModifiedFilesToFixIt( changes, request_data ):
         request_data[ 'line_num' ],
         request_data[ 'column_codepoint' ] ),
       chunks )
+
+
+def _CsDiagnosticToLspSeverity( diagnostic ):
+  if diagnostic.kind_ == 'ERROR':
+    return 1
+  if diagnostic.kind_ == 'WARNING':
+    return 2
+  return 3

ycmd/completers/language_server/language_server_completer.py

@@ -1663,9 +1663,14 @@ def GetDetailedDiagnostic( self, request_data ):
       return responses.BuildDisplayMessageResponse(
           'No diagnostics for current file.' )
 
+    # Prefer errors to warnings and warnings to infos.
+    diagnostics.sort( key = lambda d: d[ 'severity' ] )
+
+    # request_data uses 1-based offsets, but LSP diagnostics use 0-based.
+    # It's easier to shift this one offset by -1 than to shift all diag ranges.
     current_column = lsp.CodepointsToUTF16CodeUnits(
         GetFileLines( request_data, current_file )[ current_line_lsp ],
-        request_data[ 'column_codepoint' ] )
+        request_data[ 'column_codepoint' ] ) - 1
     minimum_distance = None
 
     message = 'No diagnostics for current line.'
@@ -2981,13 +2986,15 @@ def _DistanceOfPointToRange( point, range ):
   # Single-line range.
   if start[ 'line' ] == end[ 'line' ]:
     # 0 if point is within range, otherwise distance from start/end.
-    return max( 0, point[ 'character' ] - end[ 'character' ],
+    # +1 takes into account that, visually, end is one character farther.
+    return max( 0, point[ 'character' ] - end[ 'character' ] + 1,
                 start[ 'character' ] - point[ 'character' ] )
 
   if start[ 'line' ] == point[ 'line' ]:
     return max( 0, start[ 'character' ] - point[ 'character' ] )
   if end[ 'line' ] == point[ 'line' ]:
-    return max( 0, point[ 'character' ] - end[ 'character' ] )
+    # +1 takes into account that, visually, end is one character farther.
+    return max( 0, point[ 'character' ] - end[ 'character' ] + 1 )
   # If not on the first or last line, then point is within range for sure.
   return 0
 

ycmd/completers/typescript/typescript_completer.py

@@ -656,6 +656,9 @@ def GetDetailedDiagnostic( self, request_data ):
     if not ts_diagnostics_on_line:
       raise ValueError( NO_DIAGNOSTIC_MESSAGE )
 
+    # Prefer errors to warnings and warnings to infos.
+    ts_diagnostics_on_line.sort( key = _TsDiagToLspSeverity )
+
     closest_ts_diagnostic = None
     distance_to_closest_ts_diagnostic = None
 
@@ -1264,3 +1267,12 @@ def _BuildTsFormatRange( request_data ):
 
 def _DisplayPartsToString( parts ):
   return ''.join( [ p[ 'text' ] for p in parts ] )
+
+
+def _TsDiagToLspSeverity( diag ):
+  category = diag[ 'category' ]
+  if category == 'error':
+    return 1
+  if category == 'warning':
+    return 2
+  return 3

ycmd/tests/clangd/diagnostics_test.py

@@ -151,6 +151,23 @@ def test_Diagnostics_Works( self, app ):
                  has_entry( 'message', contains_string( "Expected ';'" ) ) )
 
 
+  @IsolatedYcmd( { 'clangd_args': [ '--clang-tidy=0' ] } )
+  def test_Diagnostics_WarningAndErrorOnSameColumn( self, app ):
+    for col in [ 2, 22 ]:
+      with self.subTest( col = col ):
+        filepath = PathToTestFile( 'diag_ranges', 'detailed_diagnostic.cc' )
+        request = { 'filepath': filepath, 'filetype': 'cpp', 'column_num': col }
+        test = { 'request': request, 'route': '/receive_messages' }
+        RunAfterInitialized( app, test )
+        diag_data = BuildRequest( line_num = 3,
+                                  filepath = filepath,
+                                  filetype = 'cpp' )
+        result = app.post_json( '/detailed_diagnostic', diag_data ).json
+        assert_that( result,
+                     has_entry( 'message',
+                                 contains_string( 'uninitialized' ) ) )
+
+
   @IsolatedYcmd()
   def test_Diagnostics_Multiline( self, app ):
     contents = """

ycmd/tests/clangd/testdata/diag_ranges/compile_flags.txt

@@ -0,0 +1,4 @@
+-Wno-error
+-Wfor-loop-analysis
+-Wno-error=for-loop-analysis
+-Werror=uninitialized

ycmd/tests/clangd/testdata/diag_ranges/detailed_diagnostic.cc

@@ -0,0 +1,4 @@
+int main() {
+	int i;
+        for (int j; j;i){}
+}

ycmd/tests/language_server/language_server_completer_test.py

@@ -1497,7 +1497,8 @@ def test_LanguageServerCompleter_DistanceOfPointToRange_SingleLineRange(
     # Point inside range.
     _Check_Distance( ( 0, 4 ), ( 0, 2 ), ( 0, 5 ) , 0 )
     # Point to the right of range.
-    _Check_Distance( ( 0, 8 ), ( 0, 2 ), ( 0, 5 ) , 3 )
+    # +1 because diags are half-open ranges.
+    _Check_Distance( ( 0, 8 ), ( 0, 2 ), ( 0, 5 ) , 4 )
 
 
   def test_LanguageServerCompleter_DistanceOfPointToRange_MultiLineRange(
@@ -1507,4 +1508,5 @@ def test_LanguageServerCompleter_DistanceOfPointToRange_MultiLineRange(
     # Point inside range.
     _Check_Distance( ( 1, 4 ), ( 0, 2 ), ( 3, 5 ) , 0 )
     # Point to the right of range.
-    _Check_Distance( ( 3, 8 ), ( 0, 2 ), ( 3, 5 ) , 3 )
+    # +1 because diags are half-open ranges.
+    _Check_Distance( ( 3, 8 ), ( 0, 2 ), ( 3, 5 ) , 4 )