From 0972766d1dda3d3722367f8f553454ef8731b968 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 27 Jun 2023 00:41:20 +0300 Subject: [PATCH] Add more hint tests --- crates/collab/src/tests/integration_tests.rs | 226 +++++++++++ crates/editor/src/inlay_hint_cache.rs | 374 +++++++++++++++---- crates/language/src/language_settings.rs | 3 - 3 files changed, 533 insertions(+), 70 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 905ce6d2e1..3cf4d9a876 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -8134,6 +8134,232 @@ async fn test_mutual_editor_inlay_hint_cache_update( }); } +#[gpui::test] +async fn test_inlay_hint_refresh_is_forwarded( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)]) + .await; + let active_call_a = cx_a.read(ActiveCall::global); + let active_call_b = cx_b.read(ActiveCall::global); + + cx_a.update(editor::init); + cx_b.update(editor::init); + + cx_a.update(|cx| { + cx.update_global(|store: &mut SettingsStore, cx| { + store.update_user_settings::(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: false, + show_type_hints: true, + show_parameter_hints: false, + show_other_hints: true, + }) + }); + }); + }); + cx_b.update(|cx| { + cx.update_global(|store: &mut SettingsStore, cx| { + store.update_user_settings::(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: true, + show_type_hints: true, + show_parameter_hints: false, + show_other_hints: true, + }) + }); + }); + }); + let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]); + + let mut language = Language::new( + LanguageConfig { + name: "Rust".into(), + path_suffixes: vec!["rs".to_string()], + ..Default::default() + }, + Some(tree_sitter_rust::language()), + ); + let mut fake_language_servers = language + .set_fake_lsp_adapter(Arc::new(FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + inlay_hint_provider: Some(lsp::OneOf::Left(true)), + ..Default::default() + }, + ..Default::default() + })) + .await; + let language = Arc::new(language); + client_a.language_registry.add(Arc::clone(&language)); + client_b.language_registry.add(language); + + client_a + .fs + .insert_tree( + "/a", + json!({ + "main.rs": "fn main() { a } // and some long comment to ensure inlays are not trimmed out", + "other.rs": "// Test file", + }), + ) + .await; + let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await; + active_call_a + .update(cx_a, |call, cx| call.set_location(Some(&project_a), cx)) + .await + .unwrap(); + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + + let project_b = client_b.build_remote_project(project_id, cx_b).await; + active_call_b + .update(cx_b, |call, cx| call.set_location(Some(&project_b), cx)) + .await + .unwrap(); + + let workspace_a = client_a.build_workspace(&project_a, cx_a); + let workspace_b = client_b.build_workspace(&project_b, cx_b); + cx_a.foreground().start_waiting(); + cx_b.foreground().start_waiting(); + + let editor_a = workspace_a + .update(cx_a, |workspace, cx| { + workspace.open_path((worktree_id, "main.rs"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + + let editor_b = workspace_b + .update(cx_b, |workspace, cx| { + workspace.open_path((worktree_id, "main.rs"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + + let fake_language_server = fake_language_servers.next().await.unwrap(); + let next_call_id = Arc::new(AtomicU32::new(0)); + fake_language_server + .handle_request::(move |params, _| { + let task_next_call_id = Arc::clone(&next_call_id); + async move { + assert_eq!( + params.text_document.uri, + lsp::Url::from_file_path("/a/main.rs").unwrap(), + ); + let mut current_call_id = Arc::clone(&task_next_call_id).fetch_add(1, SeqCst); + let mut new_hints = Vec::with_capacity(current_call_id as usize); + loop { + new_hints.push(lsp::InlayHint { + position: lsp::Position::new(0, current_call_id), + label: lsp::InlayHintLabel::String(current_call_id.to_string()), + kind: None, + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }); + if current_call_id == 0 { + break; + } + current_call_id -= 1; + } + Ok(Some(new_hints)) + } + }) + .next() + .await + .unwrap(); + cx_a.foreground().finish_waiting(); + cx_b.foreground().finish_waiting(); + + cx_a.foreground().run_until_parked(); + editor_a.update(cx_a, |editor, _| { + assert!( + extract_hint_labels(editor).is_empty(), + "Host should get no hints due to them turned off" + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!( + inlay_cache.allowed_hint_kinds, allowed_hint_kinds, + "Host should have allowed hint kinds set despite hints are off" + ); + assert_eq!( + inlay_cache.version, 0, + "Host should not increment its cache version due to no changes", + ); + }); + + let mut edits_made = 1; + cx_b.foreground().run_until_parked(); + editor_b.update(cx_b, |editor, _| { + assert_eq!( + vec!["0".to_string()], + extract_hint_labels(editor), + "Client should get its first hints when opens an editor" + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!( + inlay_cache.allowed_hint_kinds, allowed_hint_kinds, + "Cache should use editor settings to get the allowed hint kinds" + ); + assert_eq!( + inlay_cache.version, edits_made, + "Guest editor update the cache version after every cache/view change" + ); + }); + + fake_language_server + .request::(()) + .await + .expect("inlay refresh request failed"); + cx_a.foreground().run_until_parked(); + editor_a.update(cx_a, |editor, _| { + assert!( + extract_hint_labels(editor).is_empty(), + "Host should get nop hints due to them turned off, even after the /refresh" + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds); + assert_eq!( + inlay_cache.version, 0, + "Host should not increment its cache version due to no changes", + ); + }); + + edits_made += 1; + cx_b.foreground().run_until_parked(); + editor_b.update(cx_b, |editor, _| { + assert_eq!( + vec!["0".to_string(), "1".to_string(),], + extract_hint_labels(editor), + "Guest should get a /refresh LSP request propagated by host despite host hints are off" + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!( + inlay_cache.allowed_hint_kinds, allowed_hint_kinds, + "Inlay kinds settings never change during the test" + ); + assert_eq!( + inlay_cache.version, edits_made, + "Guest should accepted all edits and bump its cache version every time" + ); + }); +} + #[derive(Debug, Eq, PartialEq)] struct RoomParticipants { remote: Vec, diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index 1a03886c91..43fb7ba7cc 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -160,7 +160,6 @@ impl InlayHintCache { visible_hints: Vec, cx: &mut ViewContext, ) -> ControlFlow> { - dbg!(new_hint_settings); let new_allowed_hint_kinds = new_hint_settings.enabled_inlay_hint_kinds(); match (self.enabled, new_hint_settings.enabled) { (false, false) => { @@ -352,7 +351,6 @@ impl InlayHintCache { self.version += 1; self.update_tasks.clear(); self.hints.clear(); - self.allowed_hint_kinds.clear(); } } @@ -800,8 +798,7 @@ mod tests { use futures::StreamExt; use gpui::{TestAppContext, ViewHandle}; use language::{ - language_settings::{AllLanguageSettings, AllLanguageSettingsContent}, - FakeLspAdapter, Language, LanguageConfig, + language_settings::AllLanguageSettingsContent, FakeLspAdapter, Language, LanguageConfig, }; use lsp::FakeLanguageServer; use project::{FakeFs, Project}; @@ -814,11 +811,16 @@ mod tests { #[gpui::test] async fn test_basic_cache_update_with_duplicate_hints(cx: &mut gpui::TestAppContext) { - init_test(cx, |_| {}); let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]); - let (file_with_hints, editor, fake_server) = - prepare_test_objects(cx, &allowed_hint_kinds).await; - + init_test(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: true, + show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)), + show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)), + show_other_hints: allowed_hint_kinds.contains(&None), + }) + }); + let (file_with_hints, editor, fake_server) = prepare_test_objects(cx).await; let lsp_request_count = Arc::new(AtomicU32::new(0)); fake_server .handle_request::(move |params, _| { @@ -931,22 +933,7 @@ mod tests { async fn prepare_test_objects( cx: &mut TestAppContext, - allowed_hint_kinds: &HashSet>, ) -> (&'static str, ViewHandle, FakeLanguageServer) { - cx.update(|cx| { - cx.update_global(|store: &mut SettingsStore, cx| { - store.update_user_settings::(cx, |settings| { - settings.defaults.inlay_hints = Some(InlayHintSettings { - enabled: true, - show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)), - show_parameter_hints: allowed_hint_kinds - .contains(&Some(InlayHintKind::Parameter)), - show_other_hints: allowed_hint_kinds.contains(&None), - }) - }); - }); - }); - let mut language = Language::new( LanguageConfig { name: "Rust".into(), @@ -1001,57 +988,73 @@ mod tests { #[gpui::test] async fn test_hint_setting_changes(cx: &mut gpui::TestAppContext) { - init_test(cx, |_| {}); let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]); - let (file_with_hints, editor, fake_server) = - prepare_test_objects(cx, &allowed_hint_kinds).await; - + init_test(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: true, + show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)), + show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)), + show_other_hints: allowed_hint_kinds.contains(&None), + }) + }); + let (file_with_hints, editor, fake_server) = prepare_test_objects(cx).await; + let lsp_request_count = Arc::new(AtomicU32::new(0)); + let another_lsp_request_count = Arc::clone(&lsp_request_count); fake_server - .handle_request::(move |params, _| async move { - assert_eq!( - params.text_document.uri, - lsp::Url::from_file_path(file_with_hints).unwrap(), - ); - Ok(Some(vec![ - lsp::InlayHint { - position: lsp::Position::new(0, 1), - label: lsp::InlayHintLabel::String("type hint".to_string()), - kind: Some(lsp::InlayHintKind::TYPE), - text_edits: None, - tooltip: None, - padding_left: None, - padding_right: None, - data: None, - }, - lsp::InlayHint { - position: lsp::Position::new(0, 2), - label: lsp::InlayHintLabel::String("parameter hint".to_string()), - kind: Some(lsp::InlayHintKind::PARAMETER), - text_edits: None, - tooltip: None, - padding_left: None, - padding_right: None, - data: None, - }, - lsp::InlayHint { - position: lsp::Position::new(0, 3), - label: lsp::InlayHintLabel::String("other hint".to_string()), - kind: None, - text_edits: None, - tooltip: None, - padding_left: None, - padding_right: None, - data: None, - }, - ])) + .handle_request::(move |params, _| { + let task_lsp_request_count = Arc::clone(&another_lsp_request_count); + async move { + Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::SeqCst); + assert_eq!( + params.text_document.uri, + lsp::Url::from_file_path(file_with_hints).unwrap(), + ); + Ok(Some(vec![ + lsp::InlayHint { + position: lsp::Position::new(0, 1), + label: lsp::InlayHintLabel::String("type hint".to_string()), + kind: Some(lsp::InlayHintKind::TYPE), + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }, + lsp::InlayHint { + position: lsp::Position::new(0, 2), + label: lsp::InlayHintLabel::String("parameter hint".to_string()), + kind: Some(lsp::InlayHintKind::PARAMETER), + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }, + lsp::InlayHint { + position: lsp::Position::new(0, 3), + label: lsp::InlayHintLabel::String("other hint".to_string()), + kind: None, + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }, + ])) + } }) .next() .await; cx.foreground().finish_waiting(); cx.foreground().run_until_parked(); - let edits_made = 1; + let mut edits_made = 1; editor.update(cx, |editor, cx| { + assert_eq!( + lsp_request_count.load(Ordering::Relaxed), + 1, + "Should query new hints once" + ); assert_eq!( vec![ "type hint".to_string(), @@ -1076,10 +1079,247 @@ mod tests { ); }); - // + fake_server + .request::(()) + .await + .expect("inlay refresh request failed"); + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + assert_eq!( + lsp_request_count.load(Ordering::Relaxed), + 2, + "Should load new hints twice" + ); + assert_eq!( + vec![ + "type hint".to_string(), + "parameter hint".to_string(), + "other hint".to_string() + ], + cached_hint_labels(editor), + "Cached hints should not change due to allowed hint kinds settings update" + ); + assert_eq!( + vec!["type hint".to_string(), "other hint".to_string()], + visible_hint_labels(editor, cx) + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds); + assert_eq!( + inlay_cache.version, edits_made, + "Should not update cache version due to new loaded hints being the same" + ); + }); + + for (new_allowed_hint_kinds, expected_visible_hints) in [ + (HashSet::from_iter([None]), vec!["other hint".to_string()]), + ( + HashSet::from_iter([Some(InlayHintKind::Type)]), + vec!["type hint".to_string()], + ), + ( + HashSet::from_iter([Some(InlayHintKind::Parameter)]), + vec!["parameter hint".to_string()], + ), + ( + HashSet::from_iter([None, Some(InlayHintKind::Type)]), + vec!["type hint".to_string(), "other hint".to_string()], + ), + ( + HashSet::from_iter([None, Some(InlayHintKind::Parameter)]), + vec!["parameter hint".to_string(), "other hint".to_string()], + ), + ( + HashSet::from_iter([Some(InlayHintKind::Type), Some(InlayHintKind::Parameter)]), + vec!["type hint".to_string(), "parameter hint".to_string()], + ), + ( + HashSet::from_iter([ + None, + Some(InlayHintKind::Type), + Some(InlayHintKind::Parameter), + ]), + vec![ + "type hint".to_string(), + "parameter hint".to_string(), + "other hint".to_string(), + ], + ), + ] { + edits_made += 1; + update_test_settings(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: true, + show_type_hints: new_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)), + show_parameter_hints: new_allowed_hint_kinds + .contains(&Some(InlayHintKind::Parameter)), + show_other_hints: new_allowed_hint_kinds.contains(&None), + }) + }); + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + assert_eq!( + lsp_request_count.load(Ordering::Relaxed), + 2, + "Should not load new hints on allowed hint kinds change for hint kinds {new_allowed_hint_kinds:?}" + ); + assert_eq!( + vec![ + "type hint".to_string(), + "parameter hint".to_string(), + "other hint".to_string(), + ], + cached_hint_labels(editor), + "Should get its cached hints unchanged after the settings change for hint kinds {new_allowed_hint_kinds:?}" + ); + assert_eq!( + expected_visible_hints, + visible_hint_labels(editor, cx), + "Should get its visible hints filtered after the settings change for hint kinds {new_allowed_hint_kinds:?}" + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!( + inlay_cache.allowed_hint_kinds, new_allowed_hint_kinds, + "Cache should use editor settings to get the allowed hint kinds for hint kinds {new_allowed_hint_kinds:?}" + ); + assert_eq!( + inlay_cache.version, edits_made, + "The editor should update the cache version after every cache/view change for hint kinds {new_allowed_hint_kinds:?} due to visible hints change" + ); + }); + } + + edits_made += 1; + let another_allowed_hint_kinds = HashSet::from_iter([Some(InlayHintKind::Type)]); + update_test_settings(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: false, + show_type_hints: another_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)), + show_parameter_hints: another_allowed_hint_kinds + .contains(&Some(InlayHintKind::Parameter)), + show_other_hints: another_allowed_hint_kinds.contains(&None), + }) + }); + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + assert_eq!( + lsp_request_count.load(Ordering::Relaxed), + 2, + "Should not load new hints when hints got disabled" + ); + assert!( + cached_hint_labels(editor).is_empty(), + "Should clear the cache when hints got disabled" + ); + assert!( + visible_hint_labels(editor, cx).is_empty(), + "Should clear visible hints when hints got disabled" + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!( + inlay_cache.allowed_hint_kinds, another_allowed_hint_kinds, + "Should update its allowed hint kinds even when hints got disabled" + ); + assert_eq!( + inlay_cache.version, edits_made, + "The editor should update the cache version after hints got disabled" + ); + }); + + fake_server + .request::(()) + .await + .expect("inlay refresh request failed"); + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + assert_eq!( + lsp_request_count.load(Ordering::Relaxed), + 2, + "Should not load new hints when they got disabled" + ); + assert!(cached_hint_labels(editor).is_empty()); + assert!(visible_hint_labels(editor, cx).is_empty()); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, another_allowed_hint_kinds); + assert_eq!( + inlay_cache.version, edits_made, + "The editor should not update the cache version after /refresh query without updates" + ); + }); + + let final_allowed_hint_kinds = HashSet::from_iter([Some(InlayHintKind::Parameter)]); + edits_made += 1; + update_test_settings(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: true, + show_type_hints: final_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)), + show_parameter_hints: final_allowed_hint_kinds + .contains(&Some(InlayHintKind::Parameter)), + show_other_hints: final_allowed_hint_kinds.contains(&None), + }) + }); + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + assert_eq!( + lsp_request_count.load(Ordering::Relaxed), + 3, + "Should query for new hints when they got reenabled" + ); + assert_eq!( + vec![ + "type hint".to_string(), + "parameter hint".to_string(), + "other hint".to_string(), + ], + cached_hint_labels(editor), + "Should get its cached hints fully repopulated after the hints got reenabled" + ); + assert_eq!( + vec!["parameter hint".to_string()], + visible_hint_labels(editor, cx), + "Should get its visible hints repopulated and filtered after the h" + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!( + inlay_cache.allowed_hint_kinds, final_allowed_hint_kinds, + "Cache should update editor settings when hints got reenabled" + ); + assert_eq!( + inlay_cache.version, edits_made, + "Cache should update its version after hints got reenabled" + ); + }); + + fake_server + .request::(()) + .await + .expect("inlay refresh request failed"); + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + assert_eq!( + lsp_request_count.load(Ordering::Relaxed), + 4, + "Should query for new hints again" + ); + assert_eq!( + vec![ + "type hint".to_string(), + "parameter hint".to_string(), + "other hint".to_string(), + ], + cached_hint_labels(editor), + ); + assert_eq!( + vec!["parameter hint".to_string()], + visible_hint_labels(editor, cx), + ); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, final_allowed_hint_kinds,); + assert_eq!(inlay_cache.version, edits_made); + }); } - pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsContent)) { + pub(crate) fn init_test(cx: &mut TestAppContext, f: impl Fn(&mut AllLanguageSettingsContent)) { cx.foreground().forbid_parking(); cx.update(|cx| { diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index aeceac9493..820217567a 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -172,9 +172,6 @@ fn default_true() -> bool { impl InlayHintSettings { pub fn enabled_inlay_hint_kinds(&self) -> HashSet> { let mut kinds = HashSet::default(); - if !self.enabled { - return kinds; - } if self.show_type_hints { kinds.insert(Some(InlayHintKind::Type)); }