internal: better factoring for to_proto::completion

One source completion can produce up to two lsp completions.
Additionally, `preselct` and `sort_text` are global properties of the
whole set of completions, so the right granularity here is to convert
many completions.

As a side-benefit, we no loger allocate intermediate vec.

crates/rust-analyzer/src/handlers.rs

@@ -22,12 +22,10 @@ use lsp_types::{
     FoldingRangeParams, HoverContents, Location, NumberOrString, Position, PrepareRenameResponse,
     Range, RenameParams, SemanticTokensDeltaParams, SemanticTokensFullDeltaResult,
     SemanticTokensParams, SemanticTokensRangeParams, SemanticTokensRangeResult,
-    SemanticTokensResult, SymbolInformation, SymbolTag, TextDocumentIdentifier,
+    SemanticTokensResult, SymbolInformation, SymbolTag, TextDocumentIdentifier, Url, WorkspaceEdit,
-    TextDocumentPositionParams, Url, WorkspaceEdit,
 };
 use project_model::TargetKind;
-use serde::{Deserialize, Serialize};
+use serde_json::json;
-use serde_json::{json, to_value};
 use stdx::format_to;
 use syntax::{algo, ast, AstNode, TextRange, TextSize};
 
@@ -764,23 +762,13 @@ pub(crate) fn handle_completion(
     };
     let line_index = snap.file_line_index(position.file_id)?;
 
-    let insert_replace_support =
+    let items = to_proto::completion_items(
-        snap.config.insert_replace_support().then(|| text_document_position.position);
+        snap.config.insert_replace_support(),
-    let items: Vec<CompletionItem> = items
+        completion_config.enable_imports_on_the_fly,
-        .into_iter()
+        &line_index,
-        .flat_map(|item| {
+        text_document_position.clone(),
-            let mut new_completion_items =
+        items.clone(),
-                to_proto::completion_item(insert_replace_support, &line_index, item.clone());
+    );
-
-            if completion_config.enable_imports_on_the_fly {
-                for new_item in &mut new_completion_items {
-                    fill_resolve_data(&mut new_item.data, &item, &text_document_position);
-                }
-            }
-
-            new_completion_items
-        })
-        .collect();
 
     let completion_list = lsp_types::CompletionList { is_incomplete: true, items };
     Ok(Some(completion_list.into()))
@@ -800,16 +788,13 @@ pub(crate) fn handle_completion_resolve(
         .into());
     }
 
-    let resolve_data = match original_completion
+    let data = match original_completion.data.take() {
-        .data
+        Some(it) => it,
-        .take()
-        .map(serde_json::from_value::<CompletionResolveData>)
-        .transpose()?
-    {
-        Some(data) => data,
         None => return Ok(original_completion),
     };
 
+    let resolve_data: lsp_ext::CompletionResolveData = serde_json::from_value(data)?;
+
     let file_id = from_proto::file_id(&snap, &resolve_data.position.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
     let offset = from_proto::offset(&line_index, resolve_data.position.position);
@@ -1760,29 +1745,3 @@ fn run_rustfmt(
         Ok(Some(to_proto::text_edit_vec(&line_index, diff(&file, &new_text))))
     }
 }
-
-#[derive(Debug, Serialize, Deserialize)]
-struct CompletionResolveData {
-    position: lsp_types::TextDocumentPositionParams,
-    full_import_path: String,
-    imported_name: String,
-}
-
-fn fill_resolve_data(
-    resolve_data: &mut Option<serde_json::Value>,
-    item: &ide::CompletionItem,
-    position: &TextDocumentPositionParams,
-) -> Option<()> {
-    let import_edit = item.import_to_add()?;
-    let import_path = &import_edit.import.import_path;
-
-    *resolve_data = Some(
-        to_value(CompletionResolveData {
-            position: position.to_owned(),
-            full_import_path: import_path.to_string(),
-            imported_name: import_path.segments().last()?.to_string(),
-        })
-        .unwrap(),
-    );
-    Some(())
-}

crates/rust-analyzer/src/lsp_ext.rs

@@ -499,3 +499,10 @@ pub enum WorkspaceSymbolSearchKind {
     OnlyTypes,
     AllSymbols,
 }
+
+#[derive(Debug, Serialize, Deserialize)]
+pub struct CompletionResolveData {
+    pub position: lsp_types::TextDocumentPositionParams,
+    pub full_import_path: String,
+    pub imported_name: String,
+}

crates/rust-analyzer/src/to_proto.rs

@@ -197,11 +197,35 @@ pub(crate) fn snippet_text_edit_vec(
         .collect()
 }
 
-pub(crate) fn completion_item(
+pub(crate) fn completion_items(
-    insert_replace_support: Option<lsp_types::Position>,
+    insert_replace_support: bool,
+    enable_imports_on_the_fly: bool,
     line_index: &LineIndex,
-    item: CompletionItem,
+    tdpp: lsp_types::TextDocumentPositionParams,
+    items: Vec<CompletionItem>,
 ) -> Vec<lsp_types::CompletionItem> {
+    let mut res = Vec::with_capacity(items.len());
+    for item in items {
+        completion_item(
+            &mut res,
+            insert_replace_support,
+            enable_imports_on_the_fly,
+            line_index,
+            &tdpp,
+            item,
+        )
+    }
+    res
+}
+
+fn completion_item(
+    acc: &mut Vec<lsp_types::CompletionItem>,
+    insert_replace_support: bool,
+    enable_imports_on_the_fly: bool,
+    line_index: &LineIndex,
+    tdpp: &lsp_types::TextDocumentPositionParams,
+    item: CompletionItem,
+) {
     let mut additional_text_edits = Vec::new();
 
     // LSP does not allow arbitrary edits in completion, so we have to do a
@@ -211,6 +235,7 @@ pub(crate) fn completion_item(
         let source_range = item.source_range();
         for indel in item.text_edit().iter() {
             if indel.delete.contains_range(source_range) {
+                let insert_replace_support = insert_replace_support.then(|| tdpp.position);
                 text_edit = Some(if indel.delete == source_range {
                     self::completion_text_edit(line_index, insert_replace_support, indel.clone())
                 } else {
@@ -253,8 +278,22 @@ pub(crate) fn completion_item(
         lsp_item.command = Some(command::trigger_parameter_hints());
     }
 
-    let mut res = match item.ref_match() {
+    lsp_item.insert_text_format = Some(insert_text_format(item.insert_text_format()));
-        Some((mutability, relevance)) => {
+    if enable_imports_on_the_fly {
+        if let Some(import_edit) = item.import_to_add() {
+            let import_path = &import_edit.import.import_path;
+            if let Some(import_name) = import_path.segments().last() {
+                let data = lsp_ext::CompletionResolveData {
+                    position: tdpp.clone(),
+                    full_import_path: import_path.to_string(),
+                    imported_name: import_name.to_string(),
+                };
+                lsp_item.data = Some(to_value(data).unwrap());
+            }
+        }
+    }
+
+    if let Some((mutability, relevance)) = item.ref_match() {
         let mut lsp_item_with_ref = lsp_item.clone();
         set_score(&mut lsp_item_with_ref, relevance);
         lsp_item_with_ref.label =
@@ -266,16 +305,11 @@ pub(crate) fn completion_item(
             };
             *new_text = format!("&{}{}", mutability.as_keyword_for_ref(), new_text);
         }
-            vec![lsp_item_with_ref, lsp_item]
+
-        }
+        acc.push(lsp_item_with_ref);
-        None => vec![lsp_item],
     };
 
-    for lsp_item in res.iter_mut() {
+    acc.push(lsp_item);
-        lsp_item.insert_text_format = Some(insert_text_format(item.insert_text_format()));
-    }
-
-    return res;
 
     fn set_score(res: &mut lsp_types::CompletionItem, relevance: CompletionRelevance) {
         if relevance.is_relevant() {
@@ -1179,7 +1213,9 @@ mod tests {
             encoding: OffsetEncoding::Utf16,
         };
         let (analysis, file_id) = Analysis::from_single_file(text);
-        let completions: Vec<(String, Option<String>)> = analysis
+
+        let file_position = ide_db::base_db::FilePosition { file_id, offset };
+        let mut items = analysis
             .completions(
                 &ide::CompletionConfig {
                     enable_postfix_completions: true,
@@ -1196,15 +1232,26 @@ mod tests {
                         skip_glob_imports: true,
                     },
                 },
-                ide_db::base_db::FilePosition { file_id, offset },
+                file_position,
             )
             .unwrap()
-            .unwrap()
+            .unwrap();
-            .into_iter()
+        items.retain(|c| c.label().ends_with("arg"));
-            .filter(|c| c.label().ends_with("arg"))
+        let items = completion_items(
-            .map(|c| completion_item(None, &line_index, c))
+            false,
-            .flat_map(|comps| comps.into_iter().map(|c| (c.label, c.sort_text)))
+            false,
-            .collect();
+            &line_index,
+            lsp_types::TextDocumentPositionParams {
+                text_document: lsp_types::TextDocumentIdentifier {
+                    uri: "file://main.rs".parse().unwrap(),
+                },
+                position: position(&line_index, file_position.offset),
+            },
+            items,
+        );
+        let items: Vec<(String, Option<String>)> =
+            items.into_iter().map(|c| (c.label, c.sort_text)).collect();
+
         expect_test::expect![[r#"
             [
                 (
@@ -1221,7 +1268,7 @@ mod tests {
                 ),
             ]
         "#]]
-        .assert_debug_eq(&completions);
+        .assert_debug_eq(&items);
     }
 
     #[test]

docs/dev/lsp-extensions.md

@@ -1,5 +1,5 @@
 <!---
-lsp_ext.rs hash: 3f2879db0013a72
+lsp_ext.rs hash: 3b2931972b33198b
 
 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue: