From b96501f767414b17b320b5443cb87b5998502eb2 Mon Sep 17 00:00:00 2001
From: Boris Staletic <boris.staletic@protonmail.com>
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