Fix race condition between auto-indent and on-type-formatting (#32005)

Maxim created

This PR addresses to fix (#31308) a race condition where auto-indent (in
buffer.cs) and on-type-formatting (in lsp_store.rs) concurrently
calculate indentation using the same buffer snapshot.

Previous Solution (Abandoned): 
https://github.com/zed-industries/zed/pull/31340

Final Solution:
Delay applying on-type-formatting until auto-indent is complete.

Issue:

If AutoindentMode finishes first, formatting works correctly. If
"Formatting on typing" starts before AutoindentMode completes, it
results in double indentation.

Closes #31308

Release Notes:

- Fixed a race condition resulting in incorrect buffer contents when combining auto-indent and on-type-formatting

Change summary

crates/editor/src/editor_tests.rs                 | 52 +++++++++++++++++
crates/editor/src/test/editor_lsp_test_context.rs |  2 
crates/language/src/buffer.rs                     | 20 ++++++
crates/lsp/src/lsp.rs                             |  2 
crates/project/src/lsp_store.rs                   | 35 +++++++---
5 files changed, 97 insertions(+), 14 deletions(-)

Detailed changes

crates/editor/src/editor_tests.rs 🔗

@@ -14551,6 +14551,58 @@ async fn test_on_type_formatting_not_triggered(cx: &mut TestAppContext) {
     });
 }
 
+#[gpui::test(iterations = 20, seeds(31))]
+async fn test_on_type_formatting_is_applied_after_autoindent(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+
+    let mut cx = EditorLspTestContext::new_rust(
+        lsp::ServerCapabilities {
+            document_on_type_formatting_provider: Some(lsp::DocumentOnTypeFormattingOptions {
+                first_trigger_character: ".".to_string(),
+                more_trigger_character: None,
+            }),
+            ..Default::default()
+        },
+        cx,
+    )
+    .await;
+
+    cx.update_buffer(|buffer, _| {
+        // This causes autoindent to be async.
+        buffer.set_sync_parse_timeout(Duration::ZERO)
+    });
+
+    cx.set_state("fn c() {\n    d()ˇ\n}\n");
+    cx.simulate_keystroke("\n");
+    cx.run_until_parked();
+
+    let buffer_cloned =
+        cx.multibuffer(|multi_buffer, _| multi_buffer.as_singleton().unwrap().clone());
+    let mut request =
+        cx.set_request_handler::<lsp::request::OnTypeFormatting, _, _>(move |_, _, mut cx| {
+            let buffer_cloned = buffer_cloned.clone();
+            async move {
+                buffer_cloned.update(&mut cx, |buffer, _| {
+                    assert_eq!(
+                        buffer.text(),
+                        "fn c() {\n    d()\n        .\n}\n",
+                        "OnTypeFormatting should triggered after autoindent applied"
+                    )
+                })?;
+
+                Ok(Some(vec![]))
+            }
+        });
+
+    cx.simulate_keystroke(".");
+    cx.run_until_parked();
+
+    cx.assert_editor_state("fn c() {\n    d()\n        .ˇ\n}\n");
+    assert!(request.next().await.is_some());
+    request.close();
+    assert!(request.next().await.is_none());
+}
+
 #[gpui::test]
 async fn test_language_server_restart_due_to_settings_change(cx: &mut TestAppContext) {
     init_test(cx, |_| {});

crates/editor/src/test/editor_lsp_test_context.rs 🔗

@@ -351,7 +351,7 @@ impl EditorLspTestContext {
         T: 'static + request::Request,
         T::Params: 'static + Send,
         F: 'static + Send + FnMut(lsp::Url, T::Params, gpui::AsyncApp) -> Fut,
-        Fut: 'static + Send + Future<Output = Result<T::Result>>,
+        Fut: 'static + Future<Output = Result<T::Result>>,
     {
         let url = self.buffer_lsp_url.clone();
         self.lsp.set_request_handler::<T, _, _>(move |params, cx| {

crates/language/src/buffer.rs 🔗

@@ -106,6 +106,7 @@ pub struct Buffer {
     reload_task: Option<Task<Result<()>>>,
     language: Option<Arc<Language>>,
     autoindent_requests: Vec<Arc<AutoindentRequest>>,
+    wait_for_autoindent_txs: Vec<oneshot::Sender<()>>,
     pending_autoindent: Option<Task<()>>,
     sync_parse_timeout: Duration,
     syntax_map: Mutex<SyntaxMap>,
@@ -944,6 +945,7 @@ impl Buffer {
             sync_parse_timeout: Duration::from_millis(1),
             parse_status: watch::channel(ParseStatus::Idle),
             autoindent_requests: Default::default(),
+            wait_for_autoindent_txs: Default::default(),
             pending_autoindent: Default::default(),
             language: None,
             remote_selections: Default::default(),
@@ -1449,7 +1451,7 @@ impl Buffer {
         self.syntax_map.lock().contains_unknown_injections()
     }
 
-    #[cfg(test)]
+    #[cfg(any(test, feature = "test-support"))]
     pub fn set_sync_parse_timeout(&mut self, timeout: Duration) {
         self.sync_parse_timeout = timeout;
     }
@@ -1600,6 +1602,9 @@ impl Buffer {
             }
         } else {
             self.autoindent_requests.clear();
+            for tx in self.wait_for_autoindent_txs.drain(..) {
+                tx.send(()).ok();
+            }
         }
     }
 
@@ -1781,6 +1786,9 @@ impl Buffer {
         cx: &mut Context<Self>,
     ) {
         self.autoindent_requests.clear();
+        for tx in self.wait_for_autoindent_txs.drain(..) {
+            tx.send(()).ok();
+        }
 
         let edits: Vec<_> = indent_sizes
             .into_iter()
@@ -2120,6 +2128,16 @@ impl Buffer {
         self.text.give_up_waiting();
     }
 
+    pub fn wait_for_autoindent_applied(&mut self) -> Option<oneshot::Receiver<()>> {
+        let mut rx = None;
+        if !self.autoindent_requests.is_empty() {
+            let channel = oneshot::channel();
+            self.wait_for_autoindent_txs.push(channel.0);
+            rx = Some(channel.1);
+        }
+        rx
+    }
+
     /// Stores a set of selections that should be broadcasted to all of the buffer's replicas.
     pub fn set_active_selections(
         &mut self,

crates/lsp/src/lsp.rs 🔗

@@ -1593,7 +1593,7 @@ impl FakeLanguageServer {
         T: 'static + request::Request,
         T::Params: 'static + Send,
         F: 'static + Send + FnMut(T::Params, gpui::AsyncApp) -> Fut,
-        Fut: 'static + Send + Future<Output = Result<T::Result>>,
+        Fut: 'static + Future<Output = Result<T::Result>>,
     {
         let (responded_tx, responded_rx) = futures::channel::mpsc::unbounded();
         self.server.remove_request_handler::<T>();

crates/project/src/lsp_store.rs 🔗

@@ -5098,17 +5098,30 @@ impl LspStore {
                 .as_ref(),
             )
         });
-        self.request_lsp(
-            buffer.clone(),
-            LanguageServerToQuery::FirstCapable,
-            OnTypeFormatting {
-                position,
-                trigger,
-                options,
-                push_to_history,
-            },
-            cx,
-        )
+
+        cx.spawn(async move |this, cx| {
+            if let Some(waiter) =
+                buffer.update(cx, |buffer, _| buffer.wait_for_autoindent_applied())?
+            {
+                waiter.await?;
+            }
+            cx.update(|cx| {
+                this.update(cx, |this, cx| {
+                    this.request_lsp(
+                        buffer.clone(),
+                        LanguageServerToQuery::FirstCapable,
+                        OnTypeFormatting {
+                            position,
+                            trigger,
+                            options,
+                            push_to_history,
+                        },
+                        cx,
+                    )
+                })
+            })??
+            .await
+        })
     }
 
     pub fn code_actions(