Always wait for completion resolve before applying the completion edits (#18907)

After rust-lang/rust-analyzer#18167 and certain
people who type and complete rapidly, it turned out that we have not
waited for `completionItem/resolve` to finish before applying the
completion results.

Release Notes:

- Fixed completion items applied improperly on fast typing

Author: SomeoneToIgnore

crates/collab/src/tests/editor_tests.rs

@@ -379,51 +379,75 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
         .next()
         .await
         .unwrap();
-    cx_a.executor().finish_waiting();
-
     // Open the buffer on the host.
     let buffer_a = project_a
         .update(cx_a, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx))
         .await
         .unwrap();
-    cx_a.executor().run_until_parked();
 
     buffer_a.read_with(cx_a, |buffer, _| {
         assert_eq!(buffer.text(), "fn main() { a. }")
     });
 
-    // Confirm a completion on the guest.
-    editor_b.update(cx_b, |editor, cx| {
-        assert!(editor.context_menu_visible());
-        editor.confirm_completion(&ConfirmCompletion { item_ix: Some(0) }, cx);
-        assert_eq!(editor.text(cx), "fn main() { a.first_method() }");
-    });
-
     // Return a resolved completion from the host's language server.
     // The resolved completion has an additional text edit.
     fake_language_server.handle_request::<lsp::request::ResolveCompletionItem, _, _>(
         |params, _| async move {
-            assert_eq!(params.label, "first_method(…)");
-            Ok(lsp::CompletionItem {
-                label: "first_method(…)".into(),
-                detail: Some("fn(&mut self, B) -> C".into()),
-                text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
-                    new_text: "first_method($1)".to_string(),
-                    range: lsp::Range::new(lsp::Position::new(0, 14), lsp::Position::new(0, 14)),
-                })),
-                additional_text_edits: Some(vec![lsp::TextEdit {
-                    new_text: "use d::SomeTrait;\n".to_string(),
-                    range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)),
-                }]),
-                insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
-                ..Default::default()
+            Ok(match params.label.as_str() {
+                "first_method(…)" => lsp::CompletionItem {
+                    label: "first_method(…)".into(),
+                    detail: Some("fn(&mut self, B) -> C".into()),
+                    text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+                        new_text: "first_method($1)".to_string(),
+                        range: lsp::Range::new(
+                            lsp::Position::new(0, 14),
+                            lsp::Position::new(0, 14),
+                        ),
+                    })),
+                    additional_text_edits: Some(vec![lsp::TextEdit {
+                        new_text: "use d::SomeTrait;\n".to_string(),
+                        range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)),
+                    }]),
+                    insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
+                    ..Default::default()
+                },
+                "second_method(…)" => lsp::CompletionItem {
+                    label: "second_method(…)".into(),
+                    detail: Some("fn(&mut self, C) -> D<E>".into()),
+                    text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+                        new_text: "second_method()".to_string(),
+                        range: lsp::Range::new(
+                            lsp::Position::new(0, 14),
+                            lsp::Position::new(0, 14),
+                        ),
+                    })),
+                    insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
+                    additional_text_edits: Some(vec![lsp::TextEdit {
+                        new_text: "use d::SomeTrait;\n".to_string(),
+                        range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)),
+                    }]),
+                    ..Default::default()
+                },
+                _ => panic!("unexpected completion label: {:?}", params.label),
             })
         },
     );
+    cx_a.executor().finish_waiting();
+    cx_a.executor().run_until_parked();
 
-    // The additional edit is applied.
+    // Confirm a completion on the guest.
+    editor_b
+        .update(cx_b, |editor, cx| {
+            assert!(editor.context_menu_visible());
+            editor.confirm_completion(&ConfirmCompletion { item_ix: Some(0) }, cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
     cx_a.executor().run_until_parked();
+    cx_b.executor().run_until_parked();
 
+    // The additional edit is applied.
     buffer_a.read_with(cx_a, |buffer, _| {
         assert_eq!(
             buffer.text(),
@@ -516,9 +540,15 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
     cx_b.executor().run_until_parked();
 
     // When accepting the completion, the snippet is insert.
+    editor_b
+        .update(cx_b, |editor, cx| {
+            assert!(editor.context_menu_visible());
+            editor.confirm_completion(&ConfirmCompletion { item_ix: Some(0) }, cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
     editor_b.update(cx_b, |editor, cx| {
-        assert!(editor.context_menu_visible());
-        editor.confirm_completion(&ConfirmCompletion { item_ix: Some(0) }, cx);
         assert_eq!(
             editor.text(cx),
             "use d::SomeTrait;\nfn main() { a.first_method(); a.third_method(, , ) }"

crates/copilot/src/copilot_completion_provider.rs

@@ -363,10 +363,12 @@ mod tests {
 
             // Confirming a completion inserts it and hides the context menu, without showing
             // the copilot suggestion afterwards.
-            editor
-                .confirm_completion(&Default::default(), cx)
-                .unwrap()
-                .detach();
+            editor.confirm_completion(&Default::default(), cx).unwrap()
+        })
+        .await
+        .unwrap();
+
+        cx.update_editor(|editor, cx| {
             assert!(!editor.context_menu_visible());
             assert!(!editor.has_active_inline_completion(cx));
             assert_eq!(editor.text(cx), "one.completion_a\ntwo\nthree\n");

crates/editor/src/debounced_delay.rs

@@ -1,4 +1,4 @@
-use std::time::Duration;
+use std::{ops::ControlFlow, time::Duration};
 
 use futures::{channel::oneshot, FutureExt};
 use gpui::{Task, ViewContext};
@@ -7,7 +7,7 @@ use crate::Editor;
 
 pub struct DebouncedDelay {
     task: Option<Task<()>>,
-    cancel_channel: Option<oneshot::Sender<()>>,
+    cancel_channel: Option<oneshot::Sender<ControlFlow<()>>>,
 }
 
 impl DebouncedDelay {
@@ -23,17 +23,22 @@ impl DebouncedDelay {
         F: 'static + Send + FnOnce(&mut Editor, &mut ViewContext<Editor>) -> Task<()>,
     {
         if let Some(channel) = self.cancel_channel.take() {
-            _ = channel.send(());
+            channel.send(ControlFlow::Break(())).ok();
         }
 
-        let (sender, mut receiver) = oneshot::channel::<()>();
+        let (sender, mut receiver) = oneshot::channel::<ControlFlow<()>>();
         self.cancel_channel = Some(sender);
 
         drop(self.task.take());
         self.task = Some(cx.spawn(move |model, mut cx| async move {
             let mut timer = cx.background_executor().timer(delay).fuse();
             futures::select_biased! {
-                _ = receiver => return,
+                interrupt = receiver => {
+                    match interrupt {
+                        Ok(ControlFlow::Break(())) | Err(_) => return,
+                        Ok(ControlFlow::Continue(())) => {},
+                    }
+                }
                 _ = timer => {}
             }
 
@@ -42,4 +47,11 @@ impl DebouncedDelay {
             }
         }));
     }
+
+    pub fn start_now(&mut self) -> Option<Task<()>> {
+        if let Some(channel) = self.cancel_channel.take() {
+            channel.send(ControlFlow::Continue(())).ok();
+        }
+        self.task.take()
+    }
 }

crates/editor/src/editor.rs

@@ -4427,16 +4427,49 @@ impl Editor {
         &mut self,
         item_ix: Option<usize>,
         intent: CompletionIntent,
-        cx: &mut ViewContext<Editor>,
-    ) -> Option<Task<std::result::Result<(), anyhow::Error>>> {
-        use language::ToOffset as _;
-
+        cx: &mut ViewContext<Self>,
+    ) -> Option<Task<anyhow::Result<()>>> {
         let completions_menu = if let ContextMenu::Completions(menu) = self.hide_context_menu(cx)? {
             menu
         } else {
             return None;
         };
 
+        let mut resolve_task_store = completions_menu
+            .selected_completion_documentation_resolve_debounce
+            .lock();
+        let selected_completion_resolve = resolve_task_store.start_now();
+        let menu_pre_resolve = self
+            .completion_documentation_pre_resolve_debounce
+            .start_now();
+        drop(resolve_task_store);
+
+        Some(cx.spawn(|editor, mut cx| async move {
+            match (selected_completion_resolve, menu_pre_resolve) {
+                (None, None) => {}
+                (Some(resolve), None) | (None, Some(resolve)) => resolve.await,
+                (Some(resolve_1), Some(resolve_2)) => {
+                    futures::join!(resolve_1, resolve_2);
+                }
+            }
+            if let Some(apply_edits_task) = editor.update(&mut cx, |editor, cx| {
+                editor.apply_resolved_completion(completions_menu, item_ix, intent, cx)
+            })? {
+                apply_edits_task.await?;
+            }
+            Ok(())
+        }))
+    }
+
+    fn apply_resolved_completion(
+        &mut self,
+        completions_menu: CompletionsMenu,
+        item_ix: Option<usize>,
+        intent: CompletionIntent,
+        cx: &mut ViewContext<'_, Editor>,
+    ) -> Option<Task<anyhow::Result<Option<language::Transaction>>>> {
+        use language::ToOffset as _;
+
         let mat = completions_menu
             .matches
             .get(item_ix.unwrap_or(completions_menu.selected_item))?;
@@ -4595,11 +4628,7 @@ impl Editor {
             // so we should automatically call signature_help
             self.show_signature_help(&ShowSignatureHelp, cx);
         }
-
-        Some(cx.foreground_executor().spawn(async move {
-            apply_edits.await?;
-            Ok(())
-        }))
+        Some(apply_edits)
     }
 
     pub fn toggle_code_actions(&mut self, action: &ToggleCodeActions, cx: &mut ViewContext<Self>) {

crates/editor/src/editor_tests.rs

@@ -7996,7 +7996,7 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
             .unwrap()
     });
     cx.assert_editor_state(indoc! {"
-        one.second_completionˇ
+        one.ˇ
         two
         three
     "});
@@ -8029,9 +8029,9 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
     cx.assert_editor_state(indoc! {"
         one.second_completionˇ
         two
-        three
-        additional edit
-    "});
+        thoverlapping additional editree
+
+        additional edit"});
 
     cx.set_state(indoc! {"
         one.second_completion
@@ -8091,8 +8091,8 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
     });
     cx.assert_editor_state(indoc! {"
         one.second_completion
-        two sixth_completionˇ
-        three sixth_completionˇ
+        two siˇ
+        three siˇ
         additional edit
     "});
 
@@ -8133,9 +8133,11 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
             .confirm_completion(&ConfirmCompletion::default(), cx)
             .unwrap()
     });
-    cx.assert_editor_state("editor.closeˇ");
+    cx.assert_editor_state("editor.cloˇ");
     handle_resolve_completion_request(&mut cx, None).await;
     apply_additional_edits.await.unwrap();
+    cx.assert_editor_state(indoc! {"
+    editor.closeˇ"});
 }
 
 #[gpui::test]
@@ -10140,7 +10142,7 @@ async fn test_completions_with_additional_edits(cx: &mut gpui::TestAppContext) {
             .confirm_completion(&ConfirmCompletion::default(), cx)
             .unwrap()
     });
-    cx.assert_editor_state(indoc! {"fn main() { let a = 2.Some(2)ˇ; }"});
+    cx.assert_editor_state(indoc! {"fn main() { let a = 2.ˇ; }"});
 
     cx.handle_request::<lsp::request::ResolveCompletionItem, _, _>(move |_, _, _| {
         let task_completion_item = completion_item.clone();

crates/editor/src/hover_popover.rs

@@ -821,7 +821,7 @@ mod tests {
                 hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
                 completion_provider: Some(lsp::CompletionOptions {
                     trigger_characters: Some(vec![".".to_string(), ":".to_string()]),
-                    resolve_provider: Some(true),
+                    resolve_provider: Some(false),
                     ..Default::default()
                 }),
                 ..Default::default()
@@ -913,12 +913,15 @@ mod tests {
         assert_eq!(counter.load(atomic::Ordering::Acquire), 1);
 
         //apply a completion and check it was successfully applied
-        let _apply_additional_edits = cx.update_editor(|editor, cx| {
-            editor.context_menu_next(&Default::default(), cx);
-            editor
-                .confirm_completion(&ConfirmCompletion::default(), cx)
-                .unwrap()
-        });
+        let () = cx
+            .update_editor(|editor, cx| {
+                editor.context_menu_next(&Default::default(), cx);
+                editor
+                    .confirm_completion(&ConfirmCompletion::default(), cx)
+                    .unwrap()
+            })
+            .await
+            .unwrap();
         cx.assert_editor_state(indoc! {"
             one.second_completionˇ
             two

crates/editor/src/test/editor_test_context.rs

@@ -13,6 +13,7 @@ use itertools::Itertools;
 use language::{Buffer, BufferSnapshot, LanguageRegistry};
 use multi_buffer::{ExcerptRange, ToPoint};
 use parking_lot::RwLock;
+use pretty_assertions::assert_eq;
 use project::{FakeFs, Project};
 use std::{
     any::TypeId,

crates/vim/src/normal/repeat.rs

@@ -387,7 +387,7 @@ mod test {
             lsp::ServerCapabilities {
                 completion_provider: Some(lsp::CompletionOptions {
                     trigger_characters: Some(vec![".".to_string(), ":".to_string()]),
-                    resolve_provider: Some(true),
+                    resolve_provider: Some(false),
                     ..Default::default()
                 }),
                 ..Default::default()
@@ -432,7 +432,9 @@ mod test {
         request.next().await;
         cx.condition(|editor, _| editor.context_menu_visible())
             .await;
-        cx.simulate_keystrokes("down enter ! escape");
+        cx.simulate_keystrokes("down enter");
+        cx.run_until_parked();
+        cx.simulate_keystrokes("! escape");
 
         cx.assert_state(
             indoc! {"