Skip to content

Commit a5f4086

Browse files
committed
Add support codeAction/resolve requests
So... We advertise that we can handle CodeActions that are either missing an edit or a command (or both). We also advertise that we will preserve server `data` that we receive in a CodeAction, when we resolve the said fixit. This allows more servers to take advantage of the lazy code actions. The server chooses whether it supports either of the two. If it does, it advertises itself as a "code action resolve provider". For servers that are code action resolve providers: - If either edit or command is missing, we need to try resolving the code action via codeAction/resolve. - Unless we actually got a LSP Command, in which case codeAction/resove is skipped. - After resolving it that way we have a CodeAction in one of these forms: - A LSP Command - A LSP CodeAction with only an edit. - A LSP CodeAction with only a command. - A LSP CodeAction with both an edit and a command. - Edits are WorkspaceEdits and can easily be converted into ycmd FixIts. - Commands are to be executed, yielding ApplyEdits. A single ApplyEdit can be converted into a ycmd FixIt. For servers that are not code action resolve providers, the steps are the same, but we skip the codeAction/resolve route. One thing missing is properly defined handling of fixit resolutions that yield multiple fixits. That can happen in a few ways: - On /resolve_fixit, by a LSP command yielding multiple ApplyEdits. - When eagerly resolving a fixit, again by a LSP command yielding multiple ApplyEdits. - Even if all commands always yield a single ApplyEdit, if a CodeAction has both an edit and a command, that's again two fixits. The first two above don't seem to be used by any server ever. The LSP specs nudges servers away from doing that, but no promises. We are still not supporting any scenario where resolving a single fixit results in more than one fixit. Another scenario that does not seem to happen: - The server is a code action resove provider. - The received CodeAction has neither an edit nor a command. - After resolving, we get only a command. - We then need to execute the command and collect ApplyEdits. In practice, such code actions resolve to a CodeAction containing an edit. As for the ycmd<->client side of things... it's a bit ugly on the ycmd side, but we're completely preserving the API, so clients do not need to change a thing. Previously, clients got `{ 'fixits': [ { 'command': LSPCommand ... } ] }` for unresolved fixits. However, we have not given any guarantees about the value of `'command'`. We have only said that it should be returned to ycmd for the purposes of `/resolve_fixit`. With this pull request, we need to pass the entire CodeAction, but we're still putting it in the `command` key.
1 parent 0dd5feb commit a5f4086

File tree

7 files changed

+258
-193
lines changed

7 files changed

+258
-193
lines changed

ycmd/completers/java/java_completer.py

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import threading
2525

2626
from ycmd import responses, utils
27-
from ycmd.completers.language_server import language_server_protocol as lsp
2827
from ycmd.completers.language_server import language_server_completer
2928
from ycmd.utils import LOGGER
3029

@@ -624,24 +623,13 @@ def GetDoc( self, request_data ):
624623

625624

626625
def OrganizeImports( self, request_data ):
627-
fixit = {
628-
'resolve': True,
629-
'command': {
630-
'title': 'Organize Imports',
631-
'command': 'java.edit.organizeImports',
632-
'arguments': [ lsp.FilePathToUri( request_data[ 'filepath' ] ) ]
633-
}
634-
}
635-
return self._ResolveFixit( request_data, fixit )
636-
637-
638-
def CodeActionCommandToFixIt( self, request_data, command ):
639-
# JDT wants us to special case `java.apply.workspaceEdit`
640-
# https://github.com/eclipse/eclipse.jdt.ls/issues/376
641-
if command[ 'command' ][ 'command' ] == 'java.apply.workspaceEdit':
642-
command[ 'edit' ] = command.pop( 'command' )[ 'arguments' ][ 0 ]
643-
return super().CodeActionLiteralToFixIt( request_data, command )
644-
return super().CodeActionCommandToFixIt( request_data, command )
626+
fixits = super().GetCodeActions( request_data )[ 'fixits' ]
627+
for fixit in fixits:
628+
if fixit[ 'command' ][ 'kind' ] == 'source.organizeImports':
629+
return self._ResolveFixit( request_data, fixit )
630+
# We should never get here. With codeAction/resolve support,
631+
# JDT always sends the organizeImports code action.
632+
raise RuntimeError( 'OrganizeImports not available.' )
645633

646634

647635
def GetServerName( self ):

ycmd/completers/language_server/language_server_completer.py

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2850,7 +2850,6 @@ def GetCodeActions( self, request_data ):
28502850
cursor_range_ls,
28512851
matched_diagnostics ),
28522852
REQUEST_TIMEOUT_COMMAND )
2853-
28542853
return self.CodeActionResponseToFixIts( request_data,
28552854
code_actions[ 'result' ] )
28562855

@@ -2861,28 +2860,22 @@ def CodeActionResponseToFixIts( self, request_data, code_actions ):
28612860

28622861
fixits = []
28632862
for code_action in code_actions:
2864-
if 'edit' in code_action:
2865-
# TODO: Start supporting a mix of WorkspaceEdits and Commands
2866-
# once there's a need for such
2867-
assert 'command' not in code_action
2868-
2869-
# This is a WorkspaceEdit literal
2870-
fixits.append( self.CodeActionLiteralToFixIt( request_data,
2871-
code_action ) )
2872-
continue
2873-
2874-
# Either a CodeAction or a Command
2875-
assert 'command' in code_action
2876-
2877-
action_command = code_action[ 'command' ]
2878-
if isinstance( action_command, dict ):
2879-
# CodeAction with a 'command' rather than 'edit'
2880-
fixits.append( self.CodeActionCommandToFixIt( request_data,
2881-
code_action ) )
2863+
capabilities = self._server_capabilities[ 'codeActionProvider' ]
2864+
if ( ( isinstance( capabilities, dict ) and
2865+
capabilities.get( 'resolveProvider' ) ) or
2866+
'command' in code_action ):
2867+
# If server is a code action resolve provider, either we are obligated
2868+
# to resolve, or we have a command in the code action response.
2869+
# If server does not want us to resolve, but sends a command anyway,
2870+
# we still need to lazily execute that command.
2871+
fixits.append( responses.UnresolvedFixIt( code_action,
2872+
code_action[ 'title' ],
2873+
code_action.get( 'kind' ) ) )
28822874
continue
2875+
# No resoving here - just a simple code action literal.
2876+
fixits.append( self.CodeActionLiteralToFixIt( request_data,
2877+
code_action ) )
28832878

2884-
# It is a Command
2885-
fixits.append( self.CommandToFixIt( request_data, code_action ) )
28862879

28872880
# Show a list of actions to the user to select which one to apply.
28882881
# This is (probably) a more common workflow for "code action".
@@ -2986,10 +2979,44 @@ def Format( self, request_data ):
29862979

29872980

29882981
def _ResolveFixit( self, request_data, fixit ):
2989-
if not fixit[ 'resolve' ]:
2990-
return { 'fixits': [ fixit ] }
2982+
code_action = fixit[ 'command' ]
2983+
capabilities = self._server_capabilities[ 'codeActionProvider' ]
2984+
if ( isinstance( capabilities, dict ) and
2985+
capabilities.get( 'resolveProvider' ) ):
2986+
# Resolve through codeAction/resolve request, before resolving commands.
2987+
# If the server is an asshole, it might be a code action resolve
2988+
# provider, but send a LSP Command instead. We can not resolve those with
2989+
# codeAction/resolve!
2990+
if ( 'command' not in code_action or
2991+
isinstance( code_action[ 'command' ], str ) ):
2992+
request_id = self.GetConnection().NextRequestId()
2993+
msg = lsp.CodeActionResolve( request_id, code_action )
2994+
code_action = self.GetConnection().GetResponse(
2995+
request_id,
2996+
msg,
2997+
REQUEST_TIMEOUT_COMMAND )[ 'result' ]
29912998

2992-
unresolved_fixit = fixit[ 'command' ]
2999+
result = []
3000+
if 'edit' in code_action:
3001+
result.append( self.CodeActionLiteralToFixIt( request_data,
3002+
code_action ) )
3003+
3004+
if 'command' in code_action:
3005+
assert not result, 'Code actions with edit and command is not supported.'
3006+
if isinstance( code_action[ 'command' ], str ):
3007+
unresolved_command_fixit = self.CommandToFixIt( request_data,
3008+
code_action )
3009+
else:
3010+
unresolved_command_fixit = self.CodeActionCommandToFixIt( request_data,
3011+
code_action )
3012+
result.append( self._ResolveFixitCommand( request_data,
3013+
unresolved_command_fixit ) )
3014+
3015+
return responses.BuildFixItResponse( result )
3016+
3017+
3018+
def _ResolveFixitCommand( self, request_data, fixit ):
3019+
unresolved_fixit = fixit.command
29933020
collector = EditCollector()
29943021
with self.GetConnection().CollectApplyEdits( collector ):
29953022
self.GetCommandResponse(
@@ -3001,19 +3028,23 @@ def _ResolveFixit( self, request_data, fixit ):
30013028
response = collector.requests
30023029
assert len( response ) < 2
30033030
if not response:
3004-
return responses.BuildFixItResponse( [ responses.FixIt(
3031+
return responses.FixIt(
30053032
responses.Location( request_data[ 'line_num' ],
30063033
request_data[ 'column_num' ],
30073034
request_data[ 'filepath' ] ),
3008-
[] ) ] )
3035+
[] )
30093036
fixit = WorkspaceEditToFixIt(
30103037
request_data,
30113038
response[ 0 ][ 'edit' ],
30123039
unresolved_fixit[ 'title' ] )
3013-
return responses.BuildFixItResponse( [ fixit ] )
3040+
return fixit
30143041

30153042

30163043
def ResolveFixit( self, request_data ):
3044+
fixit = request_data[ 'fixit' ]
3045+
if 'command' not in fixit:
3046+
# Somebody has sent us an already resolved fixit.
3047+
return { 'fixits': [ fixit ] }
30173048
return self._ResolveFixit( request_data, request_data[ 'fixit' ] )
30183049

30193050

ycmd/completers/language_server/language_server_protocol.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,13 @@ def Initialize( request_id,
326326
'refactor.inline',
327327
'refactor.rewrite',
328328
'source',
329-
'source.organizeImports' ]
329+
'source.organizeImports',
330+
'source.fixAll' ]
330331
}
332+
},
333+
'dataSupport': True,
334+
'resolveSupport': {
335+
'properties': [ 'edit', 'command' ]
331336
}
332337
},
333338
'completion': {
@@ -580,6 +585,10 @@ def CodeAction( request_id, request_data, best_match_range, diagnostics ):
580585
} )
581586

582587

588+
def CodeActionResolve( request_id, code_action ):
589+
return BuildRequest( request_id, 'codeAction/resolve', code_action )
590+
591+
583592
def Rename( request_id, request_data, new_name ):
584593
return BuildRequest( request_id, 'textDocument/rename', {
585594
'textDocument': TextDocumentIdentifier( request_data ),

ycmd/tests/clangd/subcommands_test.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,8 @@ def FixIt_Check_cpp11_DelAdd( results ):
316316
has_entries( {
317317
'text': 'Move function body to declaration',
318318
'resolve': True,
319-
'command': has_entries( { 'command': 'clangd.applyTweak' } )
319+
'command': has_entries( { 'command': has_entries( {
320+
'command': 'clangd.applyTweak' } ) } )
320321
} ),
321322
)
322323
} ) )

ycmd/tests/go/subcommands_test.py

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def CombineRequest( request, data ):
7171
# We also ignore errors here, but then we check the response code
7272
# ourself. This is to allow testing of requests returning errors.
7373
response = app.post_json(
74-
'/run_completer_command',
74+
test.get( 'route', '/run_completer_command' ),
7575
CombineRequest( test[ 'request' ], {
7676
'completer_target': 'filetype_default',
7777
'contents': contents,
@@ -91,8 +91,14 @@ def CombineRequest( request, data ):
9191
return response.json
9292

9393

94-
def RunFixItTest( app, description, filepath, line, col, fixits_for_line ):
95-
RunTest( app, {
94+
def RunFixItTest( app,
95+
description,
96+
filepath,
97+
line,
98+
col,
99+
fixits_for_line,
100+
chosen_fixit = None ):
101+
test = {
96102
'description': description,
97103
'request': {
98104
'command': 'FixIt',
@@ -104,7 +110,17 @@ def RunFixItTest( app, description, filepath, line, col, fixits_for_line ):
104110
'response': requests.codes.ok,
105111
'data': fixits_for_line,
106112
}
107-
} )
113+
}
114+
if chosen_fixit is not None:
115+
test_no_expect = test.copy()
116+
test_no_expect.pop( 'expect' )
117+
response = RunTest( app, test_no_expect )
118+
request = test[ 'request' ]
119+
request.update( {
120+
'fixit': response[ 'fixits' ][ chosen_fixit ]
121+
} )
122+
test[ 'route' ] = '/resolve_fixit'
123+
RunTest( app, test )
108124

109125

110126
def RunHierarchyTest( app, kind, direction, location, expected, code ):
@@ -445,9 +461,6 @@ def test_Subcommands_FixIt_NullResponse( self, app ):
445461
filepath, 1, 1, has_entry( 'fixits', empty() ) )
446462

447463

448-
@ExpectedFailure(
449-
'Gopls bug. See https://github.com/golang/go/issues/68904',
450-
matches_regexp( 'Browse free symbols' ) )
451464
@SharedYcmd
452465
def test_Subcommands_FixIt_Simple( self, app ):
453466
filepath = PathToTestFile( 'fixit.go' )
@@ -464,7 +477,7 @@ def test_Subcommands_FixIt_Simple( self, app ):
464477
} ),
465478
)
466479
} )
467-
RunFixItTest( app, 'Only one fixit returned', filepath, 1, 1, fixit )
480+
RunFixItTest( app, 'Only one fixit returned', filepath, 1, 1, fixit, 0 )
468481

469482

470483
@SharedYcmd

0 commit comments

Comments
 (0)