From a35ef5b79f093998ca800ae14985c76a2f7994c9 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 9 Dec 2024 21:56:43 -0700 Subject: [PATCH] Fix diagnostics randomized tests (#21775) These were silently passing after the delay in updating diagnostics was added. Co-Authored-By: Max cc @someonetoignore Release Notes: - N/A --------- Co-authored-by: Max --- crates/diagnostics/src/diagnostics_tests.rs | 21 +++++++++-- crates/editor/src/editor_tests.rs | 14 ++++---- crates/project/src/lsp_store.rs | 13 +++++-- crates/project/src/project.rs | 40 ++++----------------- crates/project/src/project_tests.rs | 39 +++++++++++--------- 5 files changed, 66 insertions(+), 61 deletions(-) diff --git a/crates/diagnostics/src/diagnostics_tests.rs b/crates/diagnostics/src/diagnostics_tests.rs index 6ee1a90511..e399fe08f3 100644 --- a/crates/diagnostics/src/diagnostics_tests.rs +++ b/crates/diagnostics/src/diagnostics_tests.rs @@ -809,7 +809,7 @@ async fn test_random_diagnostics(cx: &mut TestAppContext, mut rng: StdRng) { updated_language_servers.insert(server_id); - project.update(cx, |project, cx| { + lsp_store.update(cx, |lsp_store, cx| { log::info!("updating diagnostics. language server {server_id} path {path:?}"); randomly_update_diagnostics_for_path( &fs, @@ -818,10 +818,12 @@ async fn test_random_diagnostics(cx: &mut TestAppContext, mut rng: StdRng) { &mut next_group_id, &mut rng, ); - project + lsp_store .update_diagnostic_entries(server_id, path, None, diagnostics.clone(), cx) .unwrap() }); + cx.executor() + .advance_clock(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10)); cx.run_until_parked(); } @@ -842,10 +844,25 @@ async fn test_random_diagnostics(cx: &mut TestAppContext, mut rng: StdRng) { cx, ) }); + cx.executor() + .advance_clock(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10)); cx.run_until_parked(); let mutated_excerpts = get_diagnostics_excerpts(&mutated_view, cx); let reference_excerpts = get_diagnostics_excerpts(&reference_view, cx); + + for ((path, language_server_id), diagnostics) in current_diagnostics { + for diagnostic in diagnostics { + let found_excerpt = reference_excerpts.iter().any(|info| { + let row_range = info.range.context.start.row..info.range.context.end.row; + info.path == path.strip_prefix("/test").unwrap() + && info.language_server == language_server_id + && row_range.contains(&diagnostic.range.start.0.row) + }); + assert!(found_excerpt, "diagnostic not found in reference view"); + } + } + assert_eq!(mutated_excerpts, reference_excerpts); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 0290340441..468eec6fc3 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9923,7 +9923,8 @@ async fn go_to_prev_overlapping_diagnostic( init_test(cx, |_| {}); let mut cx = EditorTestContext::new(cx).await; - let project = cx.update_editor(|editor, _| editor.project.clone().unwrap()); + let lsp_store = + cx.update_editor(|editor, cx| editor.project.as_ref().unwrap().read(cx).lsp_store()); cx.set_state(indoc! {" ˇfn func(abc def: i32) -> u32 { @@ -9931,8 +9932,8 @@ async fn go_to_prev_overlapping_diagnostic( "}); cx.update(|cx| { - project.update(cx, |project, cx| { - project + lsp_store.update(cx, |lsp_store, cx| { + lsp_store .update_diagnostics( LanguageServerId(0), lsp::PublishDiagnosticsParams { @@ -10021,11 +10022,12 @@ async fn test_diagnostics_with_links(cx: &mut TestAppContext) { fn func(abˇc def: i32) -> u32 { } "}); - let project = cx.update_editor(|editor, _| editor.project.clone().unwrap()); + let lsp_store = + cx.update_editor(|editor, cx| editor.project.as_ref().unwrap().read(cx).lsp_store()); cx.update(|cx| { - project.update(cx, |project, cx| { - project.update_diagnostics( + lsp_store.update(cx, |lsp_store, cx| { + lsp_store.update_diagnostics( LanguageServerId(0), lsp::PublishDiagnosticsParams { uri: lsp::Url::from_file_path("/root/file").unwrap(), diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index f719b91942..8f93e0bf67 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1013,7 +1013,7 @@ impl LspStore { ) { match event { BufferStoreEvent::BufferAdded(buffer) => { - self.register_buffer(buffer, cx).log_err(); + self.on_buffer_added(buffer, cx).log_err(); } BufferStoreEvent::BufferChangedFilePath { buffer, old_file } => { if let Some(old_file) = File::from_dyn(old_file.as_ref()) { @@ -1120,7 +1120,7 @@ impl LspStore { } } - fn register_buffer( + fn on_buffer_added( &mut self, buffer: &Model, cx: &mut ModelContext, @@ -2913,6 +2913,15 @@ impl LspStore { } } + pub fn diagnostic_summary(&self, include_ignored: bool, cx: &AppContext) -> DiagnosticSummary { + let mut summary = DiagnosticSummary::default(); + for (_, _, path_summary) in self.diagnostic_summaries(include_ignored, cx) { + summary.error_count += path_summary.error_count; + summary.warning_count += path_summary.warning_count; + } + summary + } + pub fn diagnostic_summaries<'a>( &'a self, include_ignored: bool, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 6ab800460e..b5fd4ba4a9 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -47,9 +47,9 @@ use gpui::{ use itertools::Itertools; use language::{ language_settings::InlayHintKind, proto::split_operations, Buffer, BufferEvent, - CachedLspAdapter, Capability, CodeLabel, DiagnosticEntry, Documentation, File as _, Language, - LanguageName, LanguageRegistry, PointUtf16, ToOffset, ToPointUtf16, Toolchain, ToolchainList, - Transaction, Unclipped, + CachedLspAdapter, Capability, CodeLabel, Documentation, File as _, Language, LanguageName, + LanguageRegistry, PointUtf16, ToOffset, ToPointUtf16, Toolchain, ToolchainList, Transaction, + Unclipped, }; use lsp::{ CodeActionKind, CompletionContext, CompletionItemKind, DocumentHighlightKind, LanguageServer, @@ -2568,31 +2568,6 @@ impl Project { .update(cx, |store, _| store.reset_last_formatting_failure()); } - pub fn update_diagnostics( - &mut self, - language_server_id: LanguageServerId, - params: lsp::PublishDiagnosticsParams, - disk_based_sources: &[String], - cx: &mut ModelContext, - ) -> Result<()> { - self.lsp_store.update(cx, |lsp_store, cx| { - lsp_store.update_diagnostics(language_server_id, params, disk_based_sources, cx) - }) - } - - pub fn update_diagnostic_entries( - &mut self, - server_id: LanguageServerId, - abs_path: PathBuf, - version: Option, - diagnostics: Vec>>, - cx: &mut ModelContext, - ) -> Result<(), anyhow::Error> { - self.lsp_store.update(cx, |lsp_store, cx| { - lsp_store.update_diagnostic_entries(server_id, abs_path, version, diagnostics, cx) - }) - } - pub fn reload_buffers( &self, buffers: HashSet>, @@ -3446,12 +3421,9 @@ impl Project { } pub fn diagnostic_summary(&self, include_ignored: bool, cx: &AppContext) -> DiagnosticSummary { - let mut summary = DiagnosticSummary::default(); - for (_, _, path_summary) in self.diagnostic_summaries(include_ignored, cx) { - summary.error_count += path_summary.error_count; - summary.warning_count += path_summary.warning_count; - } - summary + self.lsp_store + .read(cx) + .diagnostic_summary(include_ignored, cx) } pub fn diagnostic_summaries<'a>( diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 0bd681a588..ab6fb9d9f9 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -6,8 +6,9 @@ use gpui::{AppContext, SemanticVersion, UpdateGlobal}; use http_client::Url; use language::{ language_settings::{language_settings, AllLanguageSettings, LanguageSettingsContent}, - tree_sitter_rust, tree_sitter_typescript, Diagnostic, DiagnosticSet, DiskState, FakeLspAdapter, - LanguageConfig, LanguageMatcher, LanguageName, LineEnding, OffsetRangeExt, Point, ToPoint, + tree_sitter_rust, tree_sitter_typescript, Diagnostic, DiagnosticEntry, DiagnosticSet, + DiskState, FakeLspAdapter, LanguageConfig, LanguageMatcher, LanguageName, LineEnding, + OffsetRangeExt, Point, ToPoint, }; use lsp::{ notification::DidRenameFiles, DiagnosticSeverity, DocumentChanges, FileOperationFilter, @@ -987,6 +988,7 @@ async fn test_single_file_worktrees_diagnostics(cx: &mut gpui::TestAppContext) { .await; let project = Project::test(fs, ["/dir/a.rs".as_ref(), "/dir/b.rs".as_ref()], cx).await; + let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); let buffer_a = project .update(cx, |project, cx| project.open_local_buffer("/dir/a.rs", cx)) @@ -997,8 +999,8 @@ async fn test_single_file_worktrees_diagnostics(cx: &mut gpui::TestAppContext) { .await .unwrap(); - project.update(cx, |project, cx| { - project + lsp_store.update(cx, |lsp_store, cx| { + lsp_store .update_diagnostics( LanguageServerId(0), lsp::PublishDiagnosticsParams { @@ -1015,7 +1017,7 @@ async fn test_single_file_worktrees_diagnostics(cx: &mut gpui::TestAppContext) { cx, ) .unwrap(); - project + lsp_store .update_diagnostics( LanguageServerId(0), lsp::PublishDiagnosticsParams { @@ -1086,6 +1088,7 @@ async fn test_omitted_diagnostics(cx: &mut gpui::TestAppContext) { .await; let project = Project::test(fs, ["/root/dir".as_ref()], cx).await; + let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); let (worktree, _) = project .update(cx, |project, cx| { project.find_or_create_worktree("/root/dir", true, cx) @@ -1103,8 +1106,8 @@ async fn test_omitted_diagnostics(cx: &mut gpui::TestAppContext) { let other_worktree_id = worktree.update(cx, |tree, _| tree.id()); let server_id = LanguageServerId(0); - project.update(cx, |project, cx| { - project + lsp_store.update(cx, |lsp_store, cx| { + lsp_store .update_diagnostics( server_id, lsp::PublishDiagnosticsParams { @@ -1121,7 +1124,7 @@ async fn test_omitted_diagnostics(cx: &mut gpui::TestAppContext) { cx, ) .unwrap(); - project + lsp_store .update_diagnostics( server_id, lsp::PublishDiagnosticsParams { @@ -2018,9 +2021,9 @@ async fn test_empty_diagnostic_ranges(cx: &mut gpui::TestAppContext) { project.update(cx, |project, cx| { project.lsp_store.update(cx, |lsp_store, cx| { lsp_store - .update_buffer_diagnostics( - &buffer, + .update_diagnostic_entries( LanguageServerId(0), + PathBuf::from("/dir/a.rs"), None, vec![ DiagnosticEntry { @@ -2078,9 +2081,10 @@ async fn test_diagnostics_from_multiple_language_servers(cx: &mut gpui::TestAppC .await; let project = Project::test(fs, ["/dir".as_ref()], cx).await; + let lsp_store = project.read_with(cx, |project, _| project.lsp_store.clone()); - project.update(cx, |project, cx| { - project + lsp_store.update(cx, |lsp_store, cx| { + lsp_store .update_diagnostic_entries( LanguageServerId(0), Path::new("/dir/a.rs").to_owned(), @@ -2097,7 +2101,7 @@ async fn test_diagnostics_from_multiple_language_servers(cx: &mut gpui::TestAppC cx, ) .unwrap(); - project + lsp_store .update_diagnostic_entries( LanguageServerId(1), Path::new("/dir/a.rs").to_owned(), @@ -2116,7 +2120,7 @@ async fn test_diagnostics_from_multiple_language_servers(cx: &mut gpui::TestAppC .unwrap(); assert_eq!( - project.diagnostic_summary(false, cx), + lsp_store.diagnostic_summary(false, cx), DiagnosticSummary { error_count: 2, warning_count: 0, @@ -3698,6 +3702,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) { .await; let project = Project::test(fs.clone(), ["/the-dir".as_ref()], cx).await; + let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); let buffer = project .update(cx, |p, cx| p.open_local_buffer("/the-dir/a.rs", cx)) .await @@ -3791,9 +3796,9 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) { version: None, }; - project - .update(cx, |p, cx| { - p.update_diagnostics(LanguageServerId(0), message, &[], cx) + lsp_store + .update(cx, |lsp_store, cx| { + lsp_store.update_diagnostics(LanguageServerId(0), message, &[], cx) }) .unwrap(); let buffer = buffer.update(cx, |buffer, _| buffer.snapshot());