Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort diagnostics according to severity for detailed diag responses #1728

Merged
merged 1 commit into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@
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

Check warning on line 981 in ycmd/completers/cs/cs_completer.py

View check run for this annotation

Codecov / codecov/patch

ycmd/completers/cs/cs_completer.py#L979-L981

Added lines #L979 - L981 were not covered by tests
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 @@ -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.'
Expand Down Expand Up @@ -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

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 @@
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 _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

Check warning on line 1278 in ycmd/completers/typescript/typescript_completer.py

View check run for this annotation

Codecov / codecov/patch

ycmd/completers/typescript/typescript_completer.py#L1276-L1278

Added lines #L1276 - L1278 were not covered by tests
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 )
Loading