From 9ea8a9a1d3a23609ef835ba8949bfa928374c9cd Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 16 Apr 2025 16:18:02 -0600 Subject: [PATCH] Fix more inlay/excerpt race conditions (#28914) Closes #ISSUE Release Notes: - N/A --- crates/diagnostics/src/diagnostics_tests.rs | 162 +++++++++++++++++++- crates/editor/src/display_map/inlay_map.rs | 9 +- 2 files changed, 164 insertions(+), 7 deletions(-) diff --git a/crates/diagnostics/src/diagnostics_tests.rs b/crates/diagnostics/src/diagnostics_tests.rs index 09d18166d9..896839ae24 100644 --- a/crates/diagnostics/src/diagnostics_tests.rs +++ b/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> = + 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::(); + 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); diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 925b7a6d4b..29a0dcd273 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/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.