From 5fb0927a4bb0b3c53ff08b50e1bc984b6e7ae241 Mon Sep 17 00:00:00 2001 From: Boris Staletic Date: Sun, 14 Jan 2024 10:28:58 +0100 Subject: [PATCH] Sort diagnostics according to severity for detailed diag responses 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. --- ycmd/completers/cs/cs_completer.py | 11 +++++++++++ .../language_server_completer.py | 13 ++++++++++--- .../typescript/typescript_completer.py | 12 ++++++++++++ ycmd/tests/clangd/diagnostics_test.py | 17 +++++++++++++++++ .../testdata/diag_ranges/compile_flags.txt | 4 ++++ .../testdata/diag_ranges/detailed_diagnostic.cc | 4 ++++ .../language_server_completer_test.py | 6 ++++-- 7 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 ycmd/tests/clangd/testdata/diag_ranges/compile_flags.txt create mode 100644 ycmd/tests/clangd/testdata/diag_ranges/detailed_diagnostic.cc diff --git a/ycmd/completers/cs/cs_completer.py b/ycmd/completers/cs/cs_completer.py index f7d0174e63..18f52691a4 100644 --- a/ycmd/completers/cs/cs_completer.py +++ b/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 diff --git a/ycmd/completers/language_server/language_server_completer.py b/ycmd/completers/language_server/language_server_completer.py index 9f0ff09fda..6c17a83435 100644 --- a/ycmd/completers/language_server/language_server_completer.py +++ b/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 diff --git a/ycmd/completers/typescript/typescript_completer.py b/ycmd/completers/typescript/typescript_completer.py index 0d4cd699ca..9735f36f9f 100644 --- a/ycmd/completers/typescript/typescript_completer.py +++ b/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 diff --git a/ycmd/tests/clangd/diagnostics_test.py b/ycmd/tests/clangd/diagnostics_test.py index 290170ff5e..49161203dd 100644 --- a/ycmd/tests/clangd/diagnostics_test.py +++ b/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 = """ diff --git a/ycmd/tests/clangd/testdata/diag_ranges/compile_flags.txt b/ycmd/tests/clangd/testdata/diag_ranges/compile_flags.txt new file mode 100644 index 0000000000..9181fe41fc --- /dev/null +++ b/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 diff --git a/ycmd/tests/clangd/testdata/diag_ranges/detailed_diagnostic.cc b/ycmd/tests/clangd/testdata/diag_ranges/detailed_diagnostic.cc new file mode 100644 index 0000000000..93b6e23690 --- /dev/null +++ b/ycmd/tests/clangd/testdata/diag_ranges/detailed_diagnostic.cc @@ -0,0 +1,4 @@ +int main() { + int i; + for (int j; j;i){} +} diff --git a/ycmd/tests/language_server/language_server_completer_test.py b/ycmd/tests/language_server/language_server_completer_test.py index db5259a248..56f4f51c94 100644 --- a/ycmd/tests/language_server/language_server_completer_test.py +++ b/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 )