From b96501f767414b17b320b5443cb87b5998502eb2 Mon Sep 17 00:00:00 2001 From: Boris Staletic Date: Mon, 2 Dec 2024 20:34:21 +0100 Subject: [PATCH] Handle servers that refuse to resolve code action literals The protocol expects us to call `codeAction/resolve` whenever a codeaction lacks either an `edit` or a `command`. Code action literals can lack the `data` field, which some servers (hint: rust-analyzer) require to respond to `codeAction/resolve`. However, we can still apply such code actions, so we do. --- build.py | 2 +- .../language_server_completer.py | 21 +++-- ycmd/tests/rust/__init__.py | 33 ++++++++ ycmd/tests/rust/diagnostics_test.py | 84 ++++++++++++++----- .../rust/get_completions_proc_macro_test.py | 33 +++++--- ycmd/tests/rust/inlay_hints_test.py | 2 +- ycmd/tests/rust/subcommands_test.py | 51 ++++++----- ycmd/tests/rust/testdata/common/src/main.rs | 6 ++ 8 files changed, 167 insertions(+), 65 deletions(-) diff --git a/build.py b/build.py index 2b651f8d..d21b0211 100755 --- a/build.py +++ b/build.py @@ -95,7 +95,7 @@ JDTLS_SHA256 = ( '028e274d06f4a61cad4ffd56f89ef414a8f65613c6d05d9467651b7fb03dae7b' ) -DEFAULT_RUST_TOOLCHAIN = 'nightly-2024-06-11' +DEFAULT_RUST_TOOLCHAIN = 'nightly-2024-12-12' RUST_ANALYZER_DIR = p.join( DIR_OF_THIRD_PARTY, 'rust-analyzer' ) BUILD_ERROR_MESSAGE = ( diff --git a/ycmd/completers/language_server/language_server_completer.py b/ycmd/completers/language_server/language_server_completer.py index 489e15d0..d8a7ee72 100644 --- a/ycmd/completers/language_server/language_server_completer.py +++ b/ycmd/completers/language_server/language_server_completer.py @@ -2988,22 +2988,29 @@ class LanguageServerCompleter( Completer ): # provider, but send a LSP Command instead. We can not resolve those with # codeAction/resolve! if ( 'command' not in code_action or - isinstance( code_action[ 'command' ], str ) ): + not isinstance( code_action[ 'command' ], str ) ): request_id = self.GetConnection().NextRequestId() msg = lsp.CodeActionResolve( request_id, code_action ) - code_action = self.GetConnection().GetResponse( - request_id, - msg, - REQUEST_TIMEOUT_COMMAND )[ 'result' ] + try: + code_action = self.GetConnection().GetResponse( + request_id, + msg, + REQUEST_TIMEOUT_COMMAND )[ 'result' ] + except ResponseFailedException: + # Even if resolving has failed, we might still be able to apply + # what we have previously received... + # See https://github.com/rust-lang/rust-analyzer/issues/18428 + if not ( 'edit' in code_action or 'command' in code_action ): + raise result = [] if 'edit' in code_action: result.append( self.CodeActionLiteralToFixIt( request_data, code_action ) ) - if 'command' in code_action: + if command := code_action.get( 'command' ): assert not result, 'Code actions with edit and command is not supported.' - if isinstance( code_action[ 'command' ], str ): + if isinstance( command, str ): unresolved_command_fixit = self.CommandToFixIt( request_data, code_action ) else: diff --git a/ycmd/tests/rust/__init__.py b/ycmd/tests/rust/__init__.py index 16df6fed..70e9a629 100644 --- a/ycmd/tests/rust/__init__.py +++ b/ycmd/tests/rust/__init__.py @@ -17,11 +17,14 @@ import functools import os +import time +from pprint import pformat from ycmd.tests.test_utils import ( BuildRequest, ClearCompletionsCache, IgnoreExtraConfOutsideTestsFolder, IsolatedApp, + PollForMessagesTimeoutException, SetUpApp, StopCompleterServer, WaitUntilCompleterServerReady ) @@ -52,6 +55,36 @@ def StartRustCompleterServerInDirectory( app, directory ): WaitUntilCompleterServerReady( app, 'rust' ) +def PollForMessages( app, request_data, timeout = 60 ): + expiration = time.time() + timeout + while True: + if time.time() > expiration: + raise PollForMessagesTimeoutException( 'Waited for diagnostics to be ' + f'ready for { timeout } seconds, aborting.' ) + + default_args = { + 'line_num' : 1, + 'column_num': 1, + } + args = dict( default_args ) + args.update( request_data ) + + response = app.post_json( '/receive_messages', BuildRequest( **args ) ).json + + print( f'poll response: { pformat( response ) }' ) + + if isinstance( response, bool ): + if not response: + raise RuntimeError( 'The message poll was aborted by the server' ) + elif isinstance( response, list ): + return response + else: + raise AssertionError( + f'Message poll response was wrong type: { type( response ).__name__ }' ) + + time.sleep( 0.25 ) + + def SharedYcmd( test ): global shared_app diff --git a/ycmd/tests/rust/diagnostics_test.py b/ycmd/tests/rust/diagnostics_test.py index cd70efcd..93aa6418 100644 --- a/ycmd/tests/rust/diagnostics_test.py +++ b/ycmd/tests/rust/diagnostics_test.py @@ -26,10 +26,9 @@ import json import os from ycmd.tests.rust import setUpModule, tearDownModule # noqa -from ycmd.tests.rust import PathToTestFile, SharedYcmd +from ycmd.tests.rust import PathToTestFile, SharedYcmd, PollForMessages from ycmd.tests.test_utils import ( BuildRequest, LocationMatcher, - PollForMessages, PollForMessagesTimeoutException, RangeMatcher, WaitForDiagnosticsToBeReady, @@ -74,10 +73,42 @@ DIAG_MATCHERS_PER_FILE = { ( 21, 10 ) ) ), 'fixit_available': False } ), + has_entries( { + 'kind': 'ERROR', + 'text': 'cannot assign twice to immutable variable `foo`\n' + 'cannot assign twice to immutable variable [E0384]', + 'location': LocationMatcher( MAIN_FILEPATH, 27, 5 ), + 'location_extent': RangeMatcher( MAIN_FILEPATH, ( 27, 5 ), ( 27, 13 ) ), + 'ranges': contains_exactly( RangeMatcher( MAIN_FILEPATH, + ( 27, 5 ), + ( 27, 13 ) ) ), + 'fixit_available': False + } ), + has_entries( { + 'kind': 'HINT', + 'text': 'first assignment to `foo` [E0384]', + 'location': LocationMatcher( MAIN_FILEPATH, 26, 9 ), + 'location_extent': RangeMatcher( MAIN_FILEPATH, ( 26, 9 ), ( 26, 12 ) ), + 'ranges': contains_exactly( RangeMatcher( MAIN_FILEPATH, + ( 26, 9 ), + ( 26, 12 ) ) ), + 'fixit_available': False + } ), + has_entries( { + 'kind': 'HINT', + 'text': 'consider making this binding mutable: `mut ` [E0384]', + 'location': LocationMatcher( MAIN_FILEPATH, 26, 9 ), + 'location_extent': RangeMatcher( MAIN_FILEPATH, ( 26, 9 ), ( 26, 9 ) ), + 'ranges': contains_exactly( RangeMatcher( MAIN_FILEPATH, + ( 26, 9 ), + ( 26, 9 ) ) ), + 'fixit_available': False + } ), ), TEST_FILEPATH: contains_inanyorder( has_entries( { 'kind': 'WARNING', + 'text': 'function cannot return without recursing\n' 'a `loop` may express intention better if this is ' 'on purpose\n' @@ -131,27 +162,34 @@ class DiagnosticsTest( TestCase ): 'no field `build_` on type `test::Builder`\nunknown field [E0609]' ) ) - @WithRetry() @SharedYcmd def test_Diagnostics_FileReadyToParse( self, app ): - filepath = PathToTestFile( 'common', 'src', 'main.rs' ) - contents = ReadFile( filepath ) - with open( filepath, 'w' ) as f: - f.write( contents ) - event_data = BuildRequest( event_name = 'FileSave', - contents = contents, - filepath = filepath, - filetype = 'rust' ) - app.post_json( '/event_notification', event_data ) - - # It can take a while for the diagnostics to be ready. - results = WaitForDiagnosticsToBeReady( app, filepath, contents, 'rust' ) - print( f'completer response: { pformat( results ) }' ) - - assert_that( results, DIAG_MATCHERS_PER_FILE[ filepath ] ) - - - @WithRetry() + for filename in [ 'main.rs', 'test.rs' ]: + with self.subTest( filename = filename ): + @WithRetry() + def Test(): + filepath = PathToTestFile( 'common', 'src', filename ) + contents = ReadFile( filepath ) + with open( filepath, 'w' ) as f: + f.write( contents ) + event_data = BuildRequest( event_name = 'FileSave', + contents = contents, + filepath = filepath, + filetype = 'rust' ) + app.post_json( '/event_notification', event_data ) + + # It can take a while for the diagnostics to be ready. + results = WaitForDiagnosticsToBeReady( app, + filepath, + contents, + 'rust' ) + print( f'completer response: { pformat( results ) }' ) + + assert_that( results, DIAG_MATCHERS_PER_FILE[ filepath ] ) + Test() + + + @WithRetry( { 'reruns': 1000 } ) @SharedYcmd def test_Diagnostics_Poll( self, app ): project_dir = PathToTestFile( 'common' ) @@ -170,10 +208,10 @@ class DiagnosticsTest( TestCase ): seen = {} try: - for message in PollForMessages( app, + for message in reversed( PollForMessages( app, { 'filepath': filepath, 'contents': contents, - 'filetype': 'rust' } ): + 'filetype': 'rust' } ) ): print( f'Message { pformat( message ) }' ) if 'diagnostics' in message: if message[ 'diagnostics' ] == []: diff --git a/ycmd/tests/rust/get_completions_proc_macro_test.py b/ycmd/tests/rust/get_completions_proc_macro_test.py index 77d17ef5..534b4dc7 100644 --- a/ycmd/tests/rust/get_completions_proc_macro_test.py +++ b/ycmd/tests/rust/get_completions_proc_macro_test.py @@ -19,8 +19,9 @@ import time from hamcrest import ( assert_that, has_item, empty, + equal_to, has_key, - is_not ) + has_entry ) from pprint import pformat from unittest import TestCase @@ -37,7 +38,7 @@ from ycmd.utils import ReadFile class GetCompletionsProcMacroTest( TestCase ): @WithRetry() - @IsolatedYcmd() + @IsolatedYcmd( { 'max_num_candidates_to_detail': 0 } ) def test_GetCompletions_ProcMacro( self, app ): StartRustCompleterServerInDirectory( app, PathToTestFile( 'macro' ) ) @@ -68,17 +69,27 @@ class GetCompletionsProcMacroTest( TestCase ): ) ) - # This completer does not require or support resolve - assert_that( results[ 0 ], is_not( has_key( 'resolve' ) ) ) - assert_that( results[ 0 ], is_not( has_key( 'item' ) ) ) + checked_candidate = None + for candidate in results: + if candidate[ 'insertion_text' ] == 'checkpoint': + checked_candidate = candidate + break + + unresolved_item = checked_candidate[ 'extra_data' ] + assert_that( unresolved_item, has_key( 'resolve' ) ) + assert_that( unresolved_item, has_key( 'item' ) ) + assert_that( checked_candidate, has_entry( 'detailed_info', + 'checkpoint\n\n' ) ) - # So (erroneously) resolving an item returns the item - completion_data[ 'resolve' ] = 0 + completion_data[ 'resolve' ] = unresolved_item[ 'resolve' ] response = app.post_json( '/resolve_completion', completion_data ).json print( f"Resolve resolve: { pformat( response ) }" ) - # We can't actually check the result because we don't know what completion - # resolve ID 0 actually is (could be anything), so we just check that we - # get 1 result, and that there are no errors. - assert_that( response[ 'completion' ], is_not( None ) ) assert_that( response[ 'errors' ], empty() ) + assert_that( response[ 'completion' ][ 'detailed_info' ], + equal_to( + "checkpoint\n" + "\n" + "Validate that all current expectations for " + "all methods have\n" + "been satisfied, and discard them." ) ) diff --git a/ycmd/tests/rust/inlay_hints_test.py b/ycmd/tests/rust/inlay_hints_test.py index ed46b466..1457f032 100644 --- a/ycmd/tests/rust/inlay_hints_test.py +++ b/ycmd/tests/rust/inlay_hints_test.py @@ -107,7 +107,7 @@ class SignatureHelpTest( TestCase ): has_entries( { 'kind': 'Type', 'position': LocationMatcher( filepath, 12, 10 ), - 'label': ': Builder ' + 'label': ': Builder' } ), ), } ) diff --git a/ycmd/tests/rust/subcommands_test.py b/ycmd/tests/rust/subcommands_test.py index 3b422a18..a77b3fe8 100644 --- a/ycmd/tests/rust/subcommands_test.py +++ b/ycmd/tests/rust/subcommands_test.py @@ -561,28 +561,35 @@ class SubcommandsTest( TestCase ): def test_Subcommands_FixIt_Basic( self, app ): filepath = PathToTestFile( 'common', 'src', 'main.rs' ) - RunFixItTest( app, { - 'description': 'Simple FixIt test', - 'chosen_fixit': 2, - 'request': { - 'command': 'FixIt', - 'line_num': 18, - 'column_num': 2, - 'filepath': filepath - }, - 'expect': { - 'response': requests.codes.ok, - 'data': has_entries( { - 'fixits': has_item( has_entries( { - 'chunks': contains_exactly( - ChunkMatcher( 'pub(crate) ', - LocationMatcher( filepath, 18, 1 ), - LocationMatcher( filepath, 18, 1 ) ) - ) - } ) ) + for line, column, choice, chunks in [ + ( 18, 2, 2, [ + ChunkMatcher( 'pub(crate) ', + LocationMatcher( filepath, 18, 1 ), + LocationMatcher( filepath, 18, 1 ) ) ] ), + ( 27, 5, 0, [ + ChunkMatcher( 'mut ', + LocationMatcher( filepath, 26, 9 ), + LocationMatcher( filepath, 26, 9 ) ) ] ), + ]: + with self.subTest( line = line, column = column, choice = choice ): + RunFixItTest( app, { + 'description': 'Simple FixIt test', + 'chosen_fixit': choice, + 'request': { + 'command': 'FixIt', + 'line_num': line, + 'column_num': column, + 'filepath': filepath + }, + 'expect': { + 'response': requests.codes.ok, + 'data': has_entries( { + 'fixits': has_item( has_entries( { + 'chunks': contains_exactly( *chunks ) + } ) ) + } ) + }, } ) - }, - } ) @IsolatedYcmd() @@ -596,7 +603,7 @@ class SubcommandsTest( TestCase ): 'macro' ), ( - { 'req': ( 'main.rs', 14, 19 ), 'res': ( 'test.rs', 4, 12 ) }, + { 'req': ( 'main.rs', 9, 24 ), 'res': ( 'main.rs', 6, 8 ) }, 'common' ), ]: diff --git a/ycmd/tests/rust/testdata/common/src/main.rs b/ycmd/tests/rust/testdata/common/src/main.rs index daf27813..41fd1b76 100644 --- a/ycmd/tests/rust/testdata/common/src/main.rs +++ b/ycmd/tests/rust/testdata/common/src/main.rs @@ -21,3 +21,9 @@ fn format_test() { a : i32 = 5; } + +fn code_action_literal() -> i32 { + let foo = 5; + foo += 1; + foo +} -- 2.45.2