Skip to content

Commit

Permalink
Sort diagnostics according to severity for detailed diag responses
Browse files Browse the repository at this point in the history
Users are more likely to be interested in errors than warnings in
/detailed_diagnostic responses. That means we need to sort the
diagnostics before finding the one nearest to the cursor.

For LSP completers, that's easy as the severity is numerical.
For Omnisharp-roslyn and TSServer, the severity is a word and we can't
just say `sort( key = severity )`.

Since LSP severity was convenient for comparison, I have opted for
sorting C# and TS diags by the equivalent LSP severity.
  • Loading branch information
bstaletic committed Feb 15, 2024
1 parent f11e293 commit 0f3d0aa
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 5 deletions.
11 changes: 11 additions & 0 deletions ycmd/completers/cs/cs_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
13 changes: 10 additions & 3 deletions ycmd/completers/language_server/language_server_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1659,9 +1659,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.'
Expand Down Expand Up @@ -2943,13 +2948,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

Expand Down
12 changes: 12 additions & 0 deletions ycmd/completers/typescript/typescript_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
17 changes: 17 additions & 0 deletions ycmd/tests/clangd/diagnostics_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = """
Expand Down
4 changes: 4 additions & 0 deletions ycmd/tests/clangd/testdata/diag_ranges/compile_flags.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-Wno-error
-Wfor-loop-analysis
-Wno-error=for-loop-analysis
-Werror=uninitialized
4 changes: 4 additions & 0 deletions ycmd/tests/clangd/testdata/diag_ranges/detailed_diagnostic.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
int main() {
int i;
for (int j; j;i){}
}
6 changes: 4 additions & 2 deletions ycmd/tests/language_server/language_server_completer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 )

0 comments on commit 0f3d0aa

Please sign in to comment.