Skip to content

Commit

Permalink
Handle servers that refuse to resolve code action literals
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bstaletic committed Dec 13, 2024
1 parent c485180 commit 2594df3
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 65 deletions.
2 changes: 1 addition & 1 deletion build.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def Exit( self ):
'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 = (
Expand Down
21 changes: 14 additions & 7 deletions ycmd/completers/language_server/language_server_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2988,22 +2988,29 @@ def _ResolveFixit( self, request_data, fixit ):
# 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

Check warning on line 3004 in ycmd/completers/language_server/language_server_completer.py

View check run for this annotation

Codecov / codecov/patch

ycmd/completers/language_server/language_server_completer.py#L3004

Added line #L3004 was not covered by tests

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:
Expand Down
33 changes: 33 additions & 0 deletions ycmd/tests/rust/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down Expand Up @@ -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

Expand Down
84 changes: 61 additions & 23 deletions ycmd/tests/rust/diagnostics_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@
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,
Expand Down Expand Up @@ -74,10 +73,42 @@
( 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'
Expand Down Expand Up @@ -131,27 +162,34 @@ def test_Diagnostics_DetailedDiags( self, app ):
'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' )
Expand All @@ -170,10 +208,10 @@ def test_Diagnostics_Poll( self, app ):
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' ] == []:
Expand Down
32 changes: 21 additions & 11 deletions ycmd/tests/rust/get_completions_proc_macro_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
from hamcrest import ( assert_that,
has_item,
empty,
equal_to,
has_key,
is_not )
has_entry )
from pprint import pformat
from unittest import TestCase

Expand All @@ -37,7 +38,7 @@

class GetCompletionsProcMacroTest( TestCase ):
@WithRetry()
@IsolatedYcmd()
@IsolatedYcmd( { 'max_num_candidates_to_detail': 0 } )
def test_GetCompletions_ProcMacro( self, app ):
StartRustCompleterServerInDirectory( app, PathToTestFile( 'macro' ) )

Expand Down Expand Up @@ -68,17 +69,26 @@ def test_GetCompletions_ProcMacro( self, app ):
)
)

# 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

# So (erroneously) resolving an item returns the item
completion_data[ 'resolve' ] = 0
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' ) )

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." ) )
2 changes: 1 addition & 1 deletion ycmd/tests/rust/inlay_hints_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def test_basic( self, app ):
has_entries( {
'kind': 'Type',
'position': LocationMatcher( filepath, 12, 10 ),
'label': ': Builder '
'label': ': Builder'
} ),
),
} )
Expand Down
51 changes: 29 additions & 22 deletions ycmd/tests/rust/subcommands_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,28 +561,35 @@ def test_Subcommands_FixIt_EmptyResponse( self, app ):
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()
Expand All @@ -596,7 +603,7 @@ def test_Subcommands_GoTo_WorksAfterChangingProject( self, app ):
'macro'
),
(
{ 'req': ( 'main.rs', 14, 19 ), 'res': ( 'test.rs', 4, 12 ) },
{ 'req': ( 'main.rs', 9, 24 ), 'res': ( 'main.rs', 6, 8 ) },
'common'
),
]:
Expand Down
6 changes: 6 additions & 0 deletions ycmd/tests/rust/testdata/common/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,9 @@ fn format_test() {
a :
i32 = 5;
}

fn code_action_literal() -> i32 {
let foo = 5;
foo += 1;
foo
}

0 comments on commit 2594df3

Please sign in to comment.