From 734b95a1ac9a65cec45d8f9024d53638e6a3cd2e Mon Sep 17 00:00:00 2001
From: Kirill Bulatov <mail4score@gmail.com>
Date: Mon, 3 May 2021 22:58:53 +0300
Subject: [PATCH] Code review fixes

---
 crates/rust-analyzer/src/handlers.rs | 38 ++++++++++++++++++++--------
 crates/rust-analyzer/src/lsp_ext.rs  |  2 --
 crates/rust-analyzer/src/to_proto.rs |  4 +--
 docs/dev/lsp-extensions.md           |  3 ---
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 0fd03bbaa72..f6e40f87222 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -1058,41 +1058,44 @@ pub(crate) fn handle_code_action_resolve(
         .only
         .map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect());
 
-    let assist_kind: AssistKind = match params.kind.parse() {
-        Ok(kind) => kind,
+    let (assist_index, assist_resolve) = match parse_action_id(&params.id) {
+        Ok(parsed_data) => parsed_data,
         Err(e) => {
             return Err(LspError::new(
                 ErrorCode::InvalidParams as i32,
-                format!("For the assist to resolve, failed to parse the kind: {}", e),
+                format!("Failed to parse action id string '{}': {}", params.id, e),
             )
             .into())
         }
     };
 
+    let expected_assist_id = assist_resolve.assist_id.clone();
+    let expected_kind = assist_resolve.assist_kind;
+
     let assists = snap.analysis.assists_with_fixes(
         &assists_config,
         &snap.config.diagnostics(),
-        AssistResolveStrategy::Single(SingleResolve { assist_id: params.id.clone(), assist_kind }),
+        AssistResolveStrategy::Single(assist_resolve),
         frange,
     )?;
 
-    let assist = match assists.get(params.index) {
+    let assist = match assists.get(assist_index) {
         Some(assist) => assist,
         None => return Err(LspError::new(
             ErrorCode::InvalidParams as i32,
             format!(
-                "Failed to find the assist for index {} provided by the resolve request. Expected assist id: {:?}",
-                params.index, params.id,
+                "Failed to find the assist for index {} provided by the resolve request. Resolve request assist id: {}",
+                assist_index, params.id,
             ),
         )
         .into())
     };
-    if assist.id.0 != params.id || assist.id.1 != assist_kind {
+    if assist.id.0 != expected_assist_id || assist.id.1 != expected_kind {
         return Err(LspError::new(
             ErrorCode::InvalidParams as i32,
             format!(
-                "Failed to find exactly the same assist at index {} for the resolve parameters given. Expected id and kind: {}, {:?}, actual id: {:?}.",
-                params.index, params.id, assist_kind, assist.id
+                "Mismatching assist at index {} for the resolve parameters given. Resolve request assist id: {}, actual id: {:?}.",
+                assist_index, params.id, assist.id
             ),
         )
         .into());
@@ -1102,6 +1105,21 @@ pub(crate) fn handle_code_action_resolve(
     Ok(code_action)
 }
 
+fn parse_action_id(action_id: &str) -> Result<(usize, SingleResolve), String> {
+    let id_parts = action_id.split(':').collect_vec();
+    match id_parts.as_slice() {
+        &[assist_id_string, assist_kind_string, index_string] => {
+            let assist_kind: AssistKind = assist_kind_string.parse()?;
+            let index: usize = match index_string.parse() {
+                Ok(index) => index,
+                Err(e) => return Err(format!("Incorrect index string: {}", e)),
+            };
+            Ok((index, SingleResolve { assist_id: assist_id_string.to_string(), assist_kind }))
+        }
+        _ => Err("Action id contains incorrect number of segments".to_string()),
+    }
+}
+
 pub(crate) fn handle_code_lens(
     snap: GlobalStateSnapshot,
     params: lsp_types::CodeLensParams,
diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs
index 292aedc9c2c..b8835a5349b 100644
--- a/crates/rust-analyzer/src/lsp_ext.rs
+++ b/crates/rust-analyzer/src/lsp_ext.rs
@@ -303,8 +303,6 @@ pub struct CodeAction {
 pub struct CodeActionData {
     pub code_action_params: lsp_types::CodeActionParams,
     pub id: String,
-    pub kind: String,
-    pub index: usize,
 }
 
 #[derive(Debug, Eq, PartialEq, Clone, Default, Deserialize, Serialize)]
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index 62e16680b4a..1d27aa7b373 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -897,10 +897,8 @@ pub(crate) fn code_action(
         (Some(it), _) => res.edit = Some(snippet_workspace_edit(snap, it)?),
         (None, Some((index, code_action_params))) => {
             res.data = Some(lsp_ext::CodeActionData {
-                id: assist.id.0.to_string(),
+                id: format!("{}:{}:{}", assist.id.0, assist.id.1.name(), index),
                 code_action_params,
-                kind: assist.id.1.name().to_string(),
-                index,
             });
         }
         (None, None) => {
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md
index e2ea695f260..f0f981802ef 100644
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@@ -81,7 +81,6 @@ If this capability is set, `CodeAction` returned from the server contain an addi
 interface CodeAction {
     title: string;
     group?: string;
-    data?: string;
     ...
 }
 ```
@@ -102,8 +101,6 @@ The set of actions `[ { title: "foo" }, { group: "frobnicate", title: "bar" }, {
 
 Alternatively, selecting `frobnicate` could present a user with an additional menu to choose between `bar` and `baz`.
 
-`data` field contains optional json data for deferred resolve of the action data that's slow to compute in the original request.
-
 ### Example
 
 ```rust