Fix more inlay/excerpt race conditions (#28914)

Conrad Irwin created

Closes #ISSUE

Release Notes:

- N/A

Change summary

crates/diagnostics/src/diagnostics_tests.rs | 162 ++++++++++++++++++++++
crates/editor/src/display_map/inlay_map.rs  |   9 
2 files changed, 164 insertions(+), 7 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics_tests.rs 🔗

@@ -1,9 +1,9 @@
 use super::*;
 use collections::{HashMap, HashSet};
 use editor::{
-    DisplayPoint,
+    DisplayPoint, InlayId,
     actions::{GoToDiagnostic, GoToPreviousDiagnostic, MoveToBeginning},
-    display_map::DisplayRow,
+    display_map::{DisplayRow, Inlay},
     test::{editor_content_with_blocks, editor_test_context::EditorTestContext},
 };
 use gpui::{TestAppContext, VisualTestContext};
@@ -620,7 +620,7 @@ async fn test_diagnostics_multiple_servers(cx: &mut TestAppContext) {
 }
 
 #[gpui::test(iterations = 20)]
-async fn test_random_diagnostics(cx: &mut TestAppContext, mut rng: StdRng) {
+async fn test_random_diagnostics_blocks(cx: &mut TestAppContext, mut rng: StdRng) {
     init_test(cx);
 
     let operations = env::var("OPERATIONS")
@@ -779,6 +779,162 @@ async fn test_random_diagnostics(cx: &mut TestAppContext, mut rng: StdRng) {
     }
 }
 
+// similar to above, but with inlays. Used to find panics when mixing diagnostics and inlays.
+#[gpui::test]
+async fn test_random_diagnostics_with_inlays(cx: &mut TestAppContext, mut rng: StdRng) {
+    init_test(cx);
+
+    let operations = env::var("OPERATIONS")
+        .map(|i| i.parse().expect("invalid `OPERATIONS` variable"))
+        .unwrap_or(10);
+
+    let fs = FakeFs::new(cx.executor());
+    fs.insert_tree(path!("/test"), json!({})).await;
+
+    let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await;
+    let lsp_store = project.read_with(cx, |project, _| project.lsp_store());
+    let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx));
+    let cx = &mut VisualTestContext::from_window(*window, cx);
+    let workspace = window.root(cx).unwrap();
+
+    let mutated_diagnostics = window.build_entity(cx, |window, cx| {
+        ProjectDiagnosticsEditor::new(true, project.clone(), workspace.downgrade(), window, cx)
+    });
+
+    workspace.update_in(cx, |workspace, window, cx| {
+        workspace.add_item_to_center(Box::new(mutated_diagnostics.clone()), window, cx);
+    });
+    mutated_diagnostics.update_in(cx, |diagnostics, window, _cx| {
+        assert!(diagnostics.focus_handle.is_focused(window));
+    });
+
+    let mut next_id = 0;
+    let mut next_filename = 0;
+    let mut language_server_ids = vec![LanguageServerId(0)];
+    let mut updated_language_servers = HashSet::default();
+    let mut current_diagnostics: HashMap<(PathBuf, LanguageServerId), Vec<lsp::Diagnostic>> =
+        Default::default();
+    let mut next_inlay_id = 0;
+
+    for _ in 0..operations {
+        match rng.gen_range(0..100) {
+            // language server completes its diagnostic check
+            0..=20 if !updated_language_servers.is_empty() => {
+                let server_id = *updated_language_servers.iter().choose(&mut rng).unwrap();
+                log::info!("finishing diagnostic check for language server {server_id}");
+                lsp_store.update(cx, |lsp_store, cx| {
+                    lsp_store.disk_based_diagnostics_finished(server_id, cx)
+                });
+
+                if rng.gen_bool(0.5) {
+                    cx.run_until_parked();
+                }
+            }
+
+            21..=50 => mutated_diagnostics.update_in(cx, |diagnostics, window, cx| {
+                diagnostics.editor.update(cx, |editor, cx| {
+                    let snapshot = editor.snapshot(window, cx);
+                    if snapshot.buffer_snapshot.len() > 0 {
+                        let position = rng.gen_range(0..snapshot.buffer_snapshot.len());
+                        let position = snapshot.buffer_snapshot.clip_offset(position, Bias::Left);
+                        log::info!(
+                            "adding inlay at {position}/{}: {:?}",
+                            snapshot.buffer_snapshot.len(),
+                            snapshot.buffer_snapshot.text(),
+                        );
+
+                        editor.splice_inlays(
+                            &[],
+                            vec![Inlay {
+                                id: InlayId::InlineCompletion(post_inc(&mut next_inlay_id)),
+                                position: snapshot.buffer_snapshot.anchor_before(position),
+                                text: Rope::from(format!("Test inlay {next_inlay_id}")),
+                            }],
+                            cx,
+                        );
+                    }
+                });
+            }),
+
+            // language server updates diagnostics
+            _ => {
+                let (path, server_id, diagnostics) =
+                    match current_diagnostics.iter_mut().choose(&mut rng) {
+                        // update existing set of diagnostics
+                        Some(((path, server_id), diagnostics)) if rng.gen_bool(0.5) => {
+                            (path.clone(), *server_id, diagnostics)
+                        }
+
+                        // insert a set of diagnostics for a new path
+                        _ => {
+                            let path: PathBuf =
+                                format!(path!("/test/{}.rs"), post_inc(&mut next_filename)).into();
+                            let len = rng.gen_range(128..256);
+                            let content =
+                                RandomCharIter::new(&mut rng).take(len).collect::<String>();
+                            fs.insert_file(&path, content.into_bytes()).await;
+
+                            let server_id = match language_server_ids.iter().choose(&mut rng) {
+                                Some(server_id) if rng.gen_bool(0.5) => *server_id,
+                                _ => {
+                                    let id = LanguageServerId(language_server_ids.len());
+                                    language_server_ids.push(id);
+                                    id
+                                }
+                            };
+
+                            (
+                                path.clone(),
+                                server_id,
+                                current_diagnostics.entry((path, server_id)).or_default(),
+                            )
+                        }
+                    };
+
+                updated_language_servers.insert(server_id);
+
+                lsp_store.update(cx, |lsp_store, cx| {
+                    log::info!("updating diagnostics. language server {server_id} path {path:?}");
+                    randomly_update_diagnostics_for_path(
+                        &fs,
+                        &path,
+                        diagnostics,
+                        &mut next_id,
+                        &mut rng,
+                    );
+                    lsp_store
+                        .update_diagnostics(
+                            server_id,
+                            lsp::PublishDiagnosticsParams {
+                                uri: lsp::Url::from_file_path(&path).unwrap_or_else(|_| {
+                                    lsp::Url::parse("file:///test/fallback.rs").unwrap()
+                                }),
+                                diagnostics: diagnostics.clone(),
+                                version: None,
+                            },
+                            &[],
+                            cx,
+                        )
+                        .unwrap()
+                });
+                cx.executor()
+                    .advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10));
+
+                cx.run_until_parked();
+            }
+        }
+    }
+
+    log::info!("updating mutated diagnostics view");
+    mutated_diagnostics.update_in(cx, |diagnostics, window, cx| {
+        diagnostics.update_stale_excerpts(window, cx)
+    });
+
+    cx.executor()
+        .advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10));
+    cx.run_until_parked();
+}
+
 #[gpui::test]
 async fn active_diagnostics_dismiss_after_invalidation(cx: &mut TestAppContext) {
     init_test(cx);

crates/editor/src/display_map/inlay_map.rs 🔗

@@ -36,7 +36,7 @@ enum Transform {
 
 #[derive(Debug, Clone)]
 pub struct Inlay {
-    pub(crate) id: InlayId,
+    pub id: InlayId,
     pub position: Anchor,
     pub text: text::Rope,
 }
@@ -482,6 +482,9 @@ impl InlayMap {
                 };
 
                 for inlay in &self.inlays[start_ix..] {
+                    if !inlay.position.is_valid(&buffer_snapshot) {
+                        continue;
+                    }
                     let buffer_offset = inlay.position.to_offset(&buffer_snapshot);
                     if buffer_offset > buffer_edit.new.end {
                         break;
@@ -494,9 +497,7 @@ impl InlayMap {
                         buffer_snapshot.text_summary_for_range(prefix_start..prefix_end),
                     );
 
-                    if inlay.position.is_valid(&buffer_snapshot) {
-                        new_transforms.push(Transform::Inlay(inlay.clone()), &());
-                    }
+                    new_transforms.push(Transform::Inlay(inlay.clone()), &());
                 }
 
                 // Apply the rest of the edit.