From 40fe5275cf7343596ca0130365529f3581f84944 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 25 Apr 2024 18:12:15 -0700 Subject: [PATCH] Rework project diagnostics to prevent showing inconsistent state (#10922) For a long time, we've had problems where diagnostics can end up showing up inconsistently in different views. This PR is my attempt to prevent that, and to simplify the system in the process. There are some UX changes. Diagnostic behaviors that have *not* changed: * In-buffer diagnostics update immediately when LSPs send diagnostics updates. * The diagnostic counts in the status bar indicator also update immediately. Diagnostic behaviors that this PR changes: * [x] The tab title for the project diagnostics view now simply shows the same counts as the status bar indicator - the project's current totals. Previously, this tab title showed something slightly different - the numbers of diagnostics *currently shown* in the diagnostics view's excerpts. But it was pretty confusing that you could sometimes see two different diagnostic counts. * [x] The project diagnostics view **never** updates its excerpts while the user might be in the middle of typing it that view, unless the user expressed an intent for the excerpts to update (by e.g. saving the buffer). This was the behavior we originally implemented, but has changed a few times since then, in attempts to fix other issues. I've restored that invariant. Times when the excerpts will update: * diagnostics are updated while the diagnostics view is not focused * the user changes focus away from the diagnostics view * the language server sends a `work done progress end` message for its disk-based diagnostics token (i.e. cargo check finishes) * the user saves a buffer associated with a language server, and then a debounce timer expires * [x] The project diagnostics view indicates when its diagnostics are stale. States: * when diagnostics have been updated while the diagnostics view was focused: * the indicator shows a 'refresh' icon * clicking the indicator updates the excerpts * when diagnostics have been updated, but a file has been saved, so that the diagnostics will soon update, the indicator is disabled With these UX changes, the only 'complex' part of the our diagnostics presentation is the Project Diagnostics view's excerpt management, because it needs to implement the deferred updates in order to avoid disrupting the user while they may be typing. I want to take some steps to reduce the potential for bugs in this view. * [x] Reduce the amount of state that the view uses, and simplify its implementation * [x] Add a randomized test that checks the invariant that a mutated diagnostics view matches a freshly computed diagnostics view ## Release Notes - Reworked the project diagnostics view: - Fixed an issue where the project diagnostics view could update its excerpts while you were typing in it. - Fixed bugs where the project diagnostics view could show the wrong excerpts. - Changed the diagnostics view to always update its excerpts eagerly when not focused. - Added an indicator to the project diagnostics view's toolbar, showing when diagnostics have been changed. --------- Co-authored-by: Richard Feldman --- Cargo.lock | 4 + crates/diagnostics/Cargo.toml | 4 + crates/diagnostics/src/diagnostics.rs | 1110 +++---------------- crates/diagnostics/src/diagnostics_tests.rs | 1008 +++++++++++++++++ crates/diagnostics/src/items.rs | 30 +- crates/diagnostics/src/toolbar_controls.rs | 67 +- crates/fs/src/fs.rs | 9 + crates/project/src/project.rs | 181 +-- 8 files changed, 1355 insertions(+), 1058 deletions(-) create mode 100644 crates/diagnostics/src/diagnostics_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 581375fae7..4c206e28c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3181,13 +3181,17 @@ dependencies = [ "anyhow", "client", "collections", + "ctor", "editor", + "env_logger", "futures 0.3.28", "gpui", "language", "log", "lsp", + "pretty_assertions", "project", + "rand 0.8.5", "schemars", "serde", "serde_json", diff --git a/crates/diagnostics/Cargo.toml b/crates/diagnostics/Cargo.toml index fcaacfd62a..48f05444e4 100644 --- a/crates/diagnostics/Cargo.toml +++ b/crates/diagnostics/Cargo.toml @@ -15,13 +15,16 @@ doctest = false [dependencies] anyhow.workspace = true collections.workspace = true +ctor.workspace = true editor.workspace = true +env_logger.workspace = true futures.workspace = true gpui.workspace = true language.workspace = true log.workspace = true lsp.workspace = true project.workspace = true +rand.workspace = true schemars.workspace = true serde.workspace = true settings.workspace = true @@ -40,3 +43,4 @@ serde_json.workspace = true theme = { workspace = true, features = ["test-support"] } unindent.workspace = true workspace = { workspace = true, features = ["test-support"] } +pretty_assertions.workspace = true diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 685c37ac60..b59e819db2 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -2,8 +2,11 @@ pub mod items; mod project_diagnostics_settings; mod toolbar_controls; -use anyhow::{Context as _, Result}; -use collections::{HashMap, HashSet}; +#[cfg(test)] +mod diagnostics_tests; + +use anyhow::Result; +use collections::{BTreeSet, HashSet}; use editor::{ diagnostic_block_renderer, display_map::{BlockDisposition, BlockId, BlockProperties, BlockStyle, RenderBlock}, @@ -11,7 +14,10 @@ use editor::{ scroll::Autoscroll, Editor, EditorEvent, ExcerptId, ExcerptRange, MultiBuffer, ToOffset, }; -use futures::future::try_join_all; +use futures::{ + channel::mpsc::{self, UnboundedSender}, + StreamExt as _, +}; use gpui::{ actions, div, svg, AnyElement, AnyView, AppContext, Context, EventEmitter, FocusHandle, FocusableView, HighlightStyle, InteractiveElement, IntoElement, Model, ParentElement, Render, @@ -19,8 +25,7 @@ use gpui::{ WeakView, WindowContext, }; use language::{ - Anchor, Bias, Buffer, Diagnostic, DiagnosticEntry, DiagnosticSeverity, Point, Selection, - SelectionGoal, + Bias, Buffer, Diagnostic, DiagnosticEntry, DiagnosticSeverity, Point, Selection, SelectionGoal, }; use lsp::LanguageServerId; use project::{DiagnosticSummary, Project, ProjectPath}; @@ -36,7 +41,7 @@ use std::{ use theme::ActiveTheme; pub use toolbar_controls::ToolbarControls; use ui::{h_flex, prelude::*, Icon, IconName, Label}; -use util::TryFutureExt; +use util::ResultExt; use workspace::{ item::{BreadcrumbText, Item, ItemEvent, ItemHandle, TabContentParams}, ItemNavHistory, Pane, ToolbarItemLocation, Workspace, @@ -58,11 +63,12 @@ struct ProjectDiagnosticsEditor { summary: DiagnosticSummary, excerpts: Model, path_states: Vec, - paths_to_update: HashMap>, - current_diagnostics: HashMap>, + paths_to_update: BTreeSet<(ProjectPath, LanguageServerId)>, include_warnings: bool, context: u32, - _subscriptions: Vec, + update_paths_tx: UnboundedSender<(ProjectPath, Option)>, + _update_excerpts_task: Task>, + _subscription: Subscription, } struct PathState { @@ -70,13 +76,6 @@ struct PathState { diagnostic_groups: Vec, } -#[derive(Clone, Debug, PartialEq)] -struct Jump { - path: ProjectPath, - position: Point, - anchor: Anchor, -} - struct DiagnosticGroupState { language_server_id: LanguageServerId, primary_diagnostic: DiagnosticEntry, @@ -122,40 +121,38 @@ impl ProjectDiagnosticsEditor { cx: &mut ViewContext, ) -> Self { let project_event_subscription = - cx.subscribe(&project_handle, |this, _, event, cx| match event { + cx.subscribe(&project_handle, |this, project, event, cx| match event { + project::Event::DiskBasedDiagnosticsStarted { .. } => { + cx.notify(); + } project::Event::DiskBasedDiagnosticsFinished { language_server_id } => { - log::debug!("Disk based diagnostics finished for server {language_server_id}"); - this.update_excerpts(Some(*language_server_id), cx); + log::debug!("disk based diagnostics finished for server {language_server_id}"); + this.enqueue_update_stale_excerpts(Some(*language_server_id)); } project::Event::DiagnosticsUpdated { language_server_id, path, } => { - log::debug!("Adding path {path:?} to update for server {language_server_id}"); this.paths_to_update - .entry(*language_server_id) - .or_default() - .insert(path.clone()); + .insert((path.clone(), *language_server_id)); + this.summary = project.read(cx).diagnostic_summary(false, cx); + cx.emit(EditorEvent::TitleChanged); - if this.is_dirty(cx) { - return; - } - let selections = this.editor.read(cx).selections.all::(cx); - if selections.len() < 2 - && selections - .first() - .map_or(true, |selection| selection.end == selection.start) - { - this.update_excerpts(Some(*language_server_id), cx); + if this.editor.read(cx).is_focused(cx) || this.focus_handle.is_focused(cx) { + log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. recording change"); + } else { + log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. updating excerpts"); + this.enqueue_update_stale_excerpts(Some(*language_server_id)); } } _ => {} }); let focus_handle = cx.focus_handle(); - - let focus_in_subscription = - cx.on_focus_in(&focus_handle, |diagnostics, cx| diagnostics.focus_in(cx)); + cx.on_focus_in(&focus_handle, |this, cx| this.focus_in(cx)) + .detach(); + cx.on_focus_out(&focus_handle, |this, cx| this.focus_out(cx)) + .detach(); let excerpts = cx.new_model(|cx| { MultiBuffer::new( @@ -169,35 +166,52 @@ impl ProjectDiagnosticsEditor { editor.set_vertical_scroll_margin(5, cx); editor }); - let editor_event_subscription = - cx.subscribe(&editor, |this, _editor, event: &EditorEvent, cx| { - cx.emit(event.clone()); - if event == &EditorEvent::Focused && this.path_states.is_empty() { - cx.focus(&this.focus_handle); + cx.subscribe(&editor, |this, _editor, event: &EditorEvent, cx| { + cx.emit(event.clone()); + match event { + EditorEvent::Focused => { + if this.path_states.is_empty() { + cx.focus(&this.focus_handle); + } } - }); + EditorEvent::Blurred => this.enqueue_update_stale_excerpts(None), + _ => {} + } + }) + .detach(); + + let (update_excerpts_tx, mut update_excerpts_rx) = mpsc::unbounded(); let project = project_handle.read(cx); - let summary = project.diagnostic_summary(false, cx); let mut this = Self { - project: project_handle, + project: project_handle.clone(), context, - summary, + summary: project.diagnostic_summary(false, cx), workspace, excerpts, focus_handle, editor, path_states: Default::default(), - paths_to_update: HashMap::default(), + paths_to_update: Default::default(), include_warnings: ProjectDiagnosticsSettings::get_global(cx).include_warnings, - current_diagnostics: HashMap::default(), - _subscriptions: vec![ - project_event_subscription, - editor_event_subscription, - focus_in_subscription, - ], + update_paths_tx: update_excerpts_tx, + _update_excerpts_task: cx.spawn(move |this, mut cx| async move { + while let Some((path, language_server_id)) = update_excerpts_rx.next().await { + if let Some(buffer) = project_handle + .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx))? + .await + .log_err() + { + this.update(&mut cx, |this, cx| { + this.update_excerpts(path, language_server_id, buffer, cx); + })?; + } + } + anyhow::Ok(()) + }), + _subscription: project_event_subscription, }; - this.update_excerpts(None, cx); + this.enqueue_update_all_excerpts(cx); this } @@ -228,8 +242,7 @@ impl ProjectDiagnosticsEditor { fn toggle_warnings(&mut self, _: &ToggleWarnings, cx: &mut ViewContext) { self.include_warnings = !self.include_warnings; - self.paths_to_update = self.current_diagnostics.clone(); - self.update_excerpts(None, cx); + self.enqueue_update_all_excerpts(cx); cx.notify(); } @@ -239,122 +252,65 @@ impl ProjectDiagnosticsEditor { } } - fn update_excerpts( - &mut self, - language_server_id: Option, - cx: &mut ViewContext, - ) { - log::debug!("Updating excerpts for server {language_server_id:?}"); - let mut paths_to_recheck = HashSet::default(); - let mut new_summaries: HashMap> = self - .project - .read(cx) - .diagnostic_summaries(false, cx) - .fold(HashMap::default(), |mut summaries, (path, server_id, _)| { - summaries.entry(server_id).or_default().insert(path); - summaries - }); - let mut old_diagnostics = if let Some(language_server_id) = language_server_id { - new_summaries.retain(|server_id, _| server_id == &language_server_id); - self.paths_to_update.retain(|server_id, paths| { - if server_id == &language_server_id { - paths_to_recheck.extend(paths.drain()); - false - } else { - true - } - }); - let mut old_diagnostics = HashMap::default(); - if let Some(new_paths) = new_summaries.get(&language_server_id) { - if let Some(old_paths) = self - .current_diagnostics - .insert(language_server_id, new_paths.clone()) - { - old_diagnostics.insert(language_server_id, old_paths); - } - } else { - if let Some(old_paths) = self.current_diagnostics.remove(&language_server_id) { - old_diagnostics.insert(language_server_id, old_paths); - } - } - old_diagnostics - } else { - paths_to_recheck.extend(self.paths_to_update.drain().flat_map(|(_, paths)| paths)); - mem::replace(&mut self.current_diagnostics, new_summaries.clone()) - }; - for (server_id, new_paths) in new_summaries { - match old_diagnostics.remove(&server_id) { - Some(mut old_paths) => { - paths_to_recheck.extend( - new_paths - .into_iter() - .filter(|new_path| !old_paths.remove(new_path)), - ); - paths_to_recheck.extend(old_paths); - } - None => paths_to_recheck.extend(new_paths), - } + fn focus_out(&mut self, cx: &mut ViewContext) { + if !self.focus_handle.is_focused(cx) && !self.editor.focus_handle(cx).is_focused(cx) { + self.enqueue_update_stale_excerpts(None); } - paths_to_recheck.extend(old_diagnostics.into_iter().flat_map(|(_, paths)| paths)); - - if paths_to_recheck.is_empty() { - log::debug!("No paths to recheck for language server {language_server_id:?}"); - return; - } - log::debug!( - "Rechecking {} paths for language server {:?}", - paths_to_recheck.len(), - language_server_id - ); - let project = self.project.clone(); - cx.spawn(|this, mut cx| { - async move { - let _: Vec<()> = try_join_all(paths_to_recheck.into_iter().map(|path| { - let mut cx = cx.clone(); - let project = project.clone(); - let this = this.clone(); - async move { - let buffer = project - .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx))? - .await - .with_context(|| format!("opening buffer for path {path:?}"))?; - this.update(&mut cx, |this, cx| { - this.populate_excerpts(path, language_server_id, buffer, cx); - }) - .context("missing project")?; - anyhow::Ok(()) - } - })) - .await - .context("rechecking diagnostics for paths")?; - - this.update(&mut cx, |this, cx| { - this.summary = this.project.read(cx).diagnostic_summary(false, cx); - cx.emit(EditorEvent::TitleChanged); - })?; - anyhow::Ok(()) - } - .log_err() - }) - .detach(); } - fn populate_excerpts( + /// Enqueue an update of all excerpts. Updates all paths that either + /// currently have diagnostics or are currently present in this view. + fn enqueue_update_all_excerpts(&mut self, cx: &mut ViewContext) { + self.project.update(cx, |project, cx| { + let mut paths = project + .diagnostic_summaries(false, cx) + .map(|(path, _, _)| path) + .collect::>(); + paths.extend(self.path_states.iter().map(|state| state.path.clone())); + for path in paths { + self.update_paths_tx.unbounded_send((path, None)).unwrap(); + } + }); + } + + /// Enqueue an update of the excerpts for any path whose diagnostics are known + /// to have changed. If a language server id is passed, then only the excerpts for + /// that language server's diagnostics will be updated. Otherwise, all stale excerpts + /// will be refreshed. + fn enqueue_update_stale_excerpts(&mut self, language_server_id: Option) { + for (path, server_id) in &self.paths_to_update { + if language_server_id.map_or(true, |id| id == *server_id) { + self.update_paths_tx + .unbounded_send((path.clone(), Some(*server_id))) + .unwrap(); + } + } + } + + fn update_excerpts( &mut self, - path: ProjectPath, - language_server_id: Option, + path_to_update: ProjectPath, + server_to_update: Option, buffer: Model, cx: &mut ViewContext, ) { + self.paths_to_update.retain(|(path, server_id)| { + *path != path_to_update + || server_to_update.map_or(false, |to_update| *server_id != to_update) + }); + let was_empty = self.path_states.is_empty(); let snapshot = buffer.read(cx).snapshot(); - let path_ix = match self.path_states.binary_search_by_key(&&path, |e| &e.path) { + let path_ix = match self + .path_states + .binary_search_by_key(&&path_to_update, |e| &e.path) + { Ok(ix) => ix, Err(ix) => { self.path_states.insert( ix, PathState { - path: path.clone(), + path: path_to_update.clone(), diagnostic_groups: Default::default(), }, ); @@ -373,8 +329,7 @@ impl ProjectDiagnosticsEditor { }; let path_state = &mut self.path_states[path_ix]; - let mut groups_to_add = Vec::new(); - let mut group_ixs_to_remove = Vec::new(); + let mut new_group_ixs = Vec::new(); let mut blocks_to_add = Vec::new(); let mut blocks_to_remove = HashSet::default(); let mut first_excerpt_id = None; @@ -383,10 +338,13 @@ impl ProjectDiagnosticsEditor { } else { DiagnosticSeverity::ERROR }; - let excerpts_snapshot = self.excerpts.update(cx, |excerpts, excerpts_cx| { - let mut old_groups = path_state.diagnostic_groups.iter().enumerate().peekable(); + let excerpts_snapshot = self.excerpts.update(cx, |excerpts, cx| { + let mut old_groups = mem::take(&mut path_state.diagnostic_groups) + .into_iter() + .enumerate() + .peekable(); let mut new_groups = snapshot - .diagnostic_groups(language_server_id) + .diagnostic_groups(server_to_update) .into_iter() .filter(|(_, group)| { group.entries[group.primary_ix].diagnostic.severity <= max_severity @@ -400,19 +358,20 @@ impl ProjectDiagnosticsEditor { (None, None) => break, (None, Some(_)) => to_insert = new_groups.next(), (Some((_, old_group)), None) => { - if language_server_id.map_or(true, |id| id == old_group.language_server_id) - { + if server_to_update.map_or(true, |id| id == old_group.language_server_id) { to_remove = old_groups.next(); } else { to_keep = old_groups.next(); } } - (Some((_, old_group)), Some((_, new_group))) => { + (Some((_, old_group)), Some((new_language_server_id, new_group))) => { let old_primary = &old_group.primary_diagnostic; let new_primary = &new_group.entries[new_group.primary_ix]; - match compare_diagnostics(old_primary, new_primary, &snapshot) { + match compare_diagnostics(old_primary, new_primary, &snapshot) + .then_with(|| old_group.language_server_id.cmp(new_language_server_id)) + { Ordering::Less => { - if language_server_id + if server_to_update .map_or(true, |id| id == old_group.language_server_id) { to_remove = old_groups.next(); @@ -456,6 +415,7 @@ impl ProjectDiagnosticsEditor { Point::new(range.end.row + self.context, u32::MAX), Bias::Left, ); + let excerpt_id = excerpts .insert_excerpts_after( prev_excerpt_id, @@ -464,7 +424,7 @@ impl ProjectDiagnosticsEditor { context: excerpt_start..excerpt_end, primary: Some(range.clone()), }], - excerpts_cx, + cx, ) .pop() .unwrap(); @@ -518,18 +478,19 @@ impl ProjectDiagnosticsEditor { } } - groups_to_add.push(group_state); - } else if let Some((group_ix, group_state)) = to_remove { - excerpts.remove_excerpts(group_state.excerpts.iter().copied(), excerpts_cx); - group_ixs_to_remove.push(group_ix); + new_group_ixs.push(path_state.diagnostic_groups.len()); + path_state.diagnostic_groups.push(group_state); + } else if let Some((_, group_state)) = to_remove { + excerpts.remove_excerpts(group_state.excerpts.iter().copied(), cx); blocks_to_remove.extend(group_state.blocks.iter().copied()); - } else if let Some((_, group)) = to_keep { - prev_excerpt_id = *group.excerpts.last().unwrap(); + } else if let Some((_, group_state)) = to_keep { + prev_excerpt_id = *group_state.excerpts.last().unwrap(); first_excerpt_id.get_or_insert_with(|| prev_excerpt_id); + path_state.diagnostic_groups.push(group_state); } } - excerpts.snapshot(excerpts_cx) + excerpts.snapshot(cx) }); self.editor.update(cx, |editor, cx| { @@ -550,24 +511,12 @@ impl ProjectDiagnosticsEditor { ); let mut block_ids = block_ids.into_iter(); - for group_state in &mut groups_to_add { + for ix in new_group_ixs { + let group_state = &mut path_state.diagnostic_groups[ix]; group_state.blocks = block_ids.by_ref().take(group_state.block_count).collect(); } }); - for ix in group_ixs_to_remove.into_iter().rev() { - path_state.diagnostic_groups.remove(ix); - } - path_state.diagnostic_groups.extend(groups_to_add); - path_state.diagnostic_groups.sort_unstable_by(|a, b| { - let range_a = &a.primary_diagnostic.range; - let range_b = &b.primary_diagnostic.range; - range_a - .start - .cmp(&range_b.start, &snapshot) - .then_with(|| range_a.end.cmp(&range_b.end, &snapshot)) - }); - if path_state.diagnostic_groups.is_empty() { self.path_states.remove(path_ix); } @@ -634,8 +583,32 @@ impl ProjectDiagnosticsEditor { let focus_handle = self.editor.focus_handle(cx); cx.focus(&focus_handle); } + + #[cfg(test)] + self.check_invariants(cx); + cx.notify(); } + + #[cfg(test)] + fn check_invariants(&self, cx: &mut ViewContext) { + let mut excerpts = Vec::new(); + for (id, buffer, _) in self.excerpts.read(cx).snapshot(cx).excerpts() { + if let Some(file) = buffer.file() { + excerpts.push((id, file.path().clone())); + } + } + + let mut prev_path = None; + for (_, path) in &excerpts { + if let Some(prev_path) = prev_path { + if path < prev_path { + panic!("excerpts are not sorted by path {:?}", excerpts); + } + } + prev_path = Some(path); + } + } } impl FocusableView for ProjectDiagnosticsEditor { @@ -904,762 +877,3 @@ fn compare_diagnostics( }) .then_with(|| lhs.diagnostic.message.cmp(&rhs.diagnostic.message)) } - -#[cfg(test)] -mod tests { - use super::*; - use editor::{ - display_map::{BlockContext, TransformBlock}, - DisplayPoint, GutterDimensions, - }; - use gpui::{px, AvailableSpace, Stateful, TestAppContext, VisualTestContext}; - use language::{Diagnostic, DiagnosticEntry, DiagnosticSeverity, PointUtf16, Unclipped}; - use project::FakeFs; - use serde_json::json; - use settings::SettingsStore; - use unindent::Unindent as _; - - #[gpui::test] - async fn test_diagnostics(cx: &mut TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/test", - json!({ - "consts.rs": " - const a: i32 = 'a'; - const b: i32 = c; - " - .unindent(), - - "main.rs": " - fn main() { - let x = vec![]; - let y = vec![]; - a(x); - b(y); - // comment 1 - // comment 2 - c(y); - d(x); - } - " - .unindent(), - }), - ) - .await; - - let language_server_id = LanguageServerId(0); - let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; - let window = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); - let cx = &mut VisualTestContext::from_window(*window, cx); - let workspace = window.root(cx).unwrap(); - - // Create some diagnostics - project.update(cx, |project, cx| { - project - .update_diagnostic_entries( - language_server_id, - PathBuf::from("/test/main.rs"), - None, - vec![ - DiagnosticEntry { - range: Unclipped(PointUtf16::new(1, 8))..Unclipped(PointUtf16::new(1, 9)), - diagnostic: Diagnostic { - message: - "move occurs because `x` has type `Vec`, which does not implement the `Copy` trait" - .to_string(), - severity: DiagnosticSeverity::INFORMATION, - is_primary: false, - is_disk_based: true, - group_id: 1, - ..Default::default() - }, - }, - DiagnosticEntry { - range: Unclipped(PointUtf16::new(2, 8))..Unclipped(PointUtf16::new(2, 9)), - diagnostic: Diagnostic { - message: - "move occurs because `y` has type `Vec`, which does not implement the `Copy` trait" - .to_string(), - severity: DiagnosticSeverity::INFORMATION, - is_primary: false, - is_disk_based: true, - group_id: 0, - ..Default::default() - }, - }, - DiagnosticEntry { - range: Unclipped(PointUtf16::new(3, 6))..Unclipped(PointUtf16::new(3, 7)), - diagnostic: Diagnostic { - message: "value moved here".to_string(), - severity: DiagnosticSeverity::INFORMATION, - is_primary: false, - is_disk_based: true, - group_id: 1, - ..Default::default() - }, - }, - DiagnosticEntry { - range: Unclipped(PointUtf16::new(4, 6))..Unclipped(PointUtf16::new(4, 7)), - diagnostic: Diagnostic { - message: "value moved here".to_string(), - severity: DiagnosticSeverity::INFORMATION, - is_primary: false, - is_disk_based: true, - group_id: 0, - ..Default::default() - }, - }, - DiagnosticEntry { - range: Unclipped(PointUtf16::new(7, 6))..Unclipped(PointUtf16::new(7, 7)), - diagnostic: Diagnostic { - message: "use of moved value\nvalue used here after move".to_string(), - severity: DiagnosticSeverity::ERROR, - is_primary: true, - is_disk_based: true, - group_id: 0, - ..Default::default() - }, - }, - DiagnosticEntry { - range: Unclipped(PointUtf16::new(8, 6))..Unclipped(PointUtf16::new(8, 7)), - diagnostic: Diagnostic { - message: "use of moved value\nvalue used here after move".to_string(), - severity: DiagnosticSeverity::ERROR, - is_primary: true, - is_disk_based: true, - group_id: 1, - ..Default::default() - }, - }, - ], - cx, - ) - .unwrap(); - }); - - // Open the project diagnostics view while there are already diagnostics. - let view = window.build_view(cx, |cx| { - ProjectDiagnosticsEditor::new_with_context( - 1, - project.clone(), - workspace.downgrade(), - cx, - ) - }); - let editor = view.update(cx, |view, _| view.editor.clone()); - - view.next_notification(cx).await; - assert_eq!( - editor_blocks(&editor, cx), - [ - (0, "path header block".into()), - (2, "diagnostic header".into()), - (15, "collapsed context".into()), - (16, "diagnostic header".into()), - (25, "collapsed context".into()), - ] - ); - assert_eq!( - editor.update(cx, |editor, cx| editor.display_text(cx)), - concat!( - // - // main.rs - // - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - " let x = vec![];\n", - " let y = vec![];\n", - "\n", // supporting diagnostic - " a(x);\n", - " b(y);\n", - "\n", // supporting diagnostic - " // comment 1\n", - " // comment 2\n", - " c(y);\n", - "\n", // supporting diagnostic - " d(x);\n", - "\n", // context ellipsis - // diagnostic group 2 - "\n", // primary message - "\n", // padding - "fn main() {\n", - " let x = vec![];\n", - "\n", // supporting diagnostic - " let y = vec![];\n", - " a(x);\n", - "\n", // supporting diagnostic - " b(y);\n", - "\n", // context ellipsis - " c(y);\n", - " d(x);\n", - "\n", // supporting diagnostic - "}" - ) - ); - - // Cursor is at the first diagnostic - editor.update(cx, |editor, cx| { - assert_eq!( - editor.selections.display_ranges(cx), - [DisplayPoint::new(12, 6)..DisplayPoint::new(12, 6)] - ); - }); - - // Diagnostics are added for another earlier path. - project.update(cx, |project, cx| { - project.disk_based_diagnostics_started(language_server_id, cx); - project - .update_diagnostic_entries( - language_server_id, - PathBuf::from("/test/consts.rs"), - None, - vec![DiagnosticEntry { - range: Unclipped(PointUtf16::new(0, 15))..Unclipped(PointUtf16::new(0, 15)), - diagnostic: Diagnostic { - message: "mismatched types\nexpected `usize`, found `char`".to_string(), - severity: DiagnosticSeverity::ERROR, - is_primary: true, - is_disk_based: true, - group_id: 0, - ..Default::default() - }, - }], - cx, - ) - .unwrap(); - project.disk_based_diagnostics_finished(language_server_id, cx); - }); - - view.next_notification(cx).await; - assert_eq!( - editor_blocks(&editor, cx), - [ - (0, "path header block".into()), - (2, "diagnostic header".into()), - (7, "path header block".into()), - (9, "diagnostic header".into()), - (22, "collapsed context".into()), - (23, "diagnostic header".into()), - (32, "collapsed context".into()), - ] - ); - - assert_eq!( - editor.update(cx, |editor, cx| editor.display_text(cx)), - concat!( - // - // consts.rs - // - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - "const a: i32 = 'a';\n", - "\n", // supporting diagnostic - "const b: i32 = c;\n", - // - // main.rs - // - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - " let x = vec![];\n", - " let y = vec![];\n", - "\n", // supporting diagnostic - " a(x);\n", - " b(y);\n", - "\n", // supporting diagnostic - " // comment 1\n", - " // comment 2\n", - " c(y);\n", - "\n", // supporting diagnostic - " d(x);\n", - "\n", // collapsed context - // diagnostic group 2 - "\n", // primary message - "\n", // filename - "fn main() {\n", - " let x = vec![];\n", - "\n", // supporting diagnostic - " let y = vec![];\n", - " a(x);\n", - "\n", // supporting diagnostic - " b(y);\n", - "\n", // context ellipsis - " c(y);\n", - " d(x);\n", - "\n", // supporting diagnostic - "}" - ) - ); - - // Cursor keeps its position. - editor.update(cx, |editor, cx| { - assert_eq!( - editor.selections.display_ranges(cx), - [DisplayPoint::new(19, 6)..DisplayPoint::new(19, 6)] - ); - }); - - // Diagnostics are added to the first path - project.update(cx, |project, cx| { - project.disk_based_diagnostics_started(language_server_id, cx); - project - .update_diagnostic_entries( - language_server_id, - PathBuf::from("/test/consts.rs"), - None, - vec![ - DiagnosticEntry { - range: Unclipped(PointUtf16::new(0, 15)) - ..Unclipped(PointUtf16::new(0, 15)), - diagnostic: Diagnostic { - message: "mismatched types\nexpected `usize`, found `char`" - .to_string(), - severity: DiagnosticSeverity::ERROR, - is_primary: true, - is_disk_based: true, - group_id: 0, - ..Default::default() - }, - }, - DiagnosticEntry { - range: Unclipped(PointUtf16::new(1, 15)) - ..Unclipped(PointUtf16::new(1, 15)), - diagnostic: Diagnostic { - message: "unresolved name `c`".to_string(), - severity: DiagnosticSeverity::ERROR, - is_primary: true, - is_disk_based: true, - group_id: 1, - ..Default::default() - }, - }, - ], - cx, - ) - .unwrap(); - project.disk_based_diagnostics_finished(language_server_id, cx); - }); - - view.next_notification(cx).await; - assert_eq!( - editor_blocks(&editor, cx), - [ - (0, "path header block".into()), - (2, "diagnostic header".into()), - (7, "collapsed context".into()), - (8, "diagnostic header".into()), - (13, "path header block".into()), - (15, "diagnostic header".into()), - (28, "collapsed context".into()), - (29, "diagnostic header".into()), - (38, "collapsed context".into()), - ] - ); - - assert_eq!( - editor.update(cx, |editor, cx| editor.display_text(cx)), - concat!( - // - // consts.rs - // - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - "const a: i32 = 'a';\n", - "\n", // supporting diagnostic - "const b: i32 = c;\n", - "\n", // context ellipsis - // diagnostic group 2 - "\n", // primary message - "\n", // padding - "const a: i32 = 'a';\n", - "const b: i32 = c;\n", - "\n", // supporting diagnostic - // - // main.rs - // - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - " let x = vec![];\n", - " let y = vec![];\n", - "\n", // supporting diagnostic - " a(x);\n", - " b(y);\n", - "\n", // supporting diagnostic - " // comment 1\n", - " // comment 2\n", - " c(y);\n", - "\n", // supporting diagnostic - " d(x);\n", - "\n", // context ellipsis - // diagnostic group 2 - "\n", // primary message - "\n", // filename - "fn main() {\n", - " let x = vec![];\n", - "\n", // supporting diagnostic - " let y = vec![];\n", - " a(x);\n", - "\n", // supporting diagnostic - " b(y);\n", - "\n", // context ellipsis - " c(y);\n", - " d(x);\n", - "\n", // supporting diagnostic - "}" - ) - ); - } - - #[gpui::test] - async fn test_diagnostics_multiple_servers(cx: &mut TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/test", - json!({ - "main.js": " - a(); - b(); - c(); - d(); - e(); - ".unindent() - }), - ) - .await; - - let server_id_1 = LanguageServerId(100); - let server_id_2 = LanguageServerId(101); - let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; - let window = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); - let cx = &mut VisualTestContext::from_window(*window, cx); - let workspace = window.root(cx).unwrap(); - - let view = window.build_view(cx, |cx| { - ProjectDiagnosticsEditor::new_with_context( - 1, - project.clone(), - workspace.downgrade(), - cx, - ) - }); - let editor = view.update(cx, |view, _| view.editor.clone()); - - // Two language servers start updating diagnostics - project.update(cx, |project, cx| { - project.disk_based_diagnostics_started(server_id_1, cx); - project.disk_based_diagnostics_started(server_id_2, cx); - project - .update_diagnostic_entries( - server_id_1, - PathBuf::from("/test/main.js"), - None, - vec![DiagnosticEntry { - range: Unclipped(PointUtf16::new(0, 0))..Unclipped(PointUtf16::new(0, 1)), - diagnostic: Diagnostic { - message: "error 1".to_string(), - severity: DiagnosticSeverity::WARNING, - is_primary: true, - is_disk_based: true, - group_id: 1, - ..Default::default() - }, - }], - cx, - ) - .unwrap(); - }); - - // The first language server finishes - project.update(cx, |project, cx| { - project.disk_based_diagnostics_finished(server_id_1, cx); - }); - - // Only the first language server's diagnostics are shown. - cx.executor().run_until_parked(); - assert_eq!( - editor_blocks(&editor, cx), - [ - (0, "path header block".into()), - (2, "diagnostic header".into()), - ] - ); - assert_eq!( - editor.update(cx, |editor, cx| editor.display_text(cx)), - concat!( - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - "a();\n", // - "b();", - ) - ); - - // The second language server finishes - project.update(cx, |project, cx| { - project - .update_diagnostic_entries( - server_id_2, - PathBuf::from("/test/main.js"), - None, - vec![DiagnosticEntry { - range: Unclipped(PointUtf16::new(1, 0))..Unclipped(PointUtf16::new(1, 1)), - diagnostic: Diagnostic { - message: "warning 1".to_string(), - severity: DiagnosticSeverity::ERROR, - is_primary: true, - is_disk_based: true, - group_id: 2, - ..Default::default() - }, - }], - cx, - ) - .unwrap(); - project.disk_based_diagnostics_finished(server_id_2, cx); - }); - - // Both language server's diagnostics are shown. - cx.executor().run_until_parked(); - assert_eq!( - editor_blocks(&editor, cx), - [ - (0, "path header block".into()), - (2, "diagnostic header".into()), - (6, "collapsed context".into()), - (7, "diagnostic header".into()), - ] - ); - assert_eq!( - editor.update(cx, |editor, cx| editor.display_text(cx)), - concat!( - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - "a();\n", // location - "b();\n", // - "\n", // collapsed context - // diagnostic group 2 - "\n", // primary message - "\n", // padding - "a();\n", // context - "b();\n", // - "c();", // context - ) - ); - - // Both language servers start updating diagnostics, and the first server finishes. - project.update(cx, |project, cx| { - project.disk_based_diagnostics_started(server_id_1, cx); - project.disk_based_diagnostics_started(server_id_2, cx); - project - .update_diagnostic_entries( - server_id_1, - PathBuf::from("/test/main.js"), - None, - vec![DiagnosticEntry { - range: Unclipped(PointUtf16::new(2, 0))..Unclipped(PointUtf16::new(2, 1)), - diagnostic: Diagnostic { - message: "warning 2".to_string(), - severity: DiagnosticSeverity::WARNING, - is_primary: true, - is_disk_based: true, - group_id: 1, - ..Default::default() - }, - }], - cx, - ) - .unwrap(); - project - .update_diagnostic_entries( - server_id_2, - PathBuf::from("/test/main.rs"), - None, - vec![], - cx, - ) - .unwrap(); - project.disk_based_diagnostics_finished(server_id_1, cx); - }); - - // Only the first language server's diagnostics are updated. - cx.executor().run_until_parked(); - assert_eq!( - editor_blocks(&editor, cx), - [ - (0, "path header block".into()), - (2, "diagnostic header".into()), - (7, "collapsed context".into()), - (8, "diagnostic header".into()), - ] - ); - assert_eq!( - editor.update(cx, |editor, cx| editor.display_text(cx)), - concat!( - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - "a();\n", // location - "b();\n", // - "c();\n", // context - "\n", // collapsed context - // diagnostic group 2 - "\n", // primary message - "\n", // padding - "b();\n", // context - "c();\n", // - "d();", // context - ) - ); - - // The second language server finishes. - project.update(cx, |project, cx| { - project - .update_diagnostic_entries( - server_id_2, - PathBuf::from("/test/main.js"), - None, - vec![DiagnosticEntry { - range: Unclipped(PointUtf16::new(3, 0))..Unclipped(PointUtf16::new(3, 1)), - diagnostic: Diagnostic { - message: "warning 2".to_string(), - severity: DiagnosticSeverity::WARNING, - is_primary: true, - is_disk_based: true, - group_id: 1, - ..Default::default() - }, - }], - cx, - ) - .unwrap(); - project.disk_based_diagnostics_finished(server_id_2, cx); - }); - - // Both language servers' diagnostics are updated. - cx.executor().run_until_parked(); - assert_eq!( - editor_blocks(&editor, cx), - [ - (0, "path header block".into()), - (2, "diagnostic header".into()), - (7, "collapsed context".into()), - (8, "diagnostic header".into()), - ] - ); - assert_eq!( - editor.update(cx, |editor, cx| editor.display_text(cx)), - concat!( - "\n", // filename - "\n", // padding - // diagnostic group 1 - "\n", // primary message - "\n", // padding - "b();\n", // location - "c();\n", // - "d();\n", // context - "\n", // collapsed context - // diagnostic group 2 - "\n", // primary message - "\n", // padding - "c();\n", // context - "d();\n", // - "e();", // context - ) - ); - } - - fn init_test(cx: &mut TestAppContext) { - cx.update(|cx| { - let settings = SettingsStore::test(cx); - cx.set_global(settings); - theme::init(theme::LoadThemes::JustBase, cx); - language::init(cx); - client::init_settings(cx); - workspace::init_settings(cx); - Project::init_settings(cx); - crate::init(cx); - editor::init(cx); - }); - } - - fn editor_blocks( - editor: &View, - cx: &mut VisualTestContext, - ) -> Vec<(u32, SharedString)> { - let mut blocks = Vec::new(); - cx.draw(gpui::Point::default(), AvailableSpace::min_size(), |cx| { - editor.update(cx, |editor, cx| { - let snapshot = editor.snapshot(cx); - blocks.extend( - snapshot - .blocks_in_range(0..snapshot.max_point().row()) - .enumerate() - .filter_map(|(ix, (row, block))| { - let name: SharedString = match block { - TransformBlock::Custom(block) => { - let mut element = block.render(&mut BlockContext { - context: cx, - anchor_x: px(0.), - gutter_dimensions: &GutterDimensions::default(), - line_height: px(0.), - em_width: px(0.), - max_width: px(0.), - block_id: ix, - editor_style: &editor::EditorStyle::default(), - }); - let element = element.downcast_mut::>().unwrap(); - element - .interactivity() - .element_id - .clone()? - .try_into() - .ok()? - } - - TransformBlock::ExcerptHeader { - starts_new_buffer, .. - } => { - if *starts_new_buffer { - "path header block".into() - } else { - "collapsed context".into() - } - } - }; - - Some((row, name)) - }), - ) - }); - - div().into_any() - }); - blocks - } -} diff --git a/crates/diagnostics/src/diagnostics_tests.rs b/crates/diagnostics/src/diagnostics_tests.rs new file mode 100644 index 0000000000..3e7b4b67f2 --- /dev/null +++ b/crates/diagnostics/src/diagnostics_tests.rs @@ -0,0 +1,1008 @@ +use super::*; +use collections::HashMap; +use editor::{ + display_map::{BlockContext, TransformBlock}, + DisplayPoint, GutterDimensions, +}; +use gpui::{px, AvailableSpace, Stateful, TestAppContext, VisualTestContext}; +use language::{ + Diagnostic, DiagnosticEntry, DiagnosticSeverity, OffsetRangeExt, PointUtf16, Rope, Unclipped, +}; +use pretty_assertions::assert_eq; +use project::FakeFs; +use rand::{rngs::StdRng, seq::IteratorRandom as _, Rng}; +use serde_json::json; +use settings::SettingsStore; +use std::{env, path::Path}; +use unindent::Unindent as _; +use util::{post_inc, RandomCharIter}; + +#[ctor::ctor] +fn init_logger() { + if env::var("RUST_LOG").is_ok() { + env_logger::init(); + } +} + +#[gpui::test] +async fn test_diagnostics(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/test", + json!({ + "consts.rs": " + const a: i32 = 'a'; + const b: i32 = c; + " + .unindent(), + + "main.rs": " + fn main() { + let x = vec![]; + let y = vec![]; + a(x); + b(y); + // comment 1 + // comment 2 + c(y); + d(x); + } + " + .unindent(), + }), + ) + .await; + + let language_server_id = LanguageServerId(0); + let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; + let window = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*window, cx); + let workspace = window.root(cx).unwrap(); + + // Create some diagnostics + project.update(cx, |project, cx| { + project + .update_diagnostic_entries( + language_server_id, + PathBuf::from("/test/main.rs"), + None, + vec![ + DiagnosticEntry { + range: Unclipped(PointUtf16::new(1, 8))..Unclipped(PointUtf16::new(1, 9)), + diagnostic: Diagnostic { + message: + "move occurs because `x` has type `Vec`, which does not implement the `Copy` trait" + .to_string(), + severity: DiagnosticSeverity::INFORMATION, + is_primary: false, + is_disk_based: true, + group_id: 1, + ..Default::default() + }, + }, + DiagnosticEntry { + range: Unclipped(PointUtf16::new(2, 8))..Unclipped(PointUtf16::new(2, 9)), + diagnostic: Diagnostic { + message: + "move occurs because `y` has type `Vec`, which does not implement the `Copy` trait" + .to_string(), + severity: DiagnosticSeverity::INFORMATION, + is_primary: false, + is_disk_based: true, + group_id: 0, + ..Default::default() + }, + }, + DiagnosticEntry { + range: Unclipped(PointUtf16::new(3, 6))..Unclipped(PointUtf16::new(3, 7)), + diagnostic: Diagnostic { + message: "value moved here".to_string(), + severity: DiagnosticSeverity::INFORMATION, + is_primary: false, + is_disk_based: true, + group_id: 1, + ..Default::default() + }, + }, + DiagnosticEntry { + range: Unclipped(PointUtf16::new(4, 6))..Unclipped(PointUtf16::new(4, 7)), + diagnostic: Diagnostic { + message: "value moved here".to_string(), + severity: DiagnosticSeverity::INFORMATION, + is_primary: false, + is_disk_based: true, + group_id: 0, + ..Default::default() + }, + }, + DiagnosticEntry { + range: Unclipped(PointUtf16::new(7, 6))..Unclipped(PointUtf16::new(7, 7)), + diagnostic: Diagnostic { + message: "use of moved value\nvalue used here after move".to_string(), + severity: DiagnosticSeverity::ERROR, + is_primary: true, + is_disk_based: true, + group_id: 0, + ..Default::default() + }, + }, + DiagnosticEntry { + range: Unclipped(PointUtf16::new(8, 6))..Unclipped(PointUtf16::new(8, 7)), + diagnostic: Diagnostic { + message: "use of moved value\nvalue used here after move".to_string(), + severity: DiagnosticSeverity::ERROR, + is_primary: true, + is_disk_based: true, + group_id: 1, + ..Default::default() + }, + }, + ], + cx, + ) + .unwrap(); + }); + + // Open the project diagnostics view while there are already diagnostics. + let view = window.build_view(cx, |cx| { + ProjectDiagnosticsEditor::new_with_context(1, project.clone(), workspace.downgrade(), cx) + }); + let editor = view.update(cx, |view, _| view.editor.clone()); + + view.next_notification(cx).await; + assert_eq!( + editor_blocks(&editor, cx), + [ + (0, "path header block".into()), + (2, "diagnostic header".into()), + (15, "collapsed context".into()), + (16, "diagnostic header".into()), + (25, "collapsed context".into()), + ] + ); + assert_eq!( + editor.update(cx, |editor, cx| editor.display_text(cx)), + concat!( + // + // main.rs + // + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + " let x = vec![];\n", + " let y = vec![];\n", + "\n", // supporting diagnostic + " a(x);\n", + " b(y);\n", + "\n", // supporting diagnostic + " // comment 1\n", + " // comment 2\n", + " c(y);\n", + "\n", // supporting diagnostic + " d(x);\n", + "\n", // context ellipsis + // diagnostic group 2 + "\n", // primary message + "\n", // padding + "fn main() {\n", + " let x = vec![];\n", + "\n", // supporting diagnostic + " let y = vec![];\n", + " a(x);\n", + "\n", // supporting diagnostic + " b(y);\n", + "\n", // context ellipsis + " c(y);\n", + " d(x);\n", + "\n", // supporting diagnostic + "}" + ) + ); + + // Cursor is at the first diagnostic + editor.update(cx, |editor, cx| { + assert_eq!( + editor.selections.display_ranges(cx), + [DisplayPoint::new(12, 6)..DisplayPoint::new(12, 6)] + ); + }); + + // Diagnostics are added for another earlier path. + project.update(cx, |project, cx| { + project.disk_based_diagnostics_started(language_server_id, cx); + project + .update_diagnostic_entries( + language_server_id, + PathBuf::from("/test/consts.rs"), + None, + vec![DiagnosticEntry { + range: Unclipped(PointUtf16::new(0, 15))..Unclipped(PointUtf16::new(0, 15)), + diagnostic: Diagnostic { + message: "mismatched types\nexpected `usize`, found `char`".to_string(), + severity: DiagnosticSeverity::ERROR, + is_primary: true, + is_disk_based: true, + group_id: 0, + ..Default::default() + }, + }], + cx, + ) + .unwrap(); + project.disk_based_diagnostics_finished(language_server_id, cx); + }); + + view.next_notification(cx).await; + assert_eq!( + editor_blocks(&editor, cx), + [ + (0, "path header block".into()), + (2, "diagnostic header".into()), + (7, "path header block".into()), + (9, "diagnostic header".into()), + (22, "collapsed context".into()), + (23, "diagnostic header".into()), + (32, "collapsed context".into()), + ] + ); + + assert_eq!( + editor.update(cx, |editor, cx| editor.display_text(cx)), + concat!( + // + // consts.rs + // + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + "const a: i32 = 'a';\n", + "\n", // supporting diagnostic + "const b: i32 = c;\n", + // + // main.rs + // + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + " let x = vec![];\n", + " let y = vec![];\n", + "\n", // supporting diagnostic + " a(x);\n", + " b(y);\n", + "\n", // supporting diagnostic + " // comment 1\n", + " // comment 2\n", + " c(y);\n", + "\n", // supporting diagnostic + " d(x);\n", + "\n", // collapsed context + // diagnostic group 2 + "\n", // primary message + "\n", // filename + "fn main() {\n", + " let x = vec![];\n", + "\n", // supporting diagnostic + " let y = vec![];\n", + " a(x);\n", + "\n", // supporting diagnostic + " b(y);\n", + "\n", // context ellipsis + " c(y);\n", + " d(x);\n", + "\n", // supporting diagnostic + "}" + ) + ); + + // Cursor keeps its position. + editor.update(cx, |editor, cx| { + assert_eq!( + editor.selections.display_ranges(cx), + [DisplayPoint::new(19, 6)..DisplayPoint::new(19, 6)] + ); + }); + + // Diagnostics are added to the first path + project.update(cx, |project, cx| { + project.disk_based_diagnostics_started(language_server_id, cx); + project + .update_diagnostic_entries( + language_server_id, + PathBuf::from("/test/consts.rs"), + None, + vec![ + DiagnosticEntry { + range: Unclipped(PointUtf16::new(0, 15))..Unclipped(PointUtf16::new(0, 15)), + diagnostic: Diagnostic { + message: "mismatched types\nexpected `usize`, found `char`".to_string(), + severity: DiagnosticSeverity::ERROR, + is_primary: true, + is_disk_based: true, + group_id: 0, + ..Default::default() + }, + }, + DiagnosticEntry { + range: Unclipped(PointUtf16::new(1, 15))..Unclipped(PointUtf16::new(1, 15)), + diagnostic: Diagnostic { + message: "unresolved name `c`".to_string(), + severity: DiagnosticSeverity::ERROR, + is_primary: true, + is_disk_based: true, + group_id: 1, + ..Default::default() + }, + }, + ], + cx, + ) + .unwrap(); + project.disk_based_diagnostics_finished(language_server_id, cx); + }); + + view.next_notification(cx).await; + assert_eq!( + editor_blocks(&editor, cx), + [ + (0, "path header block".into()), + (2, "diagnostic header".into()), + (7, "collapsed context".into()), + (8, "diagnostic header".into()), + (13, "path header block".into()), + (15, "diagnostic header".into()), + (28, "collapsed context".into()), + (29, "diagnostic header".into()), + (38, "collapsed context".into()), + ] + ); + + assert_eq!( + editor.update(cx, |editor, cx| editor.display_text(cx)), + concat!( + // + // consts.rs + // + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + "const a: i32 = 'a';\n", + "\n", // supporting diagnostic + "const b: i32 = c;\n", + "\n", // context ellipsis + // diagnostic group 2 + "\n", // primary message + "\n", // padding + "const a: i32 = 'a';\n", + "const b: i32 = c;\n", + "\n", // supporting diagnostic + // + // main.rs + // + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + " let x = vec![];\n", + " let y = vec![];\n", + "\n", // supporting diagnostic + " a(x);\n", + " b(y);\n", + "\n", // supporting diagnostic + " // comment 1\n", + " // comment 2\n", + " c(y);\n", + "\n", // supporting diagnostic + " d(x);\n", + "\n", // context ellipsis + // diagnostic group 2 + "\n", // primary message + "\n", // filename + "fn main() {\n", + " let x = vec![];\n", + "\n", // supporting diagnostic + " let y = vec![];\n", + " a(x);\n", + "\n", // supporting diagnostic + " b(y);\n", + "\n", // context ellipsis + " c(y);\n", + " d(x);\n", + "\n", // supporting diagnostic + "}" + ) + ); +} + +#[gpui::test] +async fn test_diagnostics_multiple_servers(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/test", + json!({ + "main.js": " + a(); + b(); + c(); + d(); + e(); + ".unindent() + }), + ) + .await; + + let server_id_1 = LanguageServerId(100); + let server_id_2 = LanguageServerId(101); + let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; + let window = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*window, cx); + let workspace = window.root(cx).unwrap(); + + let view = window.build_view(cx, |cx| { + ProjectDiagnosticsEditor::new_with_context(1, project.clone(), workspace.downgrade(), cx) + }); + let editor = view.update(cx, |view, _| view.editor.clone()); + + // Two language servers start updating diagnostics + project.update(cx, |project, cx| { + project.disk_based_diagnostics_started(server_id_1, cx); + project.disk_based_diagnostics_started(server_id_2, cx); + project + .update_diagnostic_entries( + server_id_1, + PathBuf::from("/test/main.js"), + None, + vec![DiagnosticEntry { + range: Unclipped(PointUtf16::new(0, 0))..Unclipped(PointUtf16::new(0, 1)), + diagnostic: Diagnostic { + message: "error 1".to_string(), + severity: DiagnosticSeverity::WARNING, + is_primary: true, + is_disk_based: true, + group_id: 1, + ..Default::default() + }, + }], + cx, + ) + .unwrap(); + }); + + // The first language server finishes + project.update(cx, |project, cx| { + project.disk_based_diagnostics_finished(server_id_1, cx); + }); + + // Only the first language server's diagnostics are shown. + cx.executor().run_until_parked(); + assert_eq!( + editor_blocks(&editor, cx), + [ + (0, "path header block".into()), + (2, "diagnostic header".into()), + ] + ); + assert_eq!( + editor.update(cx, |editor, cx| editor.display_text(cx)), + concat!( + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + "a();\n", // + "b();", + ) + ); + + // The second language server finishes + project.update(cx, |project, cx| { + project + .update_diagnostic_entries( + server_id_2, + PathBuf::from("/test/main.js"), + None, + vec![DiagnosticEntry { + range: Unclipped(PointUtf16::new(1, 0))..Unclipped(PointUtf16::new(1, 1)), + diagnostic: Diagnostic { + message: "warning 1".to_string(), + severity: DiagnosticSeverity::ERROR, + is_primary: true, + is_disk_based: true, + group_id: 2, + ..Default::default() + }, + }], + cx, + ) + .unwrap(); + project.disk_based_diagnostics_finished(server_id_2, cx); + }); + + // Both language server's diagnostics are shown. + cx.executor().run_until_parked(); + assert_eq!( + editor_blocks(&editor, cx), + [ + (0, "path header block".into()), + (2, "diagnostic header".into()), + (6, "collapsed context".into()), + (7, "diagnostic header".into()), + ] + ); + assert_eq!( + editor.update(cx, |editor, cx| editor.display_text(cx)), + concat!( + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + "a();\n", // location + "b();\n", // + "\n", // collapsed context + // diagnostic group 2 + "\n", // primary message + "\n", // padding + "a();\n", // context + "b();\n", // + "c();", // context + ) + ); + + // Both language servers start updating diagnostics, and the first server finishes. + project.update(cx, |project, cx| { + project.disk_based_diagnostics_started(server_id_1, cx); + project.disk_based_diagnostics_started(server_id_2, cx); + project + .update_diagnostic_entries( + server_id_1, + PathBuf::from("/test/main.js"), + None, + vec![DiagnosticEntry { + range: Unclipped(PointUtf16::new(2, 0))..Unclipped(PointUtf16::new(2, 1)), + diagnostic: Diagnostic { + message: "warning 2".to_string(), + severity: DiagnosticSeverity::WARNING, + is_primary: true, + is_disk_based: true, + group_id: 1, + ..Default::default() + }, + }], + cx, + ) + .unwrap(); + project + .update_diagnostic_entries( + server_id_2, + PathBuf::from("/test/main.rs"), + None, + vec![], + cx, + ) + .unwrap(); + project.disk_based_diagnostics_finished(server_id_1, cx); + }); + + // Only the first language server's diagnostics are updated. + cx.executor().run_until_parked(); + assert_eq!( + editor_blocks(&editor, cx), + [ + (0, "path header block".into()), + (2, "diagnostic header".into()), + (7, "collapsed context".into()), + (8, "diagnostic header".into()), + ] + ); + assert_eq!( + editor.update(cx, |editor, cx| editor.display_text(cx)), + concat!( + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + "a();\n", // location + "b();\n", // + "c();\n", // context + "\n", // collapsed context + // diagnostic group 2 + "\n", // primary message + "\n", // padding + "b();\n", // context + "c();\n", // + "d();", // context + ) + ); + + // The second language server finishes. + project.update(cx, |project, cx| { + project + .update_diagnostic_entries( + server_id_2, + PathBuf::from("/test/main.js"), + None, + vec![DiagnosticEntry { + range: Unclipped(PointUtf16::new(3, 0))..Unclipped(PointUtf16::new(3, 1)), + diagnostic: Diagnostic { + message: "warning 2".to_string(), + severity: DiagnosticSeverity::WARNING, + is_primary: true, + is_disk_based: true, + group_id: 1, + ..Default::default() + }, + }], + cx, + ) + .unwrap(); + project.disk_based_diagnostics_finished(server_id_2, cx); + }); + + // Both language servers' diagnostics are updated. + cx.executor().run_until_parked(); + assert_eq!( + editor_blocks(&editor, cx), + [ + (0, "path header block".into()), + (2, "diagnostic header".into()), + (7, "collapsed context".into()), + (8, "diagnostic header".into()), + ] + ); + assert_eq!( + editor.update(cx, |editor, cx| editor.display_text(cx)), + concat!( + "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding + "b();\n", // location + "c();\n", // + "d();\n", // context + "\n", // collapsed context + // diagnostic group 2 + "\n", // primary message + "\n", // padding + "c();\n", // context + "d();\n", // + "e();", // context + ) + ); +} + +#[gpui::test(iterations = 20)] +async fn test_random_diagnostics(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("/test", json!({})).await; + + let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; + let window = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*window, cx); + let workspace = window.root(cx).unwrap(); + + let mutated_view = window.build_view(cx, |cx| { + ProjectDiagnosticsEditor::new_with_context(1, project.clone(), workspace.downgrade(), cx) + }); + + workspace.update(cx, |workspace, cx| { + workspace.add_item_to_center(Box::new(mutated_view.clone()), cx); + }); + mutated_view.update(cx, |view, cx| { + assert!(view.focus_handle.is_focused(cx)); + }); + + let mut next_group_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(); + + 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}"); + project.update(cx, |project, cx| { + project.disk_based_diagnostics_finished(server_id, cx) + }); + + if rng.gen_bool(0.5) { + cx.run_until_parked(); + } + } + + // 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!("/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_insert(vec![]), + ) + } + }; + + updated_language_servers.insert(server_id); + + project.update(cx, |project, cx| { + log::info!("updating diagnostics. language server {server_id} path {path:?}"); + randomly_update_diagnostics_for_path( + &fs, + &path, + diagnostics, + &mut next_group_id, + &mut rng, + ); + project + .update_diagnostic_entries(server_id, path, None, diagnostics.clone(), cx) + .unwrap() + }); + + cx.run_until_parked(); + } + } + } + + log::info!("updating mutated diagnostics view"); + mutated_view.update(cx, |view, _| view.enqueue_update_stale_excerpts(None)); + cx.run_until_parked(); + + log::info!("constructing reference diagnostics view"); + let reference_view = window.build_view(cx, |cx| { + ProjectDiagnosticsEditor::new_with_context(1, project.clone(), workspace.downgrade(), cx) + }); + cx.run_until_parked(); + + let mutated_excerpts = get_diagnostics_excerpts(&mutated_view, cx); + let reference_excerpts = get_diagnostics_excerpts(&reference_view, cx); + assert_eq!(mutated_excerpts, reference_excerpts); +} + +fn init_test(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings = SettingsStore::test(cx); + cx.set_global(settings); + theme::init(theme::LoadThemes::JustBase, cx); + language::init(cx); + client::init_settings(cx); + workspace::init_settings(cx); + Project::init_settings(cx); + crate::init(cx); + editor::init(cx); + }); +} + +#[derive(Debug, PartialEq, Eq)] +struct ExcerptInfo { + path: PathBuf, + range: ExcerptRange, + group_id: usize, + primary: bool, + language_server: LanguageServerId, +} + +fn get_diagnostics_excerpts( + view: &View, + cx: &mut VisualTestContext, +) -> Vec { + view.update(cx, |view, cx| { + let mut result = vec![]; + let mut excerpt_indices_by_id = HashMap::default(); + view.excerpts.update(cx, |multibuffer, cx| { + let snapshot = multibuffer.snapshot(cx); + for (id, buffer, range) in snapshot.excerpts() { + excerpt_indices_by_id.insert(id, result.len()); + result.push(ExcerptInfo { + path: buffer.file().unwrap().path().to_path_buf(), + range: ExcerptRange { + context: range.context.to_point(&buffer), + primary: range.primary.map(|range| range.to_point(&buffer)), + }, + group_id: usize::MAX, + primary: false, + language_server: LanguageServerId(0), + }); + } + }); + + for state in &view.path_states { + for group in &state.diagnostic_groups { + for (ix, excerpt_id) in group.excerpts.iter().enumerate() { + let excerpt_ix = excerpt_indices_by_id[excerpt_id]; + let excerpt = &mut result[excerpt_ix]; + excerpt.group_id = group.primary_diagnostic.diagnostic.group_id; + excerpt.language_server = group.language_server_id; + excerpt.primary = ix == group.primary_excerpt_ix; + } + } + } + + result + }) +} + +fn randomly_update_diagnostics_for_path( + fs: &FakeFs, + path: &Path, + diagnostics: &mut Vec>>, + next_group_id: &mut usize, + rng: &mut impl Rng, +) { + let file_content = fs.read_file_sync(path).unwrap(); + let file_text = Rope::from(String::from_utf8_lossy(&file_content).as_ref()); + + let mut group_ids = diagnostics + .iter() + .map(|d| d.diagnostic.group_id) + .collect::>(); + + let mutation_count = rng.gen_range(1..=3); + for _ in 0..mutation_count { + if rng.gen_bool(0.5) && !group_ids.is_empty() { + let group_id = *group_ids.iter().choose(rng).unwrap(); + log::info!(" removing diagnostic group {group_id}"); + diagnostics.retain(|d| d.diagnostic.group_id != group_id); + group_ids.remove(&group_id); + } else { + let group_id = *next_group_id; + *next_group_id += 1; + + let mut new_diagnostics = vec![random_diagnostic(rng, &file_text, group_id, true)]; + for _ in 0..rng.gen_range(0..=1) { + new_diagnostics.push(random_diagnostic(rng, &file_text, group_id, false)); + } + + let ix = rng.gen_range(0..=diagnostics.len()); + log::info!( + " inserting diagnostic group {group_id} at index {ix}. ranges: {:?}", + new_diagnostics + .iter() + .map(|d| (d.range.start.0, d.range.end.0)) + .collect::>() + ); + diagnostics.splice(ix..ix, new_diagnostics); + } + } +} + +fn random_diagnostic( + rng: &mut impl Rng, + file_text: &Rope, + group_id: usize, + is_primary: bool, +) -> DiagnosticEntry> { + // Intentionally allow erroneous ranges some of the time (that run off the end of the file), + // because language servers can potentially give us those, and we should handle them gracefully. + const ERROR_MARGIN: usize = 10; + + let start = rng.gen_range(0..file_text.len().saturating_add(ERROR_MARGIN)); + let end = rng.gen_range(start..file_text.len().saturating_add(ERROR_MARGIN)); + let range = Range { + start: Unclipped(file_text.offset_to_point_utf16(start)), + end: Unclipped(file_text.offset_to_point_utf16(end)), + }; + let severity = if rng.gen_bool(0.5) { + DiagnosticSeverity::WARNING + } else { + DiagnosticSeverity::ERROR + }; + let message = format!("diagnostic group {group_id}"); + + DiagnosticEntry { + range, + diagnostic: Diagnostic { + source: None, // (optional) service that created the diagnostic + code: None, // (optional) machine-readable code that identifies the diagnostic + severity, + message, + group_id, + is_primary, + is_disk_based: false, + is_unnecessary: false, + }, + } +} + +fn editor_blocks(editor: &View, cx: &mut VisualTestContext) -> Vec<(u32, SharedString)> { + let mut blocks = Vec::new(); + cx.draw(gpui::Point::default(), AvailableSpace::min_size(), |cx| { + editor.update(cx, |editor, cx| { + let snapshot = editor.snapshot(cx); + blocks.extend( + snapshot + .blocks_in_range(0..snapshot.max_point().row()) + .enumerate() + .filter_map(|(ix, (row, block))| { + let name: SharedString = match block { + TransformBlock::Custom(block) => { + let mut element = block.render(&mut BlockContext { + context: cx, + anchor_x: px(0.), + gutter_dimensions: &GutterDimensions::default(), + line_height: px(0.), + em_width: px(0.), + max_width: px(0.), + block_id: ix, + editor_style: &editor::EditorStyle::default(), + }); + let element = element.downcast_mut::>().unwrap(); + element + .interactivity() + .element_id + .clone()? + .try_into() + .ok()? + } + + TransformBlock::ExcerptHeader { + starts_new_buffer, .. + } => { + if *starts_new_buffer { + "path header block".into() + } else { + "collapsed context".into() + } + } + }; + + Some((row, name)) + }), + ) + }); + + div().into_any() + }); + blocks +} diff --git a/crates/diagnostics/src/items.rs b/crates/diagnostics/src/items.rs index 03d46ed599..715da22ef1 100644 --- a/crates/diagnostics/src/items.rs +++ b/crates/diagnostics/src/items.rs @@ -1,13 +1,11 @@ use std::time::Duration; -use collections::HashSet; use editor::Editor; use gpui::{ percentage, rems, Animation, AnimationExt, EventEmitter, IntoElement, ParentElement, Render, Styled, Subscription, Transformation, View, ViewContext, WeakView, }; use language::Diagnostic; -use lsp::LanguageServerId; use ui::{h_flex, prelude::*, Button, ButtonLike, Color, Icon, IconName, Label, Tooltip}; use workspace::{item::ItemHandle, StatusItemView, ToolbarItemEvent, Workspace}; @@ -18,7 +16,6 @@ pub struct DiagnosticIndicator { active_editor: Option>, workspace: WeakView, current_diagnostic: Option, - in_progress_checks: HashSet, _observe_active_editor: Option, } @@ -64,7 +61,20 @@ impl Render for DiagnosticIndicator { .child(Label::new(warning_count.to_string()).size(LabelSize::Small)), }; - let status = if !self.in_progress_checks.is_empty() { + let has_in_progress_checks = self + .workspace + .upgrade() + .and_then(|workspace| { + workspace + .read(cx) + .project() + .read(cx) + .language_servers_running_disk_based_diagnostics() + .next() + }) + .is_some(); + + let status = if has_in_progress_checks { Some( h_flex() .gap_2() @@ -126,15 +136,13 @@ impl DiagnosticIndicator { pub fn new(workspace: &Workspace, cx: &mut ViewContext) -> Self { let project = workspace.project(); cx.subscribe(project, |this, project, event, cx| match event { - project::Event::DiskBasedDiagnosticsStarted { language_server_id } => { - this.in_progress_checks.insert(*language_server_id); + project::Event::DiskBasedDiagnosticsStarted { .. } => { cx.notify(); } - project::Event::DiskBasedDiagnosticsFinished { language_server_id } - | project::Event::LanguageServerRemoved(language_server_id) => { + project::Event::DiskBasedDiagnosticsFinished { .. } + | project::Event::LanguageServerRemoved(_) => { this.summary = project.read(cx).diagnostic_summary(false, cx); - this.in_progress_checks.remove(language_server_id); cx.notify(); } @@ -149,10 +157,6 @@ impl DiagnosticIndicator { Self { summary: project.read(cx).diagnostic_summary(false, cx), - in_progress_checks: project - .read(cx) - .language_servers_running_disk_based_diagnostics() - .collect(), active_editor: None, workspace: workspace.weak_handle(), current_diagnostic: None, diff --git a/crates/diagnostics/src/toolbar_controls.rs b/crates/diagnostics/src/toolbar_controls.rs index 3c09e3fad9..7f4deba73e 100644 --- a/crates/diagnostics/src/toolbar_controls.rs +++ b/crates/diagnostics/src/toolbar_controls.rs @@ -1,5 +1,5 @@ use crate::ProjectDiagnosticsEditor; -use gpui::{div, EventEmitter, ParentElement, Render, ViewContext, WeakView}; +use gpui::{EventEmitter, ParentElement, Render, ViewContext, WeakView}; use ui::prelude::*; use ui::{IconButton, IconName, Tooltip}; use workspace::{item::ItemHandle, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView}; @@ -10,12 +10,23 @@ pub struct ToolbarControls { impl Render for ToolbarControls { fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { - let include_warnings = self - .editor - .as_ref() - .and_then(|editor| editor.upgrade()) - .map(|editor| editor.read(cx).include_warnings) - .unwrap_or(false); + let mut include_warnings = false; + let mut has_stale_excerpts = false; + let mut is_updating = false; + + if let Some(editor) = self.editor.as_ref().and_then(|editor| editor.upgrade()) { + let editor = editor.read(cx); + + include_warnings = editor.include_warnings; + has_stale_excerpts = !editor.paths_to_update.is_empty(); + is_updating = editor.update_paths_tx.len() > 0 + || editor + .project + .read(cx) + .language_servers_running_disk_based_diagnostics() + .next() + .is_some(); + } let tooltip = if include_warnings { "Exclude Warnings" @@ -23,17 +34,37 @@ impl Render for ToolbarControls { "Include Warnings" }; - div().child( - IconButton::new("toggle-warnings", IconName::ExclamationTriangle) - .tooltip(move |cx| Tooltip::text(tooltip, cx)) - .on_click(cx.listener(|this, _, cx| { - if let Some(editor) = this.editor.as_ref().and_then(|editor| editor.upgrade()) { - editor.update(cx, |editor, cx| { - editor.toggle_warnings(&Default::default(), cx); - }); - } - })), - ) + h_flex() + .when(has_stale_excerpts, |div| { + div.child( + IconButton::new("update-excerpts", IconName::Update) + .icon_color(Color::Info) + .disabled(is_updating) + .tooltip(move |cx| Tooltip::text("Update excerpts", cx)) + .on_click(cx.listener(|this, _, cx| { + if let Some(editor) = + this.editor.as_ref().and_then(|editor| editor.upgrade()) + { + editor.update(cx, |editor, _| { + editor.enqueue_update_stale_excerpts(None); + }); + } + })), + ) + }) + .child( + IconButton::new("toggle-warnings", IconName::ExclamationTriangle) + .tooltip(move |cx| Tooltip::text(tooltip, cx)) + .on_click(cx.listener(|this, _, cx| { + if let Some(editor) = + this.editor.as_ref().and_then(|editor| editor.upgrade()) + { + editor.update(cx, |editor, cx| { + editor.toggle_warnings(&Default::default(), cx); + }); + } + })), + ) } } diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 6748a94b00..5cdc755808 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -714,6 +714,15 @@ impl FakeFs { Ok(()) } + pub fn read_file_sync(&self, path: impl AsRef) -> Result> { + let path = path.as_ref(); + let path = normalize_path(path); + let state = self.state.lock(); + let entry = state.read_path(&path)?; + let entry = entry.lock(); + entry.file_content(&path).cloned() + } + async fn load_internal(&self, path: impl AsRef) -> Result> { let path = path.as_ref(); let path = normalize_path(path); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 34a6359db4..fa4c8d88d3 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2699,7 +2699,6 @@ impl Project { for (_, _, server) in self.language_servers_for_worktree(worktree_id) { let text = include_text(server.as_ref()).then(|| buffer.read(cx).text()); - server .notify::( lsp::DidSaveTextDocumentParams { @@ -2710,46 +2709,8 @@ impl Project { .log_err(); } - let language_server_ids = self.language_server_ids_for_buffer(buffer.read(cx), cx); - for language_server_id in language_server_ids { - if let Some(LanguageServerState::Running { - adapter, - simulate_disk_based_diagnostics_completion, - .. - }) = self.language_servers.get_mut(&language_server_id) - { - // After saving a buffer using a language server that doesn't provide - // a disk-based progress token, kick off a timer that will reset every - // time the buffer is saved. If the timer eventually fires, simulate - // disk-based diagnostics being finished so that other pieces of UI - // (e.g., project diagnostics view, diagnostic status bar) can update. - // We don't emit an event right away because the language server might take - // some time to publish diagnostics. - if adapter.disk_based_diagnostics_progress_token.is_none() { - const DISK_BASED_DIAGNOSTICS_DEBOUNCE: Duration = - Duration::from_secs(1); - - let task = cx.spawn(move |this, mut cx| async move { - cx.background_executor().timer(DISK_BASED_DIAGNOSTICS_DEBOUNCE).await; - if let Some(this) = this.upgrade() { - this.update(&mut cx, |this, cx| { - this.disk_based_diagnostics_finished( - language_server_id, - cx, - ); - this.enqueue_buffer_ordered_message( - BufferOrderedMessage::LanguageServerUpdate { - language_server_id, - message:proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(Default::default()) - }, - ) - .ok(); - }).ok(); - } - }); - *simulate_disk_based_diagnostics_completion = Some(task); - } - } + for language_server_id in self.language_server_ids_for_buffer(buffer.read(cx), cx) { + self.simulate_disk_based_diagnostics_events_if_needed(language_server_id, cx); } } BufferEvent::FileHandleChanged => { @@ -2783,6 +2744,57 @@ impl Project { None } + // After saving a buffer using a language server that doesn't provide a disk-based progress token, + // kick off a timer that will reset every time the buffer is saved. If the timer eventually fires, + // simulate disk-based diagnostics being finished so that other pieces of UI (e.g., project + // diagnostics view, diagnostic status bar) can update. We don't emit an event right away because + // the language server might take some time to publish diagnostics. + fn simulate_disk_based_diagnostics_events_if_needed( + &mut self, + language_server_id: LanguageServerId, + cx: &mut ModelContext, + ) { + const DISK_BASED_DIAGNOSTICS_DEBOUNCE: Duration = Duration::from_secs(1); + + let Some(LanguageServerState::Running { + simulate_disk_based_diagnostics_completion, + adapter, + .. + }) = self.language_servers.get_mut(&language_server_id) + else { + return; + }; + + if adapter.disk_based_diagnostics_progress_token.is_some() { + return; + } + + let prev_task = simulate_disk_based_diagnostics_completion.replace(cx.spawn( + move |this, mut cx| async move { + cx.background_executor() + .timer(DISK_BASED_DIAGNOSTICS_DEBOUNCE) + .await; + + this.update(&mut cx, |this, cx| { + this.disk_based_diagnostics_finished(language_server_id, cx); + + if let Some(LanguageServerState::Running { + simulate_disk_based_diagnostics_completion, + .. + }) = this.language_servers.get_mut(&language_server_id) + { + *simulate_disk_based_diagnostics_completion = None; + } + }) + .ok(); + }, + )); + + if prev_task.is_none() { + self.disk_based_diagnostics_started(language_server_id, cx); + } + } + fn request_buffer_diff_recalculation( &mut self, buffer: &Model, @@ -4041,13 +4053,7 @@ impl Project { match progress { lsp::WorkDoneProgress::Begin(report) => { if is_disk_based_diagnostics_progress { - language_server_status.has_pending_diagnostic_updates = true; self.disk_based_diagnostics_started(language_server_id, cx); - self.enqueue_buffer_ordered_message(BufferOrderedMessage::LanguageServerUpdate { - language_server_id, - message: proto::update_language_server::Variant::DiskBasedDiagnosticsUpdating(Default::default()) - }) - .ok(); } else { self.on_lsp_work_start( language_server_id, @@ -4092,18 +4098,7 @@ impl Project { language_server_status.progress_tokens.remove(&token); if is_disk_based_diagnostics_progress { - language_server_status.has_pending_diagnostic_updates = false; self.disk_based_diagnostics_finished(language_server_id, cx); - self.enqueue_buffer_ordered_message( - BufferOrderedMessage::LanguageServerUpdate { - language_server_id, - message: - proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated( - Default::default(), - ), - }, - ) - .ok(); } else { self.on_lsp_work_end(language_server_id, token.clone(), cx); } @@ -7708,13 +7703,7 @@ 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) - .filter(|(path, _, _)| { - let worktree = self.entry_for_path(path, cx).map(|entry| entry.is_ignored); - include_ignored || worktree == Some(false) - }) - { + for (_, _, path_summary) in self.diagnostic_summaries(include_ignored, cx) { summary.error_count += path_summary.error_count; summary.warning_count += path_summary.warning_count; } @@ -7726,20 +7715,23 @@ impl Project { include_ignored: bool, cx: &'a AppContext, ) -> impl Iterator + 'a { - self.visible_worktrees(cx) - .flat_map(move |worktree| { - let worktree = worktree.read(cx); - let worktree_id = worktree.id(); - worktree - .diagnostic_summaries() - .map(move |(path, server_id, summary)| { - (ProjectPath { worktree_id, path }, server_id, summary) - }) - }) - .filter(move |(path, _, _)| { - let worktree = self.entry_for_path(path, cx).map(|entry| entry.is_ignored); - include_ignored || worktree == Some(false) - }) + self.visible_worktrees(cx).flat_map(move |worktree| { + let worktree = worktree.read(cx); + let worktree_id = worktree.id(); + worktree + .diagnostic_summaries() + .filter_map(move |(path, server_id, summary)| { + if include_ignored + || worktree + .entry_for_path(path.as_ref()) + .map_or(false, |entry| !entry.is_ignored) + { + Some((ProjectPath { worktree_id, path }, server_id, summary)) + } else { + None + } + }) + }) } pub fn disk_based_diagnostics_started( @@ -7747,7 +7739,22 @@ impl Project { language_server_id: LanguageServerId, cx: &mut ModelContext, ) { + if let Some(language_server_status) = + self.language_server_statuses.get_mut(&language_server_id) + { + language_server_status.has_pending_diagnostic_updates = true; + } + cx.emit(Event::DiskBasedDiagnosticsStarted { language_server_id }); + if self.is_local() { + self.enqueue_buffer_ordered_message(BufferOrderedMessage::LanguageServerUpdate { + language_server_id, + message: proto::update_language_server::Variant::DiskBasedDiagnosticsUpdating( + Default::default(), + ), + }) + .ok(); + } } pub fn disk_based_diagnostics_finished( @@ -7755,7 +7762,23 @@ impl Project { language_server_id: LanguageServerId, cx: &mut ModelContext, ) { + if let Some(language_server_status) = + self.language_server_statuses.get_mut(&language_server_id) + { + language_server_status.has_pending_diagnostic_updates = false; + } + cx.emit(Event::DiskBasedDiagnosticsFinished { language_server_id }); + + if self.is_local() { + self.enqueue_buffer_ordered_message(BufferOrderedMessage::LanguageServerUpdate { + language_server_id, + message: proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated( + Default::default(), + ), + }) + .ok(); + } } pub fn active_entry(&self) -> Option {